Re: [PATCH v5 0/3] reftable/stack: use geometric table compaction

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 6087 bytes --]

On Thu, Apr 04, 2024 at 06:29:26PM +0000, Justin Tobler via GitGitGadget wrote:
> Hello again,
> 
> This is the fifth version my patch series that refactors the reftable
> compaction strategy to instead follow a geometric sequence. Changes compared
> to v4:
> 
>  * To fix some failing tests and conflicts, this patch series now depends on
>    the ps/pack-refs-auto series which is currently in next.
>  * Lifted the GIT_TEST_REFTABLE_AUTOCOMPACTION env out of the reftable
>    library and into the reftable backend code.
> 
> Thanks for taking a look!
> 
> -Justin

I've added two additional nits which you may or may not want to address.
But overall this patch series looks good to me. Thanks!

Patrick

> Justin Tobler (3):
>   reftable/stack: allow disabling of auto-compaction
>   reftable/stack: add env to disable autocompaction
>   reftable/stack: use geometric table compaction
> 
>  refs/reftable-backend.c    |   4 ++
>  reftable/reftable-writer.h |   3 +
>  reftable/stack.c           | 125 +++++++++++++++++++------------------
>  reftable/stack.h           |   4 --
>  reftable/stack_test.c      |  77 ++++++-----------------
>  t/t0610-reftable-basics.sh |  71 ++++++++++++++++-----
>  6 files changed, 146 insertions(+), 138 deletions(-)
> 
> 
> base-commit: 4b32163adf4863c6df3bb6b43540fa2ca3494e28
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1683%2Fjltobler%2Fjt%2Freftable-geometric-compaction-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1683/jltobler/jt/reftable-geometric-compaction-v5
> Pull-Request: https://github.com/gitgitgadget/git/pull/1683
> 
> Range-diff vs v4:
> 
>  -:  ----------- > 1:  a7011dbc6aa reftable/stack: allow disabling of auto-compaction
>  1:  2a0421e5f20 ! 2:  7c4fe0e9ec5 reftable/stack: add env to disable autocompaction
>      @@ Commit message
>       
>           Signed-off-by: Justin Tobler <jltobler@gmail.com>
>       
>      - ## reftable/stack.c ##
>      -@@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
>      - 	if (err)
>      - 		goto done;
>      - 
>      --	if (!add->stack->disable_auto_compact)
>      -+	if (!add->stack->disable_auto_compact &&
>      -+	    git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1))
>      - 		err = reftable_stack_auto_compact(add->stack);
>      - 
>      - done:
>      -
>      - ## reftable/system.h ##
>      -@@ reftable/system.h: license that can be found in the LICENSE file or at
>      - #include "tempfile.h"
>      - #include "hash-ll.h" /* hash ID, sizes.*/
>      - #include "dir.h" /* remove_dir_recursively, for tests.*/
>      + ## refs/reftable-backend.c ##
>      +@@
>      + #include "../reftable/reftable-merged.h"
>      + #include "../setup.h"
>      + #include "../strmap.h"
>       +#include "parse.h"
>      + #include "refs-internal.h"
>        
>      - int hash_size(uint32_t id);
>      + /*
>      +@@ refs/reftable-backend.c: static struct ref_store *reftable_be_init(struct repository *repo,
>      + 	refs->write_options.hash_id = repo->hash_algo->format_id;
>      + 	refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
>        
>      ++	if (!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1))
>      ++		refs->write_options.disable_auto_compact = 1;
>      ++
>      + 	/*
>      + 	 * Set up the main reftable stack that is hosted in GIT_COMMON_DIR.
>      + 	 * This stack contains both the shared and the main worktree refs.
>       
>        ## t/t0610-reftable-basics.sh ##
>       @@ t/t0610-reftable-basics.sh: test_expect_success 'ref transaction: writes cause auto-compaction' '
>  2:  e0f4d0dbcc1 ! 3:  8f124acf0f8 reftable/stack: use geometric table compaction
>      @@ reftable/stack_test.c: static void test_empty_add(void)
>       +
>        static void test_reftable_stack_auto_compaction(void)
>        {
>      - 	struct reftable_write_options cfg = { 0 };
>      + 	struct reftable_write_options cfg = {
>       @@ reftable/stack_test.c: static void test_reftable_stack_compaction_concurrent_clean(void)
>        int stack_test_main(int argc, const char *argv[])
>        {
>      @@ t/t0610-reftable-basics.sh: test_expect_success 'ref transaction: writes are syn
>        	EOF
>        '
>        
>      +@@ t/t0610-reftable-basics.sh: test_expect_success 'ref transaction: fails gracefully when auto compaction fail
>      + 			done ||
>      + 			exit 1
>      + 		done &&
>      +-		test_line_count = 13 .git/reftable/tables.list
>      ++		test_line_count = 10 .git/reftable/tables.list
>      + 	)
>      + '
>      + 
>       @@ t/t0610-reftable-basics.sh: test_expect_success 'pack-refs: compacts tables' '
>        
>        	test_commit -C repo A &&
>      @@ t/t0610-reftable-basics.sh: test_expect_success 'pack-refs: compacts tables' '
>        
>        	git -C repo pack-refs &&
>        	ls -1 repo/.git/reftable >table-files &&
>      +@@ t/t0610-reftable-basics.sh: test_expect_success "$command: auto compaction" '
>      + 		# The tables should have been auto-compacted, and thus auto
>      + 		# compaction should not have to do anything.
>      + 		ls -1 .git/reftable >tables-expect &&
>      +-		test_line_count = 4 tables-expect &&
>      ++		test_line_count = 3 tables-expect &&
>      + 		git $command --auto &&
>      + 		ls -1 .git/reftable >tables-actual &&
>      + 		test_cmp tables-expect tables-actual &&
>      +@@ t/t0610-reftable-basics.sh: test_expect_success "$command: auto compaction" '
>      + 		git branch B &&
>      + 		git branch C &&
>      + 		rm .git/reftable/*.lock &&
>      +-		test_line_count = 5 .git/reftable/tables.list &&
>      ++		test_line_count = 4 .git/reftable/tables.list &&
>      + 
>      + 		git $command --auto &&
>      + 		test_line_count = 1 .git/reftable/tables.list
>       @@ t/t0610-reftable-basics.sh: do
>        			umask $umask &&
>        			git init --shared=true repo &&
> 
> -- 
> gitgitgadget

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]