Subject: [PATCH v3 00/11] reftable: small set of fixes

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 11 Dec 2023 10:07:25 +0100

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.
     
         Signed-off-by: Patrick Steinhardt <[email-masked]>
     
 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