git/reftable/stack_test.c, branch v2.45.2

Merge branch 'jt/reftable-geometric-compaction'

2024-04-16T21:50:30Z

The strategy to compact multiple tables of reftables after many operations accumulate many entries has been improved to avoid accumulating too many tables uncollected. * jt/reftable-geometric-compaction: reftable/stack: use geometric table compaction reftable/stack: add env to disable autocompaction reftable/stack: expose option to disable auto-compaction

Merge branch 'ps/pack-refs-auto'

2024-04-09T21:31:45Z

"git pack-refs" learned the "--auto" option, which is a useful addition to be triggered from "git gc --auto". Acked-by: Karthik Nayak cf. * ps/pack-refs-auto: builtin/gc: pack refs when using `git maintenance run --auto` builtin/gc: forward git-gc(1)'s `--auto` flag when packing refs t6500: extract objects with "17" prefix builtin/gc: move `struct maintenance_run_opts` builtin/pack-refs: introduce new "--auto" flag builtin/pack-refs: release allocated memory refs/reftable: expose auto compaction via new flag refs: remove `PACK_REFS_ALL` flag refs: move `struct pack_refs_opts` to where it's used t/helper: drop pack-refs wrapper refs/reftable: print errors on compaction failure reftable/stack: gracefully handle failed auto-compaction due to locks reftable/stack: use error codes when locking fails during compaction reftable/error: discern locked/outdated errors reftable/stack: fix error handling in `reftable_stack_init_addition()`

reftable/stack: use geometric table compaction

2024-04-08T19:11:10Z

To reduce the number of on-disk reftables, compaction is performed. Contiguous tables with the same binary log value of size are grouped into segments. The segment that has both the lowest binary log value and contains more than one table is set as the starting point when identifying the compaction segment. Since segments containing a single table are not initially considered for compaction, if the table appended to the list does not match the previous table log value, no compaction occurs for the new table. It is therefore possible for unbounded growth of the table list. This can be demonstrated by repeating the following sequence: git branch -f foo git branch -d foo Each operation results in a new table being written with no compaction occurring until a separate operation produces a table matching the previous table log value. Instead, to avoid unbounded growth of the table list, the compaction strategy is updated to ensure tables follow a geometric sequence after each operation by individually evaluating each table in reverse index order. This strategy results in a much simpler and more robust algorithm compared to the previous one while also maintaining a minimal ordered set of tables on-disk. When creating 10 thousand references, the new strategy has no performance impact: Benchmark 1: update-ref: create refs sequentially (revision = HEAD~) Time (mean ± σ): 26.516 s ± 0.047 s [User: 17.864 s, System: 8.491 s] Range (min … max): 26.447 s … 26.569 s 10 runs Benchmark 2: update-ref: create refs sequentially (revision = HEAD) Time (mean ± σ): 26.417 s ± 0.028 s [User: 17.738 s, System: 8.500 s] Range (min … max): 26.366 s … 26.444 s 10 runs Summary update-ref: create refs sequentially (revision = HEAD) ran 1.00 ± 0.00 times faster than update-ref: create refs sequentially (revision = HEAD~) Some tests in `t0610-reftable-basics.sh` assert the on-disk state of tables and are therefore updated to specify the correct new table count. Since compaction is more aggressive in ensuring tables maintain a geometric sequence, the expected table count is reduced in these tests. In `reftable/stack_test.c` tests related to `sizes_to_segments()` are removed because the function is no longer needed. Also, the `test_suggest_compaction_segment()` test is updated to better showcase and reflect the new geometric compaction behavior. Signed-off-by: Justin Tobler Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

reftable/stack: expose option to disable auto-compaction

2024-04-08T19:11:10Z

The reftable stack already has a variable to configure whether or not to run auto-compaction, but it is inaccessible to users of the library. There exist use cases where a caller may want to have more control over auto-compaction. Move the `disable_auto_compact` option into `reftable_write_options` to allow external callers to disable auto-compaction. This will be used in a subsequent commit. Signed-off-by: Justin Tobler Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/pack-refs-auto' into jt/reftable-geometric-compaction

2024-04-05T17:34:23Z

* ps/pack-refs-auto: builtin/gc: pack refs when using `git maintenance run --auto` builtin/gc: forward git-gc(1)'s `--auto` flag when packing refs t6500: extract objects with "17" prefix builtin/gc: move `struct maintenance_run_opts` builtin/pack-refs: introduce new "--auto" flag builtin/pack-refs: release allocated memory refs/reftable: expose auto compaction via new flag refs: remove `PACK_REFS_ALL` flag refs: move `struct pack_refs_opts` to where it's used t/helper: drop pack-refs wrapper refs/reftable: print errors on compaction failure reftable/stack: gracefully handle failed auto-compaction due to locks reftable/stack: use error codes when locking fails during compaction reftable/error: discern locked/outdated errors reftable/stack: fix error handling in `reftable_stack_init_addition()`

Merge branch 'ps/reftable-unit-test-nfs-workaround'

2024-04-01T20:21:35Z

A unit test for reftable code tried to enumerate all files in a directory after reftable operations and expected to see nothing but the files it wanted to leave there, but was fooled by .nfs* cruft files left, which has been corrected. * ps/reftable-unit-test-nfs-workaround: reftable: fix tests being broken by NFS' delete-after-close semantics

reftable/stack: gracefully handle failed auto-compaction due to locks

2024-03-25T16:54:07Z

Whenever we commit a new table to the reftable stack we will end up invoking auto-compaction of the stack to keep the total number of tables at bay. This auto-compaction may fail though in case at least one of the tables which we are about to compact is locked. This is indicated by the compaction function returning `REFTABLE_LOCK_ERROR`. We do not handle this case though, and thus bubble that return value up the calling chain, which will ultimately cause a failure. Fix this bug by ignoring `REFTABLE_LOCK_ERROR`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

reftable/error: discern locked/outdated errors

2024-03-25T16:51:11Z

We currently throw two different errors into a similar-but-different error code: - Errors when trying to lock the reftable stack. - Errors when trying to write to the reftable stack which has been modified concurrently. This results in unclear error handling and user-visible error messages. Create a new `REFTABLE_OUTDATED_ERROR` so that those error conditions can be clearly told apart from each other. Adjust users of the old `REFTABLE_LOCK_ERROR` to use the new error code as required. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

reftable: fix tests being broken by NFS' delete-after-close semantics

2024-03-21T17:32:21Z

It was reported that the reftable unit tests in t0032 fail with the following assertion when running on top of NFS: running test_reftable_stack_compaction_concurrent_clean reftable/stack_test.c: 1063: failed assertion count_dir_entries(dir) == 2 Aborted Setting a breakpoint immediately before the assertion in fact shows the following list of files: ./stack_test-1027.QJBpnd ./stack_test-1027.QJBpnd/0x000000000001-0x000000000003-dad7ac80.ref ./stack_test-1027.QJBpnd/.nfs000000000001729f00001e11 ./stack_test-1027.QJBpnd/tables.list Note the weird ".nfs*" file? This file is maintained by NFS clients in order to emulate delete-after-last-close semantics that we rely on in the reftable code [1]. Instead of unlinking the file right away and keeping it open in the client, the NFS client will rename it to ".nfs*" and then delete that temporary file when the last reference to it gets dropped. Quoting the NFS FAQ: > D2. What is a "silly rename"? Why do these .nfsXXXXX files keep > showing up? > > A. Unix applications often open a scratch file and then unlink it. > They do this so that the file is not visible in the file system name > space to any other applications, and so that the system will > automatically clean up (delete) the file when the application exits. > This is known as "delete on last close", and is a tradition among > Unix applications. > > Because of the design of the NFS protocol, there is no way for a > file to be deleted from the name space but still remain in use by an > application. Thus NFS clients have to emulate this using what > already exists in the protocol. If an open file is unlinked, an NFS > client renames it to a special name that looks like ".nfsXXXXX". > This "hides" the file while it remains in use. This is known as a > "silly rename." Note that NFS servers have nothing to do with this > behavior. This of course throws off the assertion that we got exactly two files in that directory. The test in question triggers this behaviour by holding two open file descriptors to the "tables.list" file. One of the references is because we are about to append to the stack, whereas the other reference is because we want to compact it. As the compaction has just finished we already rewrote "tables.list" to point to the new contents, but the other file descriptor pointing to the old version is still open. Thus we trigger the delete-after-last-close emulation. Furthermore, it was reported that this behaviour only triggers with 4f36b8597c (reftable/stack: fix race in up-to-date check, 2024-01-18). This is expected as well because it is the first point in time where we actually keep the "tables.list" file descriptor open for the stat cache. Fix this bug by skipping over any files that start with a leading dot when counting files. While we could explicitly check for a prefix of ".nfs", other network file systems like SMB for example do the same trickery but with a ".smb" prefix. In any case though, this loosening of the assertion should be fine given that the reftable library would never write files with leading dots by itself. [1]: https://nfs.sourceforge.net/#faq_d2 Reported-by: Chuck Lever Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

reftable/record: convert old and new object IDs to arrays

2024-03-05T17:10:06Z

In 7af607c58d (reftable/record: store "val1" hashes as static arrays, 2024-01-03) and b31e3cc620 (reftable/record: store "val2" hashes as static arrays, 2024-01-03) we have converted ref records to store their object IDs in a static array. Convert log records to do the same so that their old and new object IDs are arrays, too. This change results in two allocations less per log record that we're iterating over. Before: HEAP SUMMARY: in use at exit: 13,473 bytes in 122 blocks total heap usage: 8,068,495 allocs, 8,068,373 frees, 401,011,862 bytes allocated After: HEAP SUMMARY: in use at exit: 13,473 bytes in 122 blocks total heap usage: 6,068,489 allocs, 6,068,367 frees, 361,011,822 bytes allocated Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano