diff mbox series

[v5,2/3] reftable/stack: add env to disable autocompaction

Message ID 7c4fe0e9ec597203ee37d2c2503be319e87ff5ee.1712255369.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series reftable/stack: use geometric table compaction | expand

Commit Message

Justin Tobler April 4, 2024, 6:29 p.m. UTC
From: Justin Tobler <jltobler@gmail.com>

In future tests it will be neccesary to create repositories with a set
number of tables. To make this easier, introduce the
`GIT_TEST_REFTABLE_AUTOCOMPACTION` environment variable that, when set
to false, disables autocompaction of reftables.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 refs/reftable-backend.c    |  4 ++++
 t/t0610-reftable-basics.sh | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

Comments

Patrick Steinhardt April 8, 2024, 6:12 a.m. UTC | #1
On Thu, Apr 04, 2024 at 06:29:28PM +0000, Justin Tobler via GitGitGadget wrote:
> From: Justin Tobler <jltobler@gmail.com>
> 
> In future tests it will be neccesary to create repositories with a set
> number of tables. To make this easier, introduce the
> `GIT_TEST_REFTABLE_AUTOCOMPACTION` environment variable that, when set
> to false, disables autocompaction of reftables.
> 
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  refs/reftable-backend.c    |  4 ++++
>  t/t0610-reftable-basics.sh | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 0bed6d2ab48..6b6191f89dd 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -18,6 +18,7 @@
>  #include "../reftable/reftable-merged.h"
>  #include "../setup.h"
>  #include "../strmap.h"
> +#include "parse.h"
>  #include "refs-internal.h"
>  
>  /*
> @@ -248,6 +249,9 @@ 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;
> +

This could be simplified to:

    ```
    refs->write_options.disable_auto_compact =
            !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
    ```

Patrick

>  	/*
>  	 * Set up the main reftable stack that is hosted in GIT_COMMON_DIR.
>  	 * This stack contains both the shared and the main worktree refs.
> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> index 931d888bbbc..c9e10b34684 100755
> --- a/t/t0610-reftable-basics.sh
> +++ b/t/t0610-reftable-basics.sh
> @@ -299,6 +299,27 @@ test_expect_success 'ref transaction: writes cause auto-compaction' '
>  	test_line_count = 1 repo/.git/reftable/tables.list
>  '
>  
> +test_expect_success 'ref transaction: env var disables compaction' '
> +	test_when_finished "rm -rf repo" &&
> +
> +	git init repo &&
> +	test_commit -C repo A &&
> +
> +	start=$(wc -l <repo/.git/reftable/tables.list) &&
> +	iterations=5 &&
> +	expected=$((start + iterations)) &&
> +
> +	for i in $(test_seq $iterations)
> +	do
> +		GIT_TEST_REFTABLE_AUTOCOMPACTION=false \
> +		git -C repo update-ref branch-$i HEAD || return 1
> +	done &&
> +	test_line_count = $expected repo/.git/reftable/tables.list &&
> +
> +	git -C repo update-ref foo HEAD &&
> +	test_line_count -lt $expected repo/.git/reftable/tables.list
> +'
> +
>  check_fsync_events () {
>  	local trace="$1" &&
>  	shift &&
> -- 
> gitgitgadget
>
Junio C Hamano April 8, 2024, 4:18 p.m. UTC | #2
Patrick Steinhardt <ps@pks.im> writes:

>> @@ -248,6 +249,9 @@ 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;
>> +
>
> This could be simplified to:
>
>     ```
>     refs->write_options.disable_auto_compact =
>             !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
>     ```

I presume that the .disable_auto_compact member is off initially,
given that this is inside reftable_be_init(), but your rewrite makes
it easier on readers, as they do not have to know what value the
member originally has at this point in the flow.  So even though it
replaces two lines of code with another two lines of code, it does
count as a valuable simplification at the conceptual level.

Thanks.
diff mbox series

Patch

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 0bed6d2ab48..6b6191f89dd 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -18,6 +18,7 @@ 
 #include "../reftable/reftable-merged.h"
 #include "../setup.h"
 #include "../strmap.h"
+#include "parse.h"
 #include "refs-internal.h"
 
 /*
@@ -248,6 +249,9 @@  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.
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 931d888bbbc..c9e10b34684 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -299,6 +299,27 @@  test_expect_success 'ref transaction: writes cause auto-compaction' '
 	test_line_count = 1 repo/.git/reftable/tables.list
 '
 
+test_expect_success 'ref transaction: env var disables compaction' '
+	test_when_finished "rm -rf repo" &&
+
+	git init repo &&
+	test_commit -C repo A &&
+
+	start=$(wc -l <repo/.git/reftable/tables.list) &&
+	iterations=5 &&
+	expected=$((start + iterations)) &&
+
+	for i in $(test_seq $iterations)
+	do
+		GIT_TEST_REFTABLE_AUTOCOMPACTION=false \
+		git -C repo update-ref branch-$i HEAD || return 1
+	done &&
+	test_line_count = $expected repo/.git/reftable/tables.list &&
+
+	git -C repo update-ref foo HEAD &&
+	test_line_count -lt $expected repo/.git/reftable/tables.list
+'
+
 check_fsync_events () {
 	local trace="$1" &&
 	shift &&