Fix loose db namespace and move packed-buffer handling into loose ref store #263

Byron · 2021-11-26T04:46:00Z

The loose ref db needs to loose its built-in namespace as it' won't work well in a multi-threaded context unless each thread has their own loose ref DB. The latter is possible, but will stop working once ref-table arrives. So we need to refactor.

Currently the loose ref DB is Sync, but only awkwardly usable due to its packed-buffer being outsourced into the callers responsibility.

git-repository works around this by putting packed-refs into a thread-local cache. The latter isn't optimal as it might well be shared across threads.

The new store should be Sync, be prepared for ref-table (and to unify both loose and ref under one API), and store packed-refs in a thread-safe fashion (if the feature is enabled in git-features, that is).

This is the architecture that was developed during the discovery in #259 .

https://github.com/Byron/gitoxide/blob/38e36a69f5fa6aae2f4cfee7faf11a10cea0f1d0/experiments/odb-redesign/src/lib.rs#L608-L676

Tasks

thread-local ref namespaces in git-repository
general ref store (basis)
- learned that it's currently a little unclear how ref-table will work and handle its shared state. Will it offload this to parameters like the file store? Or will it keep things internal right away? The latter is the preference currently, and if so it would serve as its own handle while being able to efficiently update its internal data structures after writing.
file db with shared mutable state for it to be doing the right thing by default
- keep previous packed parameter accessible in a somewhat lower level method for those who build abstractions on top.
- make using cached shared packs possible, similar to snapshots.
get latest version of this back into link-git of radicle-link - in progress - using mailing list based workflow. Now just a few patches without the upgrade, so nothing else to be done here.

The text was updated successfully, but these errors were encountered:

If the `threading` feature is set, the `threading` module will contain thread-safe primitives for shared ownership and mutation, otherwise these will be their single threaded counterparts. This way, single-threaded applications don't have to pay for threaded primitives.

Previously these were shared in the shared Repo instance, which makes threaded applications impossible to remain deterministic across multiple connections. Now they are local to the thread, which allowed some methods to remove their Result<> as they cannot fail anymore, the reason for this being a breaking change.

…hip (#263) Note that by that definition, I think the current `Easy` implementation does it wrongly in some places, particularly when references are handled.

…efLog` (#263)

This also takes over the existing modifiable buffer implementation and gives it interior mutability to allow more parallel read-only paths.

…263)

…n is overkill (#263) Instead, it's better to simply let the Loose db handle its packed refs buffer itself, because that's the only way it will ever be used, instead of adding too much layering and duplication of what's essentially boilerplate.

…which seems to work except for one caveat: Progress now demands to be Sync even though there is no need and it shouldn't have to think that. Have to go back and try to fix that, as it ultimately bubbles up to every method that uses such method generically, including trait bounds which really shouldn't have to be that strict.

Amazing, and unexpected, glad that worked out after all.

…Sync` (#263) This helps to assure that thread-local computations always work with the kind of types we provide. The ones that are carrying out actions are notably not `Sync` anymore. We cater to that by defining our bounds accordingly, but for those who want to use other utilities that need Sync, using types like `Repository` and `thread_local!()` is the only way to make this work.

…uable (#263) Because that way, the error type can be simpler as it contains errors related to the packed buffer being initialized. Now it's strangely on the side, but maybe one can also re-arrange errors entirely. Seems harder than putting the packed buffer where it belongs.

…only (#263) That way, file databases can't be repositioned anymore, it's recommended to recreate it if that's desired.

This is all thanks to the magic of ref-guard mapping, which indeed transfers pretty well into the land of single-threads.

The packed buffer is now handled internally while loading it on demand. When compiled with `git-features/parallel` the `file::Store` remains send and sync. The packed refs buffer is shared across clones and it's recommended to clone one `file::Store` instance per thread, each of which can use its own namespace.

…ked-refs buffer (#263) It is instead read from the internally synchronized buffer, shared across all instances.

…ter (#263)

All that without blocking on shared state. The idea is that the first transaction can safe time, which at least helps in the single-threaded case.

The latter is useful when iterating without blocking everyone else or having issues with deadlocks if an iterator tries to find a reference while the iteration is ongoing.

…refixed(…)` respectively (#263) This way, it's possible to keep shared ownership of the packed buffer while allowing the exact same iterator machinery to work as before.

That way, we avoid races around the mtime precision, which on some systems isn't high enough, to provide a consistent view right after a change.

`Reference::log_iter(…)` now is a platform instead of a forward iterator, which requires a call to `.all()` to return the forward iterator like previously. `Reference::log_iter_rev(…)` was removed in favor of `Reference::log_iter(…).rev()`.

The latter now returns a standard Platform to iterate over all reflog entries from oldest to newest or vice versa.

…ore (#263) Instead this could be done by running all tests for the file store with the type replaced to be the general store. This means we have a normal run that runs with the default types, and a general-store run that replaces the type used with the one for the general store and it should all be homogenous. The latter is certainly possible now that the loose ref store sports interior mutability and a few platforms to get rid of some state.

…ut it (#263) The question is really how reftable works and if all state can be shared and sync, or if some should rather be thread-local (similar to what the ODB is going to do).

At this low level, it's important to be clear about RefLogs and rather force the caller to specify the ref-log mode. Technically it depends on a few factors, `git-repository` deals with them, but certainly shouldn't default to anything without being clear.

That way, abstractions can still be built that have other ways of managing the packed-refs buffer, allowing it to stay more persistent.

…fer()` (#263) This makes much clearer what it actually does, as previously it might have been a stored packed buffer as well.

…hots (#263)

These methods allow using an own packed buffer, usually obtained through `cached_packed_buffer()`.

…ow_packed() (#263) This allows a stable/non-changing buffer to be used.

This makes the API complete as now there is a methods that uses the internal buffer, or the provided one, where both can have its benefits.

It's much slower than the read lock version, for sure, and inhibits concurrency even more than what libgit2 does.

…cks (#263) Here is an explanation for this: radicle-dev/radicle-link#792 (comment) It turns out the upgradable locks are only great if there is heaviest contention, as they can be a little faster.

) Multiple writers can come to the same conclusion and try to reload a packed buffer. Now they will reassuure themselves this is still needed after getting exclusive write access. Notably, git doesn't do that as it uses snapshots, and libgit2 also doesn't seem to do that. Technically this is a bit slower in the common case due to the extra state call.

Byron mentioned this issue Nov 26, 2021

thread-local loose ref namespace #264

Merged

2 tasks

Byron added a commit that referenced this issue Nov 26, 2021

Add cheap and sync loose ref DB directly to state (#263)

38c8146

Byron mentioned this issue Nov 27, 2021

general ref store #265

Merged

11 tasks

Byron added a commit that referenced this issue Nov 27, 2021

change!: move git_ref::file::WriteRefLog to `git_ref::store::WriteR…

9524732

…efLog` (#263)

Byron added a commit that referenced this issue Nov 27, 2021

sketch a store handle (#263)

fc6480b

Byron added a commit that referenced this issue Nov 27, 2021

Sketch of State is seen in store handle and store itself (#263)

f87f852

This also takes over the existing modifiable buffer implementation and gives it interior mutability to allow more parallel read-only paths.

Byron added a commit that referenced this issue Nov 27, 2021

Look into iteration, but realize that it's harder than finding refs (#…

fc753a8

…263)

Byron added a commit that referenced this issue Nov 27, 2021

RwLock seems to cause the Sync requirements, use Mutex instead (#263)

3cb5065

Amazing, and unexpected, glad that worked out after all.

Byron added a commit that referenced this issue Nov 27, 2021

refactor!: file::Store::base is now file::Store::base() and read-…

0e18753

…only (#263) That way, file databases can't be repositioned anymore, it's recommended to recreate it if that's desired.

Byron added a commit that referenced this issue Nov 27, 2021

Make it possible to return read guards with packed buffers (#263)

f5c3c8f

This is all thanks to the magic of ref-guard mapping, which indeed transfers pretty well into the land of single-threads.

Byron added a commit that referenced this issue Nov 27, 2021

Load packed buffer with interior mutability (#263)

ae2eef1

Byron added a commit that referenced this issue Nov 28, 2021

change!: file::ReferenceExt::peel_to_id_in_place(…) now without pac…

55940eb

…ked-refs buffer (#263) It is instead read from the internally synchronized buffer, shared across all instances.

Byron added a commit that referenced this issue Nov 28, 2021

Adjustments to match new signature of peel_to_ids_in_place (#263)

f87a11f

Byron added a commit that referenced this issue Nov 28, 2021

change!: file::ReferenceExt::follow(…) now without packed refs parame…

4641499

…ter (#263)

Byron added a commit that referenced this issue Nov 28, 2021

Let file transactions reuse the cached packed buffer (#263)

a9096b9

All that without blocking on shared state. The idea is that the first transaction can safe time, which at least helps in the single-threaded case.

Byron added a commit that referenced this issue Nov 28, 2021

Adjust to chagne signature of peel_to_id_in_place (again?) (#263)

dd5341f

Byron added a commit that referenced this issue Nov 28, 2021

refactor(#263)

5fc3817

Byron added a commit that referenced this issue Nov 28, 2021

Adapt to new iteration Platform in git-ref (#263)

b5a749e

Byron added a commit that referenced this issue Nov 28, 2021

fmt (#263)

fbeddeb

Byron added a commit that referenced this issue Nov 28, 2021

Assure the packed buffer is reloaded after a modification (#263)

f5570ff

That way, we avoid races around the mtime precision, which on some systems isn't high enough, to provide a consistent view right after a change.

Byron added a commit that referenced this issue Nov 28, 2021

Add platform for log iteration to hold byte buffers (#263)

1cd2362

Byron added a commit that referenced this issue Nov 28, 2021

rename!: Reference::logs() -> Reference::log_iter() (#263)

951c050

The latter now returns a standard Platform to iterate over all reflog entries from oldest to newest or vice versa.

Byron changed the title ~~Fix loose db namespace and move packed-buffer handling into 'general' store~~ Fix loose db namespace and move packed-buffer handling into loose ref store Nov 28, 2021

Byron added a commit that referenced this issue Nov 28, 2021

Adjust to changes in git-ref (#263)

1e32855

Byron added a commit that referenced this issue Nov 28, 2021

feat: add file::Store::try_find_packed(…, packed_buffer) (#263)

b030884

That way, abstractions can still be built that have other ways of managing the packed-refs buffer, allowing it to stay more persistent.

Byron added a commit that referenced this issue Nov 28, 2021

change!: rename file::Store::packed_buffer() to `…::open_packed_buf…

b431fb0

…fer()` (#263) This makes much clearer what it actually does, as previously it might have been a stored packed buffer as well.

Byron added a commit that referenced this issue Nov 28, 2021

feat: add file::Store::cached_packed_buffer() for packed refs snaps…

9eb3a31

…hots (#263)

Byron added a commit that referenced this issue Nov 28, 2021

feat: Add file::Store::iter_(prefixed_)packed() (#263)

6a17416

These methods allow using an own packed buffer, usually obtained through `cached_packed_buffer()`.

Byron added a commit that referenced this issue Nov 29, 2021

feat: add Reference::peel_to_id_in_place_packed() and Reference::foll…

03bcac2

…ow_packed() (#263) This allows a stable/non-changing buffer to be used.

Byron added a commit that referenced this issue Nov 29, 2021

feat: Add file::Store::find_packed(…, buffer) (#263)

f6181af

This makes the API complete as now there is a methods that uses the internal buffer, or the provided one, where both can have its benefits.

Byron added a commit that referenced this issue Nov 29, 2021

refactor (#263)

e41ab82

Byron closed this as completed Nov 30, 2021

Byron added a commit that referenced this issue Dec 1, 2021

object-access tests for upgradable RwLock (#263)

658125a

It's much slower than the read lock version, for sure, and inhibits concurrency even more than what libgit2 does.

Byron added a commit that referenced this issue Dec 1, 2021

Also actually upgrade the upgradable lock (#263)

25338c0

Byron added a commit that referenced this issue Dec 1, 2021

test object-access read() + write() worst-case performance (#263)

fa47115

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix loose db namespace and move packed-buffer handling into loose ref store #263

Fix loose db namespace and move packed-buffer handling into loose ref store #263

Byron commented Nov 26, 2021 •

edited

Loading

Fix loose db namespace and move packed-buffer handling into loose ref store #263

Fix loose db namespace and move packed-buffer handling into loose ref store #263

Comments

Byron commented Nov 26, 2021 • edited Loading

Tasks

Byron commented Nov 26, 2021 •

edited

Loading