tkhurana commented on code in PR #2600:
URL: https://github.com/apache/phoenix/pull/2600#discussion_r3797374894
##########
phoenix-core-server/src/main/java/org/apache/phoenix/replication/ReplicationLogDiscoveryForwarder.java:
##########
@@ -156,32 +161,42 @@ && isLogCopyThroughputAboveThreshold(srcStat.getLen(),
copyTime)
@Override
protected void processNoMoreRoundsLeft() throws IOException {
- // check if we are caught up so that we can transition to SYNC state
- // we are caught up when there are no files currently in the out progress
directory
- // and no new files exist for ongoing round
+ // A non-empty in-progress directory means this RS has claimed-but-stuck
files, so its forward
+ // path to the peer is unhealthy: neither promote its own mode nor claim
the group is in sync.
+ if (!replicationLogTracker.getInProgressFiles().isEmpty()) {
+ LOG.info("In-progress directory not empty for {}, skipping mode
promotion and sync claim",
+ logGroup);
+ return;
+ }
+
+ // No stuck files, so promote this RS's own mode on that signal alone.
Gating on the next
+ // round's
+ // shard would pin an idle RS forever: it is a shared directory holding
every co-active RS's
+ // live
+ // rotation writer. The flip is self-validating —
SyncAndForwardModeImpl.onEnter must reach the
+ // peer, so a bad promotion bounces back to STORE_AND_FORWARD.
+ logGroup.checkAndSetModeAndNotify(STORE_AND_FORWARD, SYNC_AND_FORWARD);
+
+ // The shared in-sync claim additionally requires no new files for the
ongoing round.
if (
- replicationLogTracker.getInProgressFiles().isEmpty()
- && replicationLogTracker.getNewFilesForRound(replicationLogTracker
-
.getReplicationShardDirectoryManager().getNextRound(getLastRoundProcessed())).isEmpty()
+ !replicationLogTracker.getNewFilesForRound(replicationLogTracker
+
.getReplicationShardDirectoryManager().getNextRound(getLastRoundProcessed())).isEmpty()
) {
- LOG.info("Processed all the replication log files for {}", logGroup);
- // if this RS is still in STORE_AND_FORWARD mode like when it didn't
process any file
- // move this RS to SYNC_AND_FORWARD
- logGroup.checkAndSetModeAndNotify(STORE_AND_FORWARD, SYNC_AND_FORWARD);
-
- if (syncUpdateTS <= EnvironmentEdgeManager.currentTimeMillis()) {
- try {
- long waitTime = logGroup.setHAGroupStatusToSync();
- if (waitTime != 0) {
- syncUpdateTS = EnvironmentEdgeManager.currentTimeMillis() +
waitTime;
- LOG.info("HAGroup {} will try to update HA state to sync at {}",
logGroup,
- syncUpdateTS);
- } else {
- LOG.info("HAGroup {} updated HA state to SYNC", logGroup);
- }
- } catch (Exception e) {
- LOG.info("Could not update status to sync for {}", logGroup, e);
+ LOG.info("New files present for the next round for {}, skipping sync
claim", logGroup);
+ return;
+ }
+ LOG.info("Processed all the replication log files for {}", logGroup);
+ if (syncUpdateTS <= EnvironmentEdgeManager.currentTimeMillis()) {
+ try {
+ long waitTime = logGroup.setHAGroupStatusToSync();
Review Comment:
It's pre-existing, not introduced in this PR. It also self-heals. When
onEnter bounces back, StoreAndForwardModeImpl.onEnter restarts the periodic
re-assertion task that rewrites ACTIVE_NOT_IN_SYNC every 0.7 *
zkSessionTimeout, so any transient false ACTIVE_IN_SYNC is overwritten within
one interval.
The residual risk is a narrow, self-correcting window (≤ ~`0.7 * zkTimeout`)
advertising ACTIVE_IN_SYNC against a dead peer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
