Move uid lock into LiveVersionMap #27905
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
While the LiveVersionMap is an internal class that belongs to the engine we do rely on some external locking to enforce the desired semantics. Yet, in tests we mimic the outer locking but we don't have any way to enforce or assert on that the lock is actually hold. This change moves the KeyedLock inside the LiveVersionMap that allows the engine to access it as before but enables assertions in the LiveVersionMap to ensure the lock for the modifying or reading key is actually hold.
bleskes
approved these changes
Dec 19, 2017
| private Releasable acquireLock(BytesRef uid) { | ||
| return keyedLock.acquire(uid); | ||
| } | ||
|
|
||
| private Releasable acquireLock(Term uid) { |
There was a problem hiding this comment.
I think we can clean this method up - it doesn't add much but craft and at least in one place we go and do the same conversion later on - might as well capture it as a value on the caller
try (Releasable ignore = acquireLock(get.uid())) { // we need to lock here to access the version map to do this truly in RT
+ versionValue = getVersionFromMap(get.uid().bytes());
+ }
There was a problem hiding this comment.
I am not sure I follow
s1monw
added a commit
that referenced
this pull request
Dec 20, 2017
While the LiveVersionMap is an internal class that belongs to the engine we do rely on some external locking to enforce the desired semantics. Yet, in tests we mimic the outer locking but we don't have any way to enforce or assert on that the lock is actually hold. This change moves the KeyedLock inside the LiveVersionMap that allows the engine to access it as before but enables assertions in the LiveVersionMap to ensure the lock for the modifying or reading key is actually hold.
martijnvg
added a commit
that referenced
this pull request
Dec 21, 2017
* es/6.x: (43 commits) ingest: upgraded ingest geoip's geoip2's dependencies. [TEST] logging for update by query test #27820 Use full profile on JDK 10 builds Require Gradle 4.3 Add unreleased v6.1.2 version TEST: reduce blob size #testExecuteMultipartUpload Check index under the store metadata lock (#27768) Upgrade to Lucene 7.2.0. (#27910) Fixed test to be up to date with the new database files. Use `_refresh` to shrink the version map on inactivity (#27918) Make KeyedLock reentrant (#27920) Fixes DocStats to not report index size < -1 (#27863) Disable TestZenDiscovery in cloud providers integrations test ingest: Upgraded the geolite2 databases. [Issue-27716]: CONTRIBUTING.md IntelliJ configurations settings are confusing. (#27717) [Test] Fix IndicesClientDocumentationIT (#27899) Move uid lock into LiveVersionMap (#27905) Mute testRetentionPolicyChangeDuringRecovery Increase Gradle heap space to 1536m Move GlobalCheckpointTracker and remove SequenceNumbersService (#27837) ...
clintongormley
added
:Distributed Indexing/Distributed
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.
While the LiveVersionMap is an internal class that belongs to the engine we do
rely on some external locking to enforce the desired semantics. Yet, in tests
we mimic the outer locking but we don't have any way to enforce or assert on
that the lock is actually hold. This change moves the KeyedLock inside the
LiveVersionMap that allows the engine to access it as before but enables
assertions in the LiveVersionMap to ensure the lock for the modifying or
reading key is actually hold.