Skip to content

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 21, 2022

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

rjernst added 2 commits April 20, 2022 05:55
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
@rjernst rjernst added :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >refactoring v8.3.0 labels Apr 21, 2022
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 21, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Contributor

@original-brownbear original-brownbear left a 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 :)

+ releasingCommit
+ "]";
final int refCount = snapshottedCommits.addTo(releasingCommit, -1); // release refCount
final int refCount = snapshottedCommits.merge(releasingCommit, -1, Integer::sum); // release refCount
Copy link
Contributor

@original-brownbear original-brownbear Apr 21, 2022

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?

@rjernst rjernst mentioned this pull request Apr 21, 2022
43 tasks
@rjernst rjernst merged commit 3f7b43a into elastic:master Apr 21, 2022
@rjernst rjernst deleted the hppc/snapshot_commits branch April 21, 2022 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >refactoring Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants