Merge branch 'ps/reftable-write-options'

The knobs to tweak how reftable files are written have been made
available as configuration variables.

* ps/reftable-write-options:
  refs/reftable: allow configuring geometric factor
  reftable: make the compaction factor configurable
  refs/reftable: allow disabling writing the object index
  refs/reftable: allow configuring restart interval
  reftable: use `uint16_t` to track restart interval
  refs/reftable: allow configuring block size
  reftable/dump: support dumping a table's block structure
  reftable/writer: improve error when passed an invalid block size
  reftable/writer: drop static variable used to initialize strbuf
  reftable: pass opts as constant pointer
  reftable: consistently refer to `reftable_write_options` as `opts`

11 files changed, 185 insertions, 111 deletions

diff --git a/reftable/block.h b/reftable/block.h
index e91f3d2790..1c8f25ee6e 100644
--- a/reftable/block.h
+++ b/reftable/block.h
@@ -29,7 +29,7 @@ struct block_writer {
 	uint32_t header_off;
 
 	/* How often to restart keys. */
-	int restart_interval;
+	uint16_t restart_interval;
 	int hash_size;
 
 	/* Offset of next uint8_t to write. */
diff --git a/reftable/constants.h b/reftable/constants.h
index 5eee72c4c1..f6beb843eb 100644
--- a/reftable/constants.h
+++ b/reftable/constants.h
@@ -17,5 +17,6 @@ https://developers.google.com/open-source/licenses/bsd
 
 #define MAX_RESTARTS ((1 << 16) - 1)
 #define DEFAULT_BLOCK_SIZE 4096
+#define DEFAULT_GEOMETRIC_FACTOR 2
 
 #endif
diff --git a/reftable/dump.c b/reftable/dump.c
index 26e0393c7d..41abbb8ecf 100644
--- a/reftable/dump.c
+++ b/reftable/dump.c
@@ -27,9 +27,9 @@ https://developers.google.com/open-source/licenses/bsd
 static int compact_stack(const char *stackdir)
 {
 	struct reftable_stack *stack = NULL;
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 
-	int err = reftable_new_stack(&stack, stackdir, cfg);
+	int err = reftable_new_stack(&stack, stackdir, &opts);
 	if (err < 0)
 		goto done;
 
@@ -48,6 +48,7 @@ static void print_help(void)
 	printf("usage: dump [-cst] arg\n\n"
 	       "options: \n"
 	       "  -c compact\n"
+	       "  -b dump blocks\n"
 	       "  -t dump table\n"
 	       "  -s dump stack\n"
 	       "  -6 sha256 hash format\n"
@@ -58,6 +59,7 @@ static void print_help(void)
 int reftable_dump_main(int argc, char *const *argv)
 {
 	int err = 0;
+	int opt_dump_blocks = 0;
 	int opt_dump_table = 0;
 	int opt_dump_stack = 0;
 	int opt_compact = 0;
@@ -67,6 +69,8 @@ int reftable_dump_main(int argc, char *const *argv)
 	for (; argc > 1; argv++, argc--)
 		if (*argv[1] != '-')
 			break;
+		else if (!strcmp("-b", argv[1]))
+			opt_dump_blocks = 1;
 		else if (!strcmp("-t", argv[1]))
 			opt_dump_table = 1;
 		else if (!strcmp("-6", argv[1]))
@@ -88,7 +92,9 @@ int reftable_dump_main(int argc, char *const *argv)
 
 	arg = argv[1];
 
-	if (opt_dump_table) {
+	if (opt_dump_blocks) {
+		err = reftable_reader_print_blocks(arg);
+	} else if (opt_dump_table) {
 		err = reftable_reader_print_file(arg);
 	} else if (opt_dump_stack) {
 		err = reftable_stack_print_directory(arg, opt_hash_id);
diff --git a/reftable/reader.c b/reftable/reader.c
index 481dff10d4..f23c8523db 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -856,3 +856,66 @@ done:
 	reftable_reader_free(r);
 	return err;
 }
+
+int reftable_reader_print_blocks(const char *tablename)
+{
+	struct {
+		const char *name;
+		int type;
+	} sections[] = {
+		{
+			.name = "ref",
+			.type = BLOCK_TYPE_REF,
+		},
+		{
+			.name = "obj",
+			.type = BLOCK_TYPE_OBJ,
+		},
+		{
+			.name = "log",
+			.type = BLOCK_TYPE_LOG,
+		},
+	};
+	struct reftable_block_source src = { 0 };
+	struct table_iter ti = TABLE_ITER_INIT;
+	struct reftable_reader *r = NULL;
+	size_t i;
+	int err;
+
+	err = reftable_block_source_from_file(&src, tablename);
+	if (err < 0)
+		goto done;
+
+	err = reftable_new_reader(&r, &src, tablename);
+	if (err < 0)
+		goto done;
+
+	printf("header:\n");
+	printf("  block_size: %d\n", r->block_size);
+
+	for (i = 0; i < ARRAY_SIZE(sections); i++) {
+		err = reader_start(r, &ti, sections[i].type, 0);
+		if (err < 0)
+			goto done;
+		if (err > 0)
+			continue;
+
+		printf("%s:\n", sections[i].name);
+
+		while (1) {
+			printf("  - length: %u\n", ti.br.block_len);
+			printf("    restarts: %u\n", ti.br.restart_count);
+
+			err = table_iter_next_block(&ti);
+			if (err < 0)
+				goto done;
+			if (err > 0)
+				break;
+		}
+	}
+
+done:
+	reftable_reader_free(r);
+	table_iter_close(&ti);
+	return err;
+}
diff --git a/reftable/reftable-reader.h b/reftable/reftable-reader.h
index 4a4bc2fdf8..4a04857773 100644
--- a/reftable/reftable-reader.h
+++ b/reftable/reftable-reader.h
@@ -97,5 +97,7 @@ void reftable_table_from_reader(struct reftable_table *tab,
 
 /* print table onto stdout for debugging. */
 int reftable_reader_print_file(const char *tablename);
+/* print blocks onto stdout for debugging. */
+int reftable_reader_print_blocks(const char *tablename);
 
 #endif
diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index 1b602dda58..c15632c401 100644
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -29,7 +29,7 @@ struct reftable_stack;
  *  stored in 'dir'. Typically, this should be .git/reftables.
  */
 int reftable_new_stack(struct reftable_stack **dest, const char *dir,
-		       struct reftable_write_options config);
+		       const struct reftable_write_options *opts);
 
 /* returns the update_index at which a next table should be written. */
 uint64_t reftable_stack_next_update_index(struct reftable_stack *st);
diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h
index b601a69a40..189b1f4144 100644
--- a/reftable/reftable-writer.h
+++ b/reftable/reftable-writer.h
@@ -28,7 +28,7 @@ struct reftable_write_options {
 	unsigned skip_index_objects : 1;
 
 	/* how often to write complete keys in each block. */
-	int restart_interval;
+	uint16_t restart_interval;
 
 	/* 4-byte identifier ("sha1", "s256") of the hash.
 	 * Defaults to SHA1 if unset
@@ -45,6 +45,12 @@ struct reftable_write_options {
 
 	/* boolean: Prevent auto-compaction of tables. */
 	unsigned disable_auto_compact : 1;
+
+	/*
+	 * Geometric sequence factor used by auto-compaction to decide which
+	 * tables to compact. Defaults to 2 if unset.
+	 */
+	uint8_t auto_compaction_factor;
 };
 
 /* reftable_block_stats holds statistics for a single block type */
@@ -88,7 +94,7 @@ struct reftable_stats {
 struct reftable_writer *
 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
 		    int (*flush_func)(void *),
-		    void *writer_arg, struct reftable_write_options *opts);
+		    void *writer_arg, const struct reftable_write_options *opts);
 
 /* Set the range of update indices for the records we will add. When writing a
    table into a stack, the min should be at least
diff --git a/reftable/stack.c b/reftable/stack.c
index a59ebe038d..0ebe69e81d 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -10,6 +10,7 @@ https://developers.google.com/open-source/licenses/bsd
 
 #include "../write-or-die.h"
 #include "system.h"
+#include "constants.h"
 #include "merged.h"
 #include "reader.h"
 #include "reftable-error.h"
@@ -54,15 +55,17 @@ static int reftable_fd_flush(void *arg)
 }
 
 int reftable_new_stack(struct reftable_stack **dest, const char *dir,
-		       struct reftable_write_options config)
+		       const struct reftable_write_options *_opts)
 {
 	struct reftable_stack *p = reftable_calloc(1, sizeof(*p));
 	struct strbuf list_file_name = STRBUF_INIT;
+	struct reftable_write_options opts = {0};
 	int err = 0;
 
-	if (config.hash_id == 0) {
-		config.hash_id = GIT_SHA1_FORMAT_ID;
-	}
+	if (_opts)
+		opts = *_opts;
+	if (opts.hash_id == 0)
+		opts.hash_id = GIT_SHA1_FORMAT_ID;
 
 	*dest = NULL;
 
@@ -73,7 +76,7 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
 	p->list_file = strbuf_detach(&list_file_name, NULL);
 	p->list_fd = -1;
 	p->reftable_dir = xstrdup(dir);
-	p->config = config;
+	p->opts = opts;
 
 	err = reftable_stack_reload_maybe_reuse(p, 1);
 	if (err < 0) {
@@ -255,7 +258,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 
 	/* success! */
 	err = reftable_new_merged_table(&new_merged, new_tables,
-					new_readers_len, st->config.hash_id);
+					new_readers_len, st->opts.hash_id);
 	if (err < 0)
 		goto done;
 
@@ -578,8 +581,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
 		}
 		goto done;
 	}
-	if (st->config.default_permissions) {
-		if (chmod(add->lock_file->filename.buf, st->config.default_permissions) < 0) {
+	if (st->opts.default_permissions) {
+		if (chmod(add->lock_file->filename.buf, st->opts.default_permissions) < 0) {
 			err = REFTABLE_IO_ERROR;
 			goto done;
 		}
@@ -678,7 +681,7 @@ int reftable_addition_commit(struct reftable_addition *add)
 	if (err)
 		goto done;
 
-	if (!add->stack->config.disable_auto_compact) {
+	if (!add->stack->opts.disable_auto_compact) {
 		/*
 		 * Auto-compact the stack to keep the number of tables in
 		 * control. It is possible that a concurrent writer is already
@@ -756,9 +759,9 @@ int reftable_addition_add(struct reftable_addition *add,
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
-	if (add->stack->config.default_permissions) {
+	if (add->stack->opts.default_permissions) {
 		if (chmod(get_tempfile_path(tab_file),
-			  add->stack->config.default_permissions)) {
+			  add->stack->opts.default_permissions)) {
 			err = REFTABLE_IO_ERROR;
 			goto done;
 		}
@@ -766,7 +769,7 @@ int reftable_addition_add(struct reftable_addition *add,
 	tab_fd = get_tempfile_fd(tab_file);
 
 	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd,
-				 &add->stack->config);
+				 &add->stack->opts);
 	err = write_table(wr, arg);
 	if (err < 0)
 		goto done;
@@ -849,14 +852,14 @@ static int stack_compact_locked(struct reftable_stack *st,
 	}
 	tab_fd = get_tempfile_fd(tab_file);
 
-	if (st->config.default_permissions &&
-	    chmod(get_tempfile_path(tab_file), st->config.default_permissions) < 0) {
+	if (st->opts.default_permissions &&
+	    chmod(get_tempfile_path(tab_file), st->opts.default_permissions) < 0) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
 
 	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush,
-				 &tab_fd, &st->config);
+				 &tab_fd, &st->opts);
 	err = stack_write_compact(st, wr, first, last, config);
 	if (err < 0)
 		goto done;
@@ -904,7 +907,7 @@ static int stack_write_compact(struct reftable_stack *st,
 				   st->readers[last]->max_update_index);
 
 	err = reftable_new_merged_table(&mt, subtabs, subtabs_len,
-					st->config.hash_id);
+					st->opts.hash_id);
 	if (err < 0) {
 		reftable_free(subtabs);
 		goto done;
@@ -1094,9 +1097,9 @@ static int stack_compact_range(struct reftable_stack *st,
 		goto done;
 	}
 
-	if (st->config.default_permissions) {
+	if (st->opts.default_permissions) {
 		if (chmod(get_lock_file_path(&tables_list_lock),
-			  st->config.default_permissions) < 0) {
+			  st->opts.default_permissions) < 0) {
 			err = REFTABLE_IO_ERROR;
 			goto done;
 		}
@@ -1210,12 +1213,16 @@ static int segment_size(struct segment *s)
 	return s->end - s->start;
 }
 
-struct segment suggest_compaction_segment(uint64_t *sizes, size_t n)
+struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
+					  uint8_t factor)
 {
 	struct segment seg = { 0 };
 	uint64_t bytes;
 	size_t i;
 
+	if (!factor)
+		factor = DEFAULT_GEOMETRIC_FACTOR;
+
 	/*
 	 * If there are no tables or only a single one then we don't have to
 	 * compact anything. The sequence is geometric by definition already.
@@ -1247,7 +1254,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n)
 	 * 	64, 32, 16, 8, 4, 3, 1
 	 */
 	for (i = n - 1; i > 0; i--) {
-		if (sizes[i - 1] < sizes[i] * 2) {
+		if (sizes[i - 1] < sizes[i] * factor) {
 			seg.end = i + 1;
 			bytes = sizes[i];
 			break;
@@ -1273,7 +1280,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n)
 		uint64_t curr = bytes;
 		bytes += sizes[i - 1];
 
-		if (sizes[i - 1] < curr * 2) {
+		if (sizes[i - 1] < curr * factor) {
 			seg.start = i - 1;
 			seg.bytes = bytes;
 		}
@@ -1286,7 +1293,7 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
 {
 	uint64_t *sizes =
 		reftable_calloc(st->merged->stack_len, sizeof(*sizes));
-	int version = (st->config.hash_id == GIT_SHA1_FORMAT_ID) ? 1 : 2;
+	int version = (st->opts.hash_id == GIT_SHA1_FORMAT_ID) ? 1 : 2;
 	int overhead = header_size(version) - 1;
 	int i = 0;
 	for (i = 0; i < st->merged->stack_len; i++) {
@@ -1299,7 +1306,8 @@ int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	uint64_t *sizes = stack_table_sizes_for_compaction(st);
 	struct segment seg =
-		suggest_compaction_segment(sizes, st->merged->stack_len);
+		suggest_compaction_segment(sizes, st->merged->stack_len,
+					   st->opts.auto_compaction_factor);
 	reftable_free(sizes);
 	if (segment_size(&seg) > 0)
 		return stack_compact_range_stats(st, seg.start, seg.end - 1,
@@ -1435,11 +1443,11 @@ done:
 int reftable_stack_print_directory(const char *stackdir, uint32_t hash_id)
 {
 	struct reftable_stack *stack = NULL;
-	struct reftable_write_options cfg = { .hash_id = hash_id };
+	struct reftable_write_options opts = { .hash_id = hash_id };
 	struct reftable_merged_table *merged = NULL;
 	struct reftable_table table = { NULL };
 
-	int err = reftable_new_stack(&stack, stackdir, cfg);
+	int err = reftable_new_stack(&stack, stackdir, &opts);
 	if (err < 0)
 		goto done;
 
diff --git a/reftable/stack.h b/reftable/stack.h
index d43efa4760..5b45cff4f7 100644
--- a/reftable/stack.h
+++ b/reftable/stack.h
@@ -20,7 +20,7 @@ struct reftable_stack {
 
 	char *reftable_dir;
 
-	struct reftable_write_options config;
+	struct reftable_write_options opts;
 
 	struct reftable_reader **readers;
 	size_t readers_len;
@@ -35,6 +35,7 @@ struct segment {
 	uint64_t bytes;
 };
 
-struct segment suggest_compaction_segment(uint64_t *sizes, size_t n);
+struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
+					  uint8_t factor);
 
 #endif
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 7889f818d1..0f7b1453e6 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -150,7 +150,7 @@ static void test_reftable_stack_add_one(void)
 	char *dir = get_tmp_dir(__LINE__);
 	struct strbuf scratch = STRBUF_INIT;
 	int mask = umask(002);
-	struct reftable_write_options cfg = {
+	struct reftable_write_options opts = {
 		.default_permissions = 0660,
 	};
 	struct reftable_stack *st = NULL;
@@ -163,7 +163,7 @@ static void test_reftable_stack_add_one(void)
 	};
 	struct reftable_ref_record dest = { NULL };
 	struct stat stat_result = { 0 };
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st, &write_test_ref, &ref);
@@ -186,7 +186,7 @@ static void test_reftable_stack_add_one(void)
 	strbuf_addstr(&scratch, "/tables.list");
 	err = stat(scratch.buf, &stat_result);
 	EXPECT(!err);
-	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+	EXPECT((stat_result.st_mode & 0777) == opts.default_permissions);
 
 	strbuf_reset(&scratch);
 	strbuf_addstr(&scratch, dir);
@@ -195,7 +195,7 @@ static void test_reftable_stack_add_one(void)
 	strbuf_addstr(&scratch, st->readers[0]->name);
 	err = stat(scratch.buf, &stat_result);
 	EXPECT(!err);
-	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+	EXPECT((stat_result.st_mode & 0777) == opts.default_permissions);
 #else
 	(void) stat_result;
 #endif
@@ -209,7 +209,7 @@ static void test_reftable_stack_add_one(void)
 
 static void test_reftable_stack_uptodate(void)
 {
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st1 = NULL;
 	struct reftable_stack *st2 = NULL;
 	char *dir = get_tmp_dir(__LINE__);
@@ -232,10 +232,10 @@ static void test_reftable_stack_uptodate(void)
 	/* simulate multi-process access to the same stack
 	   by creating two stacks for the same directory.
 	 */
-	err = reftable_new_stack(&st1, dir, cfg);
+	err = reftable_new_stack(&st1, dir, &opts);
 	EXPECT_ERR(err);
 
-	err = reftable_new_stack(&st2, dir, cfg);
+	err = reftable_new_stack(&st2, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st1, &write_test_ref, &ref1);
@@ -257,8 +257,7 @@ static void test_reftable_stack_uptodate(void)
 static void test_reftable_stack_transaction_api(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_addition *add = NULL;
@@ -271,8 +270,7 @@ static void test_reftable_stack_transaction_api(void)
 	};
 	struct reftable_ref_record dest = { NULL };
 
-
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	reftable_addition_destroy(add);
@@ -301,12 +299,12 @@ static void test_reftable_stack_transaction_api(void)
 static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-	struct reftable_write_options cfg = {0};
+	struct reftable_write_options opts = {0};
 	struct reftable_addition *add = NULL;
 	struct reftable_stack *st = NULL;
 	int i, n = 20, err;
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i <= n; i++) {
@@ -325,7 +323,7 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
 		 * we can ensure that we indeed honor this setting and have
 		 * better control over when exactly auto compaction runs.
 		 */
-		st->config.disable_auto_compact = i != n;
+		st->opts.disable_auto_compact = i != n;
 
 		err = reftable_stack_new_addition(&add, st);
 		EXPECT_ERR(err);
@@ -361,13 +359,13 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void)
 		.value_type = REFTABLE_REF_VAL1,
 		.value.val1 = {0x01},
 	};
-	struct reftable_write_options cfg = {0};
+	struct reftable_write_options opts = {0};
 	struct reftable_stack *st;
 	struct strbuf table_path = STRBUF_INIT;
 	char *dir = get_tmp_dir(__LINE__);
 	int err;
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st, write_test_ref, &ref);
@@ -404,8 +402,7 @@ static int write_error(struct reftable_writer *wr, void *arg)
 static void test_reftable_stack_update_index_check(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_ref_record ref1 = {
@@ -421,7 +418,7 @@ static void test_reftable_stack_update_index_check(void)
 		.value.symref = "master",
 	};
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st, &write_test_ref, &ref1);
@@ -436,12 +433,11 @@ static void test_reftable_stack_update_index_check(void)
 static void test_reftable_stack_lock_failure(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err, i;
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 	for (i = -1; i != REFTABLE_EMPTY_TABLE_ERROR; i--) {
 		err = reftable_stack_add(st, &write_error, &i);
@@ -456,7 +452,7 @@ static void test_reftable_stack_add(void)
 {
 	int i = 0;
 	int err = 0;
-	struct reftable_write_options cfg = {
+	struct reftable_write_options opts = {
 		.exact_log_message = 1,
 		.default_permissions = 0660,
 		.disable_auto_compact = 1,
@@ -469,7 +465,7 @@ static void test_reftable_stack_add(void)
 	struct stat stat_result;
 	int N = ARRAY_SIZE(refs);
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i < N; i++) {
@@ -528,7 +524,7 @@ static void test_reftable_stack_add(void)
 	strbuf_addstr(&path, "/tables.list");
 	err = stat(path.buf, &stat_result);
 	EXPECT(!err);
-	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+	EXPECT((stat_result.st_mode & 0777) == opts.default_permissions);
 
 	strbuf_reset(&path);
 	strbuf_addstr(&path, dir);
@@ -537,7 +533,7 @@ static void test_reftable_stack_add(void)
 	strbuf_addstr(&path, st->readers[0]->name);
 	err = stat(path.buf, &stat_result);
 	EXPECT(!err);
-	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+	EXPECT((stat_result.st_mode & 0777) == opts.default_permissions);
 #else
 	(void) stat_result;
 #endif
@@ -555,7 +551,7 @@ static void test_reftable_stack_add(void)
 static void test_reftable_stack_log_normalize(void)
 {
 	int err = 0;
-	struct reftable_write_options cfg = {
+	struct reftable_write_options opts = {
 		0,
 	};
 	struct reftable_stack *st = NULL;
@@ -579,7 +575,7 @@ static void test_reftable_stack_log_normalize(void)
 		.update_index = 1,
 	};
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	input.value.update.message = "one\ntwo";
@@ -612,8 +608,7 @@ static void test_reftable_stack_tombstone(void)
 {
 	int i = 0;
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_ref_record refs[2] = { { NULL } };
@@ -622,8 +617,7 @@ static void test_reftable_stack_tombstone(void)
 	struct reftable_ref_record dest = { NULL };
 	struct reftable_log_record log_dest = { NULL };
 
-
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	/* even entries add the refs, odd entries delete them. */
@@ -691,8 +685,7 @@ static void test_reftable_stack_tombstone(void)
 static void test_reftable_stack_hash_id(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err;
 
@@ -702,24 +695,24 @@ static void test_reftable_stack_hash_id(void)
 		.value.symref = "target",
 		.update_index = 1,
 	};
-	struct reftable_write_options cfg32 = { .hash_id = GIT_SHA256_FORMAT_ID };
+	struct reftable_write_options opts32 = { .hash_id = GIT_SHA256_FORMAT_ID };
 	struct reftable_stack *st32 = NULL;
-	struct reftable_write_options cfg_default = { 0 };
+	struct reftable_write_options opts_default = { 0 };
 	struct reftable_stack *st_default = NULL;
 	struct reftable_ref_record dest = { NULL };
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st, &write_test_ref, &ref);
 	EXPECT_ERR(err);
 
 	/* can't read it with the wrong hash ID. */
-	err = reftable_new_stack(&st32, dir, cfg32);
+	err = reftable_new_stack(&st32, dir, &opts32);
 	EXPECT(err == REFTABLE_FORMAT_ERROR);
 
-	/* check that we can read it back with default config too. */
-	err = reftable_new_stack(&st_default, dir, cfg_default);
+	/* check that we can read it back with default opts too. */
+	err = reftable_new_stack(&st_default, dir, &opts_default);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_read_ref(st_default, "master", &dest);
@@ -736,7 +729,7 @@ static void test_suggest_compaction_segment(void)
 {
 	uint64_t sizes[] = { 512, 64, 17, 16, 9, 9, 9, 16, 2, 16 };
 	struct segment min =
-		suggest_compaction_segment(sizes, ARRAY_SIZE(sizes));
+		suggest_compaction_segment(sizes, ARRAY_SIZE(sizes), 2);
 	EXPECT(min.start == 1);
 	EXPECT(min.end == 10);
 }
@@ -745,15 +738,14 @@ static void test_suggest_compaction_segment_nothing(void)
 {
 	uint64_t sizes[] = { 64, 32, 16, 8, 4, 2 };
 	struct segment result =
-		suggest_compaction_segment(sizes, ARRAY_SIZE(sizes));
+		suggest_compaction_segment(sizes, ARRAY_SIZE(sizes), 2);
 	EXPECT(result.start == result.end);
 }
 
 static void test_reflog_expire(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	struct reftable_log_record logs[20] = { { NULL } };
 	int N = ARRAY_SIZE(logs) - 1;
@@ -764,8 +756,7 @@ static void test_reflog_expire(void)
 	};
 	struct reftable_log_record log = { NULL };
 
-
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 1; i <= N; i++) {
@@ -828,21 +819,19 @@ static int write_nothing(struct reftable_writer *wr, void *arg)
 
 static void test_empty_add(void)
 {
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	int err;
 	char *dir = get_tmp_dir(__LINE__);
-
 	struct reftable_stack *st2 = NULL;
 
-
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_add(st, &write_nothing, NULL);
 	EXPECT_ERR(err);
 
-	err = reftable_new_stack(&st2, dir, cfg);
+	err = reftable_new_stack(&st2, dir, &opts);
 	EXPECT_ERR(err);
 	clear_dir(dir);
 	reftable_stack_destroy(st);
@@ -861,16 +850,15 @@ static int fastlog2(uint64_t sz)
 
 static void test_reftable_stack_auto_compaction(void)
 {
-	struct reftable_write_options cfg = {
+	struct reftable_write_options opts = {
 		.disable_auto_compact = 1,
 	};
 	struct reftable_stack *st = NULL;
 	char *dir = get_tmp_dir(__LINE__);
-
 	int err, i;
 	int N = 100;
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i < N; i++) {
@@ -900,13 +888,13 @@ static void test_reftable_stack_auto_compaction(void)
 
 static void test_reftable_stack_add_performs_auto_compaction(void)
 {
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
 	struct strbuf refname = STRBUF_INIT;
 	char *dir = get_tmp_dir(__LINE__);
 	int err, i, n = 20;
 
-	err = reftable_new_stack(&st, dir, cfg);
+	err = reftable_new_stack(&st, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i <= n; i++) {
@@ -921,7 +909,7 @@ static void test_reftable_stack_add_performs_auto_compaction(void)
 		 * we can ensure that we indeed honor this setting and have
 		 * better control over when exactly auto compaction runs.
 		 */
-		st->config.disable_auto_compact = i != n;
+		st->opts.disable_auto_compact = i != n;
 
 		strbuf_reset(&refname);
 		strbuf_addf(&refname, "branch-%04d", i);
@@ -948,14 +936,13 @@ static void test_reftable_stack_add_performs_auto_compaction(void)
 
 static void test_reftable_stack_compaction_concurrent(void)
 {
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st1 = NULL, *st2 = NULL;
 	char *dir = get_tmp_dir(__LINE__);
-
 	int err, i;
 	int N = 3;
 
-	err = reftable_new_stack(&st1, dir, cfg);
+	err = reftable_new_stack(&st1, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i < N; i++) {
@@ -972,7 +959,7 @@ static void test_reftable_stack_compaction_concurrent(void)
 		EXPECT_ERR(err);
 	}
 
-	err = reftable_new_stack(&st2, dir, cfg);
+	err = reftable_new_stack(&st2, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_compact_all(st1, NULL);
@@ -998,14 +985,13 @@ static void unclean_stack_close(struct reftable_stack *st)
 
 static void test_reftable_stack_compaction_concurrent_clean(void)
 {
-	struct reftable_write_options cfg = { 0 };
+	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st1 = NULL, *st2 = NULL, *st3 = NULL;
 	char *dir = get_tmp_dir(__LINE__);
-
 	int err, i;
 	int N = 3;
 
-	err = reftable_new_stack(&st1, dir, cfg);
+	err = reftable_new_stack(&st1, dir, &opts);
 	EXPECT_ERR(err);
 
 	for (i = 0; i < N; i++) {
@@ -1022,7 +1008,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 		EXPECT_ERR(err);
 	}
 
-	err = reftable_new_stack(&st2, dir, cfg);
+	err = reftable_new_stack(&st2, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_compact_all(st1, NULL);
@@ -1031,7 +1017,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 	unclean_stack_close(st1);
 	unclean_stack_close(st2);
 
-	err = reftable_new_stack(&st3, dir, cfg);
+	err = reftable_new_stack(&st3, dir, &opts);
 	EXPECT_ERR(err);
 
 	err = reftable_stack_clean(st3);
diff --git a/reftable/writer.c b/reftable/writer.c
index 10eccaaa07..45b3e9ce1f 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -117,25 +117,26 @@ static void writer_reinit_block_writer(struct reftable_writer *w, uint8_t typ)
 	w->block_writer->restart_interval = w->opts.restart_interval;
 }
 
-static struct strbuf reftable_empty_strbuf = STRBUF_INIT;
-
 struct reftable_writer *
 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
 		    int (*flush_func)(void *),
-		    void *writer_arg, struct reftable_write_options *opts)
+		    void *writer_arg, const struct reftable_write_options *_opts)
 {
 	struct reftable_writer *wp = reftable_calloc(1, sizeof(*wp));
+	struct reftable_write_options opts = {0};
+
+	if (_opts)
+		opts = *_opts;
+	options_set_defaults(&opts);
+	if (opts.block_size >= (1 << 24))
+		BUG("configured block size exceeds 16MB");
+
 	strbuf_init(&wp->block_writer_data.last_key, 0);
-	options_set_defaults(opts);
-	if (opts->block_size >= (1 << 24)) {
-		/* TODO - error return? */
-		abort();
-	}
-	wp->last_key = reftable_empty_strbuf;
-	REFTABLE_CALLOC_ARRAY(wp->block, opts->block_size);
+	strbuf_init(&wp->last_key, 0);
+	REFTABLE_CALLOC_ARRAY(wp->block, opts.block_size);
 	wp->write = writer_func;
 	wp->write_arg = writer_arg;
-	wp->opts = *opts;
+	wp->opts = opts;
 	wp->flush = flush_func;
 	writer_reinit_block_writer(wp, BLOCK_TYPE_REF);