New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
heavier_once_cell: add detached init support #7135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this eventually allow us to remove the permit
argument from the closure signature that is required by get_or_init
?
I will not be using that method in Layer once my refactorings are in, so might as well. |
2682 tests run: 2557 passed, 0 failed, 125 skipped (full report)Flaky tests (2)Postgres 16
Postgres 14
Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
d98721a at 2024-03-15T16:26:41.008Z :recycle: |
The second part of work towards fixing `Layer::keep_resident` so that it does not need to repair the internal state. #7135 added a nicer API for initialization. This PR uses it to remove a few indentation levels and the loop construction. The next PR #7175 will use the refactorings done in this PR, and always initialize the internal state after a download. Cc: #5331
Before this PR, cancellation for `LayerInner::get_or_maybe_download` could occur so that we have downloaded the layer file in the filesystem, but because of the cancellation chance, we have not set the internal `LayerInner::inner` or initialized the state. With the detached init support introduced in #7135 and in place in #7152, we can now initialize the internal state after successfully downloading in the spawned task. The next PR will fix the remaining problems that this PR leaves: - `Layer::keep_resident` is still used because - `Layer::get_or_maybe_download` always cancels an eviction, even when canceled Split off from #7030. Stacked on top of #7152. Cc: #5331.
Aiming for the design where
heavier_once_cell::OnceCell
is initialized by a future factory lead to awkwardness with howLayerInner::get_or_maybe_download
looks right now with theloop
. The loop helps with two situations:LayerInner::get_or_maybe_download
that canceled a pending eviction was canceled leaving theheavier_once_cell::OnceCell
uninitialized but needing repair by the nextLayerInner::get_or_maybe_download
By instead supporting detached initialization in
heavier_once_cell::OnceCell
via anOnceCell::get_or_detached_init
, we can fix what the monolithic #7030 does:heavier_once_cell::OnceCell
regardless of the download starter being canceledLayerInner::get_or_maybe_download
no longer stops eviction but can win it if not canceledSplit off from #7030.
Cc: #5331