-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Remove hppc from deletion policy #86072
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
Conversation
The CombinedDeletionPolicy keeps ref counts for each index snapshot using an hppc primitive map. This commit converts it to use a standard HashMap. relates elastic#84735
Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one suggestion on making it potentially even faster and a little simpler than it is today but not important :)
@@ -164,7 +163,7 @@ synchronized boolean releaseCommit(final IndexCommit snapshotCommit) { | |||
+ "], releasing commit [" | |||
+ releasingCommit | |||
+ "]"; | |||
final int refCount = snapshottedCommits.addTo(releasingCommit, -1); // release refCount | |||
final int refCount = snapshottedCommits.merge(releasingCommit, -1, Integer::sum); // release refCount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I think we could even do
final Integer refCount = snapshottedCommits.compute(releaseCommit, (key, count) -> {
if (count == 1) {
return null;
}
return count - 1;
})
and adjust the logic below to a null check instead of == 0
and drop the conditional remove
from the map. merge
seems kind of (slightly) wrong here when we assume that we have an entry in the map for sure?
The CombinedDeletionPolicy keeps ref counts for each index snapshot
using an hppc primitive map. This commit converts it to use a standard
HashMap.
relates #84735