comnetwork commented on code in PR #4707:
URL: https://github.com/apache/hbase/pull/4707#discussion_r952689280


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -8040,11 +8040,17 @@ private WriteEntry doWALAppend(WALEdit walEdit, 
BatchOperation<?> batchOp,
     try {
       long txid = this.wal.appendData(this.getRegionInfo(), walKey, walEdit);
       WriteEntry writeEntry = walKey.getWriteEntry();
-      this.attachRegionReplicationInWALAppend(batchOp, miniBatchOp, walKey, 
walEdit, writeEntry);
       // Call sync on our edit.
       if (txid != 0) {
         sync(txid, batchOp.durability);
       }
+      /**
+       * If above {@link HRegion#sync} throws Exception, the RegionServer 
should be aborted and
+       * following {@link BatchOperation#writeMiniBatchOperationsToMemStore} 
will not be executed,
+       * so there is no need to replicate to secondary replica, for this 
reason here we attach the
+       * region replication action after the {@link HRegion#sync} is 
successful.
+       */
+      this.attachRegionReplicationInWALAppend(batchOp, miniBatchOp, walKey, 
walEdit, writeEntry);

Review Comment:
   @Apache9 , the replication is caused by `MVCC.complete`, so seems that 
attaching it before WAL sync could not speed up the replication, by the 
previous discussion in HBASE-27223 and 
https://github.com/apache/hbase/pull/4633, `sync` may throws 
Exception(especially for FSHLog, it has no retry and may throws any exception 
thrown by `ProtobufLogWriter.append` and `ProtobufLogWriter.sync`,HBASE-27231 
try to solve this problem), attaching the  region replication after sync 
success try to avoid data inconsistent between primary and secondary replicas.



-- 
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: issues-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to