aboutsummaryrefslogtreecommitdiff
path: root/reftable
AgeCommit message (Collapse)Author
2024-11-21reftable/writer: optimize allocations by using a scratch bufferPatrick Steinhardt
Both `writer_add_record()` and `reftable_writer_add_ref()` get executed for every single ref record we're adding to the reftable writer. And as both functions use a local buffer to write data, the allocations we have to do here add up during larger transactions. Refactor the code to use a scratch buffer part of the `reftable_writer` itself such that we can reuse it. This signifcantly reduces the number of allocations during large transactions, e.g. when migrating refs from the "files" backend to the "reftable" backend. Before this change: HEAP SUMMARY: in use at exit: 80,048 bytes in 49 blocks total heap usage: 5,032,171 allocs, 5,032,122 frees, 418,792,092 bytes allocated After this change: HEAP SUMMARY: in use at exit: 80,048 bytes in 49 blocks total heap usage: 3,025,864 allocs, 3,025,815 frees, 372,746,291 bytes allocated This also translate into a small speedup: Benchmark 1: migrate files:reftable (refcount = 1000000, revision = HEAD~) Time (mean ± σ): 827.2 ms ± 16.5 ms [User: 689.4 ms, System: 124.9 ms] Range (min … max): 809.0 ms … 924.7 ms 50 runs Benchmark 2: migrate files:reftable (refcount = 1000000, revision = HEAD) Time (mean ± σ): 813.6 ms ± 11.6 ms [User: 679.0 ms, System: 123.4 ms] Range (min … max): 786.7 ms … 833.5 ms 50 runs Summary migrate files:reftable (refcount = 1000000, revision = HEAD) ran 1.02 ± 0.02 times faster than migrate files:reftable (refcount = 1000000, revision = HEAD~) Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: provide thin wrapper for lockfile subsystemPatrick Steinhardt
We use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers. Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts. So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/stack: drop only use of `get_locked_file_path()`Patrick Steinhardt
We've got a single callsite where we call `get_locked_file_path()`. As we're about to convert our usage of the lockfile subsystem to instead be used via a compatibility shim we'd have to implement more logic for this single callsite. While that would be okay if Git was the only supposed user of the reftable library, it's a bit more awkward when considering that we have to reimplement this functionality for every user of the library eventually. Refactor the code such that we don't call `get_locked_file_path()` anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: provide thin wrapper for tempfile subsystemPatrick Steinhardt
We use the tempfile subsystem to write temporary tables, but given that we're in the process of converting the reftable library to become standalone we cannot use this subsystem directly anymore. While we could in theory convert the code to use mkstemp(3p) instead, we'd lose access to our infrastructure that automatically prunes tempfiles via atexit(3p) or signal handlers. Provide a thin wrapper for the tempfile subsystem instead. Like this, the compatibility shim is fully self-contained in "reftable/system.c". Downstream users of the reftable library would have to implement their own tempfile shims by replacing "system.c" with a custom version. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/stack: stop using `fsync_component()` directlyPatrick Steinhardt
We're executing `fsync_component()` directly in the reftable library so that we can fsync data to disk depending on "core.fsync". But as we're in the process of converting the reftable library to become standalone we cannot use that function in the library anymore. Refactor the code such that users of the library can inject a custom fsync function via the write options. This allows us to get rid of the dependency on "write-or-die.h". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: stop depending on "hash.h"Patrick Steinhardt
We include "hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore. Introduce a new `enum reftable_hash` to replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable: explicitly handle hash format IDsPatrick Steinhardt
The hash format IDs are used for two different things across the reftable codebase: - They are used as a 32 bit unsigned integer when reading and writing the header in order to identify the hash function. - They are used internally to identify which hash function is in use. When one only considers the second usecase one might think that one can easily change the representation of those hash IDs. But because those IDs end up in the reftable header and footer on disk it is important that those never change. Create separate constants `REFTABLE_FORMAT_ID_*` and use them in contexts where we read or write reftable headers. This serves multiple purposes: - It allows us to more easily discern cases where we actually use those constants for the on-disk format. - It detangles us from the same constants that are defined in libgit.a, which is another required step to convert the reftable library to become standalone. - It makes the next step easier where we stop using `GIT_*_FORMAT_ID` constants in favor of a custom enum. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-19reftable/system: move "dir.h" to its only userPatrick Steinhardt
We still include "dir.h" in "reftable/system.h" even though it is not used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-17reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt
Convert the reftable library such that we handle failures with the new `reftable_buf` interfaces. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt
The `stack_filename()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. There are two interesting edge cases in `reftable_stack_destroy()` and `reftable_addition_close()`. Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt
The `reftable_record_key()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt
The `format_name()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/blocksource: adapt interface namePatrick Steinhardt
Adapt the name of the `strbuf` block source to no longer relate to this interface, but instead to the `reftable_buf` interface. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt
Convert the reftable library to use the `reftable_buf` interface instead of the `strbuf` interface. This is mostly a mechanical change via sed(1) with some manual fixes where functions for `strbuf` and `reftable_buf` differ. The converted code does not yet handle allocation failures. This will be handled in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt
Implement a new `reftable_buf` interface that will replace Git's own `strbuf` interface. This is done due to three reasons: - The `strbuf` interfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not in the context of the reftable library, which is supposed to be usable by third-party applications. - The `strbuf` interface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible. - The `strbuf` interface does not use the pluggable allocators that can be set up via `reftable_set_alloc()`. So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable library will be handled in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable: stop using `strbuf_addf()`Patrick Steinhardt
We're about to introduce our own `reftable_buf` type to replace `strbuf`. One function we'll have to convert is `strbuf_addf()`, which is used in a handful of places. This function uses `snprintf()` internally, which makes porting it a bit more involved: - It is not available on all platforms. - Some platforms like Windows have broken implementations. So by using `snprintf()` we'd also push the burden on downstream users of the reftable library to make available a properly working version of it. Most callsites of `strbuf_addf()` are trivial to convert to not using it. We do end up using `snprintf()` in our unit tests, but that isn't much of a problem for downstream users of the reftable library. While at it, remove a useless call to `strbuf_reset()` in `t_reftable_stack_auto_compaction_with_locked_tables()`. We don't write to the buffer before this and initialize it with `STRBUF_INIT`, so there is no need to reset anything. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-17reftable: stop using `strbuf_addbuf()`Patrick Steinhardt
We're about to introduce our own `reftable_buf` type to replace `strbuf`. Get rid of the seldomly-used `strbuf_addbuf()` function such that we have to reimplement one less function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-10Merge branch 'ps/reftable-alloc-failures'Junio C Hamano
The reftable library is now prepared to expect that the memory allocation function given to it may fail to allocate and to deal with such an error. * ps/reftable-alloc-failures: (26 commits) reftable/basics: fix segfault when growing `names` array fails reftable/basics: ban standard allocator functions reftable: introduce `REFTABLE_FREE_AND_NULL()` reftable: fix calls to free(3P) reftable: handle trivial allocation failures reftable/tree: handle allocation failures reftable/pq: handle allocation failures when adding entries reftable/block: handle allocation failures reftable/blocksource: handle allocation failures reftable/iter: handle allocation failures when creating indexed table iter reftable/stack: handle allocation failures in auto compaction reftable/stack: handle allocation failures in `stack_compact_range()` reftable/stack: handle allocation failures in `reftable_new_stack()` reftable/stack: handle allocation failures on reload reftable/reader: handle allocation failures in `reader_init_iter()` reftable/reader: handle allocation failures for unindexed reader reftable/merged: handle allocation failures in `merged_table_init_iter()` reftable/writer: handle allocation failures in `reftable_new_writer()` reftable/writer: handle allocation failures in `writer_index_hash()` reftable/record: handle allocation failures when decoding records ...
2024-10-04Merge branch 'ak/typofix-2.46-maint'Junio C Hamano
Typofixes. * ak/typofix-2.46-maint: perl: fix a typo mergetool: fix a typo reftable: fix a typo trace2: fix typos
2024-10-04reftable/basics: fix segfault when growing `names` array failsPatrick Steinhardt
When growing the `names` array fails we would end up with a `NULL` pointer. This causes two problems: - We would run into a segfault because we try to free names that we have assigned to the array already. - We lose track of the old array and cannot free its contents. Fix this issue by using a temporary variable. Like this we do not clobber the old array that we tried to reallocate, which will remain valid when a call to realloc(3P) fails. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-03reftable: fix a typoAndrew Kreimer
Fix a typo in comments. Signed-off-by: Andrew Kreimer <algonell@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/basics: ban standard allocator functionsPatrick Steinhardt
The reftable library uses pluggable allocators, which means that we shouldn't ever use the standard allocator functions. But it is an easy mistake to make to accidentally use e.g. free(3P) instead of the reftable-specific `reftable_free()` function, and we do not have any mechanism to detect this misuse right now. Introduce a couple of macros that ban the standard allocators, similar to how we do it in "banned.h". Note that we do not ban the following two classes of functions: - Macros like `FREE_AND_NULL()` or `REALLOC_ARRAY()`. As those expand to code that contains already-banned functions we'd get a compiler error even without banning those macros explicitly. - Git-specific allocators like `xmalloc()` and friends. The primary reason is that there are simply too many of them, so we're rather aiming for best effort here. Furthermore, the eventual goal is to make them unavailable in the reftable library place by not pulling them in via "git-compat-utils.h" anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: introduce `REFTABLE_FREE_AND_NULL()`Patrick Steinhardt
We have several calls to `FREE_AND_NULL()` in the reftable library, which of course uses free(3P). As the reftable allocators are pluggable we should rather call the reftable specific function, which is `reftable_free()`. Introduce a new macro `REFTABLE_FREE_AND_NULL()` and adapt the callsites accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: fix calls to free(3P)Patrick Steinhardt
There are a small set of calls to free(3P) in the reftable library. As the reftable allocators are pluggable we should rather call the reftable specific function, which is `reftable_free()`. Convert the code accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: handle trivial allocation failuresPatrick Steinhardt
Handle trivial allocation failures in the reftable library and its unit tests. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/tree: handle allocation failuresPatrick Steinhardt
The tree interfaces of the reftable library handle both insertion and searching of tree nodes with a single function, where the behaviour is altered between the two via an `insert` bit. This makes it quit awkward to handle allocation failures because on inserting we'd have to check for `NULL` pointers and return an error, whereas on searching entries we don't have to handle it as an allocation error. Split up concerns of this function into two separate functions, one for inserting entries and one for searching entries. This makes it easy for us to check for allocation errors as `tree_insert()` should never return a `NULL` pointer now. Adapt callers accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/pq: handle allocation failures when adding entriesPatrick Steinhardt
Handle allocation failures when adding entries to the pqueue. Adapt its only caller accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/block: handle allocation failuresPatrick Steinhardt
Handle allocation failures in `block_writer_init()` and `block_reader_init()`. This requires us to bubble up error codes into `writer_reinit_block_writer()`. Adapt call sites accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/blocksource: handle allocation failuresPatrick Steinhardt
Handle allocation failures in the blocksource code. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/iter: handle allocation failures when creating indexed table iterPatrick Steinhardt
Handle allocation failures in `new_indexed_table_ref_iter()`. While at it, rename the function to match our coding style. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in auto compactionPatrick Steinhardt
Handle allocation failures in `reftable_stack_auto_compact()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in `stack_compact_range()`Patrick Steinhardt
Handle allocation failures in `stack_compact_range()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures in `reftable_new_stack()`Patrick Steinhardt
Handle allocation failures in `reftable_new_stack()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/stack: handle allocation failures on reloadPatrick Steinhardt
Handle allocation failures in `reftable_stack_reload_once()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/reader: handle allocation failures in `reader_init_iter()`Patrick Steinhardt
Handle allocation failures in `reader_init_iter()`. This requires us to also adapt `reftable_reader_init_*_iterator()` to bubble up the new error codes. Adapt callers accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/reader: handle allocation failures for unindexed readerPatrick Steinhardt
Handle allocation failures when creating unindexed readers. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/merged: handle allocation failures in `merged_table_init_iter()`Patrick Steinhardt
Handle allocation failures in `merged_table_init_iter()`. While at it, merge `merged_iter_init()` into the function. It only has a single caller and merging them makes it easier to handle allocation failures consistently. This change also requires us to adapt `reftable_stack_init_*_iterator()` to bubble up the new error codes of `merged_table_iter_init()`. Adapt callsites accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/writer: handle allocation failures in `reftable_new_writer()`Patrick Steinhardt
Handle allocation failures in `reftable_new_writer()`. Adapt the function to return an error code to return such failures. While at it, rename it to match our code style as we have to touch up every callsite anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/writer: handle allocation failures in `writer_index_hash()`Patrick Steinhardt
Handle allocation errors in `writer_index_hash()`. Adjust its only caller in `reftable_writer_add_ref()` accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/record: handle allocation failures when decoding recordsPatrick Steinhardt
Handle allocation failures when decoding records. While at it, fix some error codes to be `REFTABLE_FORMAT_ERROR`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/record: handle allocation failures on copyPatrick Steinhardt
Handle allocation failures when copying records. While at it, convert from `xstrdup()` to `reftable_strdup()`. Adapt callsites to check for error codes. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/basics: handle allocation failures in `parse_names()`Patrick Steinhardt
Handle allocation failures in `parse_names()` by returning `NULL` in case any allocation fails. While at it, refactor the function to return the array directly instead of assigning it to an out-pointer. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/basics: handle allocation failures in `reftable_calloc()`Patrick Steinhardt
Handle allocation failures in `reftable_calloc()`. While at it, remove our use of `st_mult()` that would cause us to die on an overflow. From the caller's point of view there is not much of a difference between arguments that are too large to be multiplied and a request that is too big to handle by the allocator: in both cases the allocation cannot be fulfilled. And in neither of these cases do we want the reftable library to die. While we could use `unsigned_mult_overflows()` to handle the overflow gracefully, we instead open-code it to further our goal of converting the reftable codebase to become a standalone library that can be reused by external projects. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable: introduce `reftable_strdup()`Patrick Steinhardt
The reftable library provides the ability to swap out allocators. There is a gap here though, because we continue to use `xstrdup()` even in the case where all the other allocators have been swapped out. Introduce `reftable_strdup()` that uses `reftable_malloc()` to do the allocation. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/basics: merge "publicbasics" into "basics"Patrick Steinhardt
The split between "basics" and "publicbasics" is somewhat arbitrary and not in line with how we typically structure code in the reftable library. While we do indeed split up headers into a public and internal part, we don't do that for the compilation unit itself. Furthermore, the declarations for "publicbasics.c" are in "reftable-malloc.h", which isn't in line with our naming schema, either. Fix these inconsistencies by: - Merging "publicbasics.c" into "basics.c". - Renaming "reftable-malloc.h" to "reftable-basics.h" as the public header. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-02reftable/error: introduce out-of-memory error codePatrick Steinhardt
The reftable library does not use the same memory allocation functions as the rest of the Git codebase. Instead, as the reftable library is supposed to be usable as a standalone library without Git, it provides a set of pluggable memory allocators. Compared to `xmalloc()` and friends these allocators are _not_ expected to die when an allocation fails. This design choice is concious, as a library should leave it to its caller to handle any kind of error. While it is very likely that the caller cannot really do much in the case of an out-of-memory situation anyway, we are not the ones to make that decision. Curiously though, we never handle allocation errors even though memory allocation functions are allowed to fail. And as we do not plug in Git's memory allocator via `reftable_set_alloc()` either the consequence is that we'd instead segfault as soon as we run out of memory. While the easy fix would be to wire up `xmalloc()` and friends, it would only fix the usage of the reftable library in Git itself. Other users like libgit2, which is about to revive its efforts to land a backend for reftables, wouldn't be able to benefit from this solution. Instead, we are about to do it the hard way: adapt all allocation sites to perform error checking. Introduce a new error code for out-of-memory errors that we will wire up in subsequent steps. This commit also serves as the motivator for all the remaining steps in this series such that we do not have to repeat the same arguments in every single subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-30Merge branch 'ps/reftable-concurrent-writes'Junio C Hamano
Give timeout to the locking code to write to reftable. * ps/reftable-concurrent-writes: refs/reftable: reload locked stack when preparing transaction reftable/stack: allow locking of outdated stacks refs/reftable: introduce "reftable.lockTimeout"
2024-09-24reftable/stack: allow locking of outdated stacksPatrick Steinhardt
In `reftable_stack_new_addition()` we first lock the stack and then check whether it is still up-to-date. If it is not we return an error to the caller indicating that the stack is outdated. This is overly restrictive in our ref transaction interface though: we lock the stack right before we start to verify the transaction, so we do not really care whether it is outdated or not. What we really want is that the stack is up-to-date after it has been locked so that we can verify queued updates against its current state while we know that it is locked for concurrent modification. Introduce a new flag `REFTABLE_STACK_NEW_ADDITION_RELOAD` that alters the behaviour of `reftable_stack_init_addition()` in this case: when we notice that it is out-of-date we reload it instead of returning an error to the caller. This logic will be wired up in the reftable backend in the next commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-24refs/reftable: introduce "reftable.lockTimeout"Patrick Steinhardt
When multiple concurrent processes try to update references in a repository they may try to lock the same lockfiles. This can happen even when the updates are non-conflicting and can both be applied, so it doesn't always make sense to abort the transaction immediately. Both the "loose" and "packed" backends thus have a grace period that they wait for the lock to be released that can be controlled via the config values "core.filesRefLockTimeout" and "core.packedRefsTimeout", respectively. The reftable backend doesn't have such a setting yet and instead fails immediately when it sees such a lock. But the exact same concepts apply here as they do apply to the other backends. Introduce a new "reftable.lockTimeout" config that controls how long we may wait for a "tables.list" lock to be released. The default value of this config is 100ms, which is the same default as we have it for the "loose" backend. Note that even though we also lock individual tables, this config really only applies to the "tables.list" file. This is because individual tables are only ever locked when we already hold the "tables.list" lock during compaction. When we observe such a lock we in fact do not want to compact the table at all because it is already in the process of being compacted by a concurrent process. So applying the same timeout here would not make any sense and only delay progress. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-16reftable/reader: make table iterator reseekablePatrick Steinhardt
In 67ce50ba26 (Merge branch 'ps/reftable-reusable-iterator', 2024-05-30) we have refactored the interface of reftable iterators such that they can be reused in theory. This patch series only landed the required changes on the interface level, but didn't yet implement the actual logic to make iterators reusable. As it turns out almost all of the infrastructure already does support re-seeking. The only exception is the table iterator, which does not reset its `is_finished` bit. Do so and add a couple of tests that verify that we can re-seek iterators. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>