Copilot commented on code in PR #8584:
URL: https://github.com/apache/hbase/pull/8584#discussion_r3918125748


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java:
##########
@@ -1092,6 +1092,24 @@ public void removeRegion(final RegionInfo regionInfo) {
     flushedSequenceIdByRegion.remove(encodedName);
   }
 
+  /**
+   * Called on region OPEN to seed {@link #flushedSequenceIdByRegion} with the 
region's
+   * {@code openSeqNum}. Without this, the entry stays absent until the 
hosting server's next
+   * heartbeat, so {@link #getLastFlushedSequenceId} returns {@link 
HConstants#NO_SEQNUM} and
+   * WALSplitter conservatively treats already-durable edits as unflushed - 
producing orphaned
+   * recovered.edits when the source server crashes soon after a drain-move. 
Uses {@code merge} with
+   * {@link Math#max} so a heartbeat-supplied value (which may reflect flushes 
after open) is never
+   * regressed - and, unlike {@code putIfAbsent}, a stale-low prior value is 
lifted to
+   * {@code openSeqNum}. Safe because at OPEN a region cannot have flushed 
past its own
+   * {@code openSeqNum}. See HBASE-30335.
+   */
+  public void reportRegionOpen(final RegionInfo regionInfo, final long 
openSeqNum) {
+    if (openSeqNum < 0) { // NO_SEQNUM == -1
+      return;
+    }
+    flushedSequenceIdByRegion.merge(regionInfo.getEncodedNameAsBytes(), 
openSeqNum, Math::max);

Review Comment:
   This `merge` is atomic, but the heartbeat writer is not: 
`updateLastFlushedSequenceIds` reads the current value at line 292 and later 
performs a conditional `put` at line 300. An in-flight stale heartbeat can 
therefore read an absent entry, this call can install `openSeqNum`, and then 
the heartbeat can overwrite it with an older completed sequence ID, violating 
the monotonic watermark guarantee and potentially restoring the WAL-splitting 
failure. Please make the heartbeat update use the same atomic max operation and 
cover this interleaving in the test.



-- 
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]

Reply via email to