refactor(layer): use detached init #7152
Conversation
2706 tests run: 2575 passed, 0 failed, 131 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
178a2d6 at 2024-03-19T14:13:16.985Z :recycle: |
|
Might be more ecological to just merge the #7030 because of e2e re-runs. |
|
Can you update the PR description to explain how this PR fits into fixing hangs during eviction? It's not obvious if this PR is the fix or a precursor. |
minimal changes to just have one callsite for "downloaded".
koivunej
force-pushed
the
joonas/layer-use-detached-init
branch
from
7167851
to
178a2d6
Compare
Unsure when I added this to the description in relation to your comment:
That is how it fits. This is important because it will remove the case where the internal state is uninitialized:
The remaining cases after the above are:
|
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.
The second part of work towards fixing
Layer::keep_residentso 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.Split off from #7030.
Cc: #5331
Reviewing: commit by commit with whitespace hidden works.