Add N5Reader.readShard and N5Writer.writeShard - #201
Closed
bogovicj wants to merge 19 commits into
Closed
Conversation
The existing deleteBlock() loads the DatasetAttributes and calls this new variant.
Add default implementation of N5writer.writeRegion
* multiple read locks can be held across threads * only a single write lock can be held * still to do: holding a read lock does not prevent writing see #141
refactor: FileKeyLockManager is a static singleton now, and more KeyLockState logic is package-private Signed-off-by: Caleb Hulbert <cmhulbert@gmail.com>
Locks are not tied to the locking thread anymore. (A LockedChannel can be created by one thread and then be released by another thread). This required to replace ReentrantReadWriteLock with manual reader/writer counting guarded by Semaphores. LockedFileChannel is now created by KeyLockState (instead of FileKeyLockManager) because for releasing we need to distinguish between read and write locks. I revised clean-up of stale KeyLockStates by putting WeakReferences in the ConcurrentHashMap and adding a ReferenceQueue to remove entries whose KeyLockState has been GCed. I put no special logic to handle leaked LockedFileChannels (that are abandoned without being properly closed). Surprisingly, this still works. When the KeyLockState is GCed, its associated ChannelLock along with the existing FileChannel and FileLock is also GCed. At least on MacOS, this causes the JVM to release the system-level lock, and everything works out fine.
…nnel This fixes behaviour on Windows, where the we cannot open another FileChannel on the write-locked path. We have to use the channel that we locked.
Fix/thread locking
* add a few tests
bogovicj
marked this pull request as ready for review
January 30, 2026 19:51
Contributor
Author
|
Want to highlight this commit 6add658 that addresses an issue that came up during shard deletion. At this time, we need to ensure that this reading happens before a write lock is acquired, hence the fix in the commit above. We may need to revisit this after #202 |
* rather return, leaving the request's blocks as null
* test a list of nulls is returned for non-existing blocks * test number of backend read calls for readBlocks
cmhulbert
force-pushed
the
development
branch
from
February 11, 2026 20:14
1bc6aea to
ca384ff
Compare
Contributor
Author
|
these commits were merged by #203 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
And a few tests.
see #196