reftable: small set of fixes

Hi,

this is the third version of my patch series that addresses several smallish issues in the reftable backend.

There's only a small set of changes compared to v2:

- Patch 4: convert to use a struct strbuf instead of snprintf().

- Patch 5: improve commit message.

- Patch 6: note that stack_filename() resets the struct strbuf in the commit message.

- Patch 7: use the struct filelock's lock path instead of the temporary buffer.

Thanks for your suggestions, Taylor and Eric!

Patrick

Patrick Steinhardt (11):
reftable: wrap EXPECT macros in do/while
reftable: handle interrupted reads
reftable: handle interrupted writes
reftable/stack: verify that reftable_stack_add() uses
auto-compaction
reftable/stack: perform auto-compaction with transactional interface reftable/stack: reuse buffers when reloading stack reftable/stack: fix stale lock when dying
reftable/stack: fix use of unseeded randomness reftable/merged: reuse buffer to compute record keys reftable/block: introduce macro to initialize struct block_iter reftable/block: reuse buffer to compute record keys

 reftable/block.c          |  23 ++++----
 reftable/block.h          |   6 +++
 reftable/block_test.c     |   4 +-
 reftable/blocksource.c    |   2 +-
 reftable/iter.h           |   8 +--
 reftable/merged.c         |  31 +++++------
 reftable/merged.h         |   2 +
 reftable/reader.c         |   7 ++-
 reftable/readwrite_test.c |   6 +--
 reftable/stack.c          |  73 +++++++++++---------------
 reftable/stack_test.c     | 107 +++++++++++++++++++++++++++++++++++++-
 reftable/test_framework.h |  58 ++++++++++++---------
 12 files changed, 213 insertions(+), 114 deletions(-)

Range-diff against v2:

 1:  0ebbb02d32 =  1:  5b2a64ca9f reftable: wrap EXPECT macros in do/while
 2:  b404fdf066 =  2:  3e8e63ece5 reftable: handle interrupted reads
 3:  8c1d78b12b =  3:  1700d00d1c reftable: handle interrupted writes
 4:  8061b9d2fc !  4:  5e27d0a556 reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +{
     +	struct reftable_write_options cfg = { 0 };
     +	struct reftable_stack *st = NULL;
    ++	struct strbuf refname = STRBUF_INIT;
     +	char *dir = get_tmp_dir(__LINE__);
     +	int err, i, n = 20;
     +
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +			.value_type = REFTABLE_REF_SYMREF,
     +			.value.symref = "master",
     +		};
    -+		char name[100];
     +
     +		/*
     +		 * Disable auto-compaction for all but the last runs. Like this
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +		 */
     +		st->disable_auto_compact = i != n;
     +
    -+		snprintf(name, sizeof(name), "branch%04d", i);
    -+		ref.refname = name;
    ++		strbuf_reset(&refname);
    ++		strbuf_addf(&refname, "branch-%04d", i);
    ++		ref.refname = refname.buf;
     +
     +		err = reftable_stack_add(st, &write_test_ref, &ref);
     +		EXPECT_ERR(err);
    @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void)
     +	}
     +
     +	reftable_stack_destroy(st);
    ++	strbuf_release(&refname);
     +	clear_dir(dir);
     +}
     +

5: 77b9ae8aa6 ! 5: dd180eba40 reftable/stack: perform auto-compaction with transactional interface
@@ Commit message

Whenever updating references or reflog entries in the reftable stack, we need to add a new table to the stack, thus growing the stack's length by
- one. It can thus happen quite fast that the stack grows very long, which
- results in performance issues when trying to read records. But besides
- performance issues, this can also lead to exhaustion of file descriptors
- very rapidly as every single table requires a separate descriptor when
+ one. The stack can grow to become quite long rather quickly, leading to
+ performance issues when trying to read records. But besides performance
+ issues, this can also lead to exhaustion of file descriptors very
+ rapidly as every single table requires a separate descriptor when
opening the stack.

While git-pack-refs(1) fixes this issue for us by merging the tables, it
6: f797feff8d ! 6: 6ed9ba60db reftable/stack: reuse buffers when reloading stack
@@ Commit message
of the allocated buffer outside of the loop.

Refactor the code to instead reuse the buffers to reduce the number of
- allocations we need to do.
+ allocations we need to do. Note that we do not have to manually reset
+ the buffer because stack_filename() does this for us already.

7: e82a68aecd ! 7: fbd9efa56d reftable/stack: fix stale lock when dying

    @@ reftable/stack.c: static int reftable_stack_init_addition(struct reftable_additi
      	}
      	if (st->config.default_permissions) {
     -		if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
    -+		if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
    ++		if (chmod(add->lock_file->filename.buf, st->config.default_permissions) < 0) {
      			err = REFTABLE_IO_ERROR;
      			goto done;
      		}
 8:  bab4fb93df =  8:  5598460b81 reftable/stack: fix use of unseeded randomness
 9:  cbf77ec45a =  9:  79e0382603 reftable/merged: reuse buffer to compute record keys
10:  c9a1405a9a = 10:  8574ad7635 reftable/block: introduce macro to initialize `struct block_iter`
11:  02b11f3a80 = 11:  eeb6c35823 reftable/block: reuse buffer to compute record keys

base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d

2.43.0