Simplify computation of whether a GCP sync is needed #84865
Conversation
We do a bunch of allocation simply to compute whether any replica's global checkpoint lags behind the primary. With this commit we skip all that jazz and just use a loop.
DaveCTurner
added
:Distributed Indexing/CRUD
elasticmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
| @@ -2690,17 +2689,15 @@ public void maybeSyncGlobalCheckpoint(final String reason) { | |||
| final SeqNoStats stats = getEngine().getSeqNoStats(replicationTracker.getGlobalCheckpoint()); | |||
| final boolean asyncDurability = indexSettings().getTranslogDurability() == Translog.Durability.ASYNC; | |||
| if (stats.getMaxSeqNo() == stats.getGlobalCheckpoint() || asyncDurability) { | |||
| final ObjectLongMap<String> globalCheckpoints = getInSyncGlobalCheckpoints(); | |||
| final long globalCheckpoint = replicationTracker.getGlobalCheckpoint(); | |||
| final var trackedGlobalCheckpointsNeedSync = replicationTracker.trackedGlobalCheckpointsNeedSync(); | |||
There was a problem hiding this comment.
Wouldn't it make sense to keep this in line in the conditional since it's an OR so maybe we don't need to evaluate it all the time?
There was a problem hiding this comment.
Ehh maybe, I didn't want to have to reason about whether the ordering between this and checking replication tracker.pendingInSync() was important. Still the expensive bit is getInSyncGlobalCheckpoints() which we did every time anyway, and asyncDurability is almost always false so I doubt it makes much difference.
| /** | ||
| * @return true iff any tracked global checkpoint for an in-sync copy lags behind our global checkpoint | ||
| */ | ||
| public synchronized boolean trackedGlobalCheckpointsNeedSync() { |
There was a problem hiding this comment.
Why do we need to synchronize here when previously we didn't synchronize while running the same loop?
Was this a bug before?
There was a problem hiding this comment.
Previously we were synchronized in the loop within ReplicationTracker#getInSyncGlobalCheckpoints() that iterates over ReplicationTracker#checkpoints.
|
😁 better than suppressing an insightful question tho... |
DaveCTurner
deleted the
2022-03-10-replication-tracker-trackedGlobalCheckpointsNeedSync
branch
We do a bunch of allocation simply to compute whether any replica's
global checkpoint lags behind the primary. With this commit we skip all
that jazz and just use a loop.