[Draft] Atomic locking of createExecDir
#1353
Open
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
jerrymarino
force-pushed
the
jmarino/exec_dir_locking
branch
2 times, most recently
from
92bdeac
to
5e37811
Compare
Summary: today we reference an `Entry` after it has been "downloaded". Today we expire _during_ execution and `InputFetch` stage when `Entries` for keys either existed on the file system or were loaded from the distributed store in the `InputFetch` operation. Will also mutate existing entries due to `deleting` the reference to the last inode of a file. The latter manfiests in several ways e.g. like failing the report stage of a concurrent action. Prior to `CASFileCache.putDirectory` we increment `keyReferences` to prevent eviction during execution. This provides atomicity gaurentees around the _keys_ within directories while minimizing mutexes. Fixes my contention with `createExecDir` / and my write-to-CAS failures which manifested as a result. TLDR: Logical and atomic locking of an `execDir` by gaining references to keys via the new method `lockDirectoryKeys`. Because there are many references to a single `Entry` this needs to be refernce counted for each concurrent `execDir` and must upheld through the entire lifetime of the directory. Testing: I have a number of manual integration tests that need reworking so I can add them here and will add these as part of the PR to get it out of the `Draft` state
jerrymarino
force-pushed
the
jmarino/exec_dir_locking
branch
2 times, most recently
from
c020ae0
to
70a2b6d
Compare
- move loop directory out of critical section - improve logging/method naming
jerrymarino
force-pushed
the
jmarino/exec_dir_locking
branch
from
70a2b6d
to
bd4b26e
Compare
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.
Summary: today we reference an
Entryafter it has been "downloaded". Today we expire during execution andInputFetchstage whenEntriesfor keys either existed on the file system or were loaded from the distributed store in theInputFetchoperation. Will also mutate existing entries due todeletingthe reference to the last inode of a file. The latter manfiests in several ways e.g. like failing the report stage of a concurrent action.Prior to
CASFileCache.putDirectorywe incrementkeyReferencesto prevent eviction during execution. This provides atomicity guarantees around the keys within directories while minimizing mutexes.Fixes my contention with
createExecDir/ and my write-to-CAS failures which manifested as a result.TLDR:
Logical and atomic locking of an
execDirby gaining references to keys via the new methodlockDirectoryKeys. Because there are many references to a singleEntrythis needs to be reference counted for each concurrentexecDirand must upheld through the entire lifetime of the directory.Testing:
I have a number of manual integration tests that need reworking so I can add them here and will add these as part of the PR to get it out of the
Draftstate