-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Implement transactional large blob cache writes #9726
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yamadapc
force-pushed
the
issue/improve-fs-cache-reliability
branch
from
May 20, 2024 05:57
2dfa13c
to
24c77a6
Compare
yamadapc
force-pushed
the
issue/improve-fs-cache-reliability
branch
from
May 20, 2024 06:17
46149ef
to
1aca708
Compare
This breaks parcel-query because it directly reads the cache filesystem and assumes it's in a certain state. |
JakeLane
approved these changes
May 23, 2024
yamadapc
added a commit
that referenced
this pull request
Jul 9, 2024
* Revert "Hide random cache keys under feature-flag (#9804)" This reverts commit 2ce4efe. * Revert "Clean-up existing cache entries on overwrite (#9774)" This reverts commit 5b0f645. * Revert "Implement transactional large blob cache writes (#9726)" This reverts commit d988abe. * Fix conflict issues --------- Co-authored-by: pyamada (Remote Dev Environment) <pyamada@atlassian.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds unit-tests for FS/LMDB caches and implements transactional writes. This prevents a large blob write from failing partially or from broken entries from being read.
The implementation of this is in
LMDBCache.js
. Large blobs are stored through an entityLargeBlobEntry
, which contains a randomly generated key for theFSCache
large blob entries used. SinceFSCache
large blob keys are generated through this mechanism, it's not possible for a partially written entry to be read. Furthermore, it's not possible for the entry to become corrupted.In addition to this, changes were made to the
RequestTracker
cache writing.Previously,
parcel-query
worked by listing directory files under the cache directory and finding the latest large blob keys on disk. This was very coupled with the cache implementation in use. To improve this and fix breakage caused by my changes, a newRequestCacheInfo
entry is written with the latest request graph cache key.parcel-query
is changed to read this entry, then scan the request graph nodes to find the asset and bundle graph cache entries.Some
parcel-query
tests were patched as it was found that they only previously worked due to stale.parcel-cache
directories being present (e.g.: on v2 on a clean workspace, running only theparcel-query
integration tests will fail; they only pass in CI because other tests populate a directory).