Re: [PATCH v2 00/11] reftable: expose write options as config

> Hi,
>
> this is the second version of my patch series that exposes various
> options of the reftable writer via Git configuration.
>
> Changes compared to v1:
>
>   - Drop unneeded return statements.
>
>   - Move default geometric factor into "constants.h".
>
>   - Fix a typo in a commit message.
>
> Thanks!
>
> Patrick
>
> Patrick Steinhardt (11):
>   reftable: consistently refer to `reftable_write_options` as `opts`
>   reftable: consistently pass write opts as value
>   reftable/writer: drop static variable used to initialize strbuf
>   reftable/writer: improve error when passed an invalid block size
>   reftable/dump: support dumping a table's block structure
>   refs/reftable: allow configuring block size
>   reftable: use `uint16_t` to track restart interval
>   refs/reftable: allow configuring restart interval
>   refs/reftable: allow disabling writing the object index
>   reftable: make the compaction factor configurable
>   refs/reftable: allow configuring geometric factor
>
>  Documentation/config.txt          |   2 +
>  Documentation/config/reftable.txt |  49 +++++
>  refs/reftable-backend.c           |  43 ++++-
>  reftable/block.h                  |   2 +-
>  reftable/constants.h              |   1 +
>  reftable/dump.c                   |  12 +-
>  reftable/merged_test.c            |   6 +-
>  reftable/reader.c                 |  63 +++++++
>  reftable/readwrite_test.c         |  26 +--
>  reftable/refname_test.c           |   2 +-
>  reftable/reftable-reader.h        |   2 +
>  reftable/reftable-stack.h         |   2 +-
>  reftable/reftable-writer.h        |  10 +-
>  reftable/stack.c                  |  57 +++---
>  reftable/stack.h                  |   5 +-
>  reftable/stack_test.c             | 118 ++++++------
>  reftable/writer.c                 |  20 +--
>  t/t0613-reftable-write-options.sh | 286 ++++++++++++++++++++++++++++++
>  18 files changed, 576 insertions(+), 130 deletions(-)
>  create mode 100644 Documentation/config/reftable.txt
>  create mode 100755 t/t0613-reftable-write-options.sh
>
> Range-diff against v1:
>  1:  47cee6e25e =  1:  7efa566306 reftable: consistently refer to `reftable_write_options` as `opts`
>  2:  d8a0764e87 =  2:  e6f8fc09c2 reftable: consistently pass write opts as value
>  3:  c040f81fba =  3:  aa2903e3e5 reftable/writer: drop static variable used to initialize strbuf
>  4:  ef79bb1b7b =  4:  5e7cbb7b19 reftable/writer: improve error when passed an invalid block size
>  5:  4d4407d4a4 =  5:  ed1c150d90 reftable/dump: support dumping a table's block structure
>  6:  b4e4db5735 !  6:  be5bdc6dc1 refs/reftable: allow configuring block size
>     @@ refs/reftable-backend.c: static int read_ref_without_reload(struct reftable_stac
>      +		if (block_size > 16777215)
>      +			die("reftable block size cannot exceed 16MB");
>      +		opts->block_size = block_size;
>     -+		return 0;
>      +	}
>      +
>      +	return 0;
>  7:  79d9e07ca9 =  7:  05e8d1df2d reftable: use `uint16_t` to track restart interval
>  8:  653ec4dfa5 !  8:  bc0bf65553 refs/reftable: allow configuring restart interval
>     @@ Documentation/config/reftable.txt: readers during access.
>
>       ## refs/reftable-backend.c ##
>      @@ refs/reftable-backend.c: static int reftable_be_config(const char *var, const char *value,
>     + 		if (block_size > 16777215)
>       			die("reftable block size cannot exceed 16MB");
>       		opts->block_size = block_size;
>     - 		return 0;
>      +	} else if (!strcmp(var, "reftable.restartinterval")) {
>      +		unsigned long restart_interval = git_config_ulong(var, value, ctx->kvi);
>      +		if (restart_interval > UINT16_MAX)
>      +			die("reftable block size cannot exceed %u", (unsigned)UINT16_MAX);
>      +		opts->restart_interval = restart_interval;
>     -+		return 0;
>       	}
>
>       	return 0;
>  9:  6f2c481acc !  9:  6bc240fd0c refs/reftable: allow disabling writing the object index
>     @@ Documentation/config/reftable.txt: A maximum of `65535` restart points per block
>
>       ## refs/reftable-backend.c ##
>      @@ refs/reftable-backend.c: static int reftable_be_config(const char *var, const char *value,
>     + 		if (restart_interval > UINT16_MAX)
>       			die("reftable block size cannot exceed %u", (unsigned)UINT16_MAX);
>       		opts->restart_interval = restart_interval;
>     - 		return 0;
>      +	} else if (!strcmp(var, "reftable.indexobjects")) {
>      +		opts->skip_index_objects = !git_config_bool(var, value);
>     -+		return 0;
>       	}
>
>       	return 0;
> 10:  30e2e33479 ! 10:  9d4c1f0340 reftable: make the compaction factor configurable
>     @@ Commit message
>
>          Signed-off-by: Patrick Steinhardt <ps@pks.im>
>
>     + ## reftable/constants.h ##
>     +@@ reftable/constants.h: license that can be found in the LICENSE file or at
>     +
>     + #define MAX_RESTARTS ((1 << 16) - 1)
>     + #define DEFAULT_BLOCK_SIZE 4096
>     ++#define DEFAULT_GEOMETRIC_FACTOR 2
>     +
>     + #endif
>     +
>       ## reftable/reftable-writer.h ##
>      @@ reftable/reftable-writer.h: struct reftable_write_options {
>
>     @@ reftable/reftable-writer.h: struct reftable_write_options {
>       /* reftable_block_stats holds statistics for a single block type */
>
>       ## reftable/stack.c ##
>     +@@ reftable/stack.c: license that can be found in the LICENSE file or at
>     +
>     + #include "../write-or-die.h"
>     + #include "system.h"
>     ++#include "constants.h"
>     + #include "merged.h"
>     + #include "reader.h"
>     + #include "refname.h"
>      @@ reftable/stack.c: static int segment_size(struct segment *s)
>       	return s->end - s->start;
>       }
>     @@ reftable/stack.c: static int segment_size(struct segment *s)
>       	size_t i;
>
>      +	if (!factor)
>     -+		factor = 2;
>     ++		factor = DEFAULT_GEOMETRIC_FACTOR;
>      +
>       	/*
>       	 * If there are no tables or only a single one then we don't have to
> 11:  861f2e72d9 ! 11:  e1282e53fb refs/reftable: allow configuring geometric factor
>     @@ Documentation/config/reftable.txt: reftable.indexObjects::
>       The default value is `true`.
>      +
>      +reftable.geometricFactor::
>     -+	Whenever the reftable backend appends a new table to the table it
>     ++	Whenever the reftable backend appends a new table to the stack, it
>      +	performs auto compaction to ensure that there is only a handful of
>      +	tables. The backend does this by ensuring that tables form a geometric
>      +	sequence regarding the respective sizes of each table.
>     @@ Documentation/config/reftable.txt: reftable.indexObjects::
>
>       ## refs/reftable-backend.c ##
>      @@ refs/reftable-backend.c: static int reftable_be_config(const char *var, const char *value,
>     + 		opts->restart_interval = restart_interval;
>       	} else if (!strcmp(var, "reftable.indexobjects")) {
>       		opts->skip_index_objects = !git_config_bool(var, value);
>     - 		return 0;
>      +	} else if (!strcmp(var, "reftable.geometricfactor")) {
>      +		unsigned long factor = git_config_ulong(var, value, ctx->kvi);
>      +		if (factor > UINT8_MAX)
>
> base-commit: d4cc1ec35f3bcce816b69986ca41943f6ce21377
> --
> 2.45.0