Patrick Steinhardt <ps@xxxxxx> writes:
> 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@xxxxxx>
>
> + ## 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
The range diff looks good to me, thanks for the quick iteration :)
Attachment:
signature.asc
Description: PGP signature
