[GitHub] [hbase] Apache9 commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…

2022-08-23 Thread GitBox 


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


##
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:
   OK, we will complete the MVCC entry after calling this method.



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

[GitHub] [hbase] Apache9 commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…

2022-08-15 Thread GitBox 


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


##
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:
   For most cases the sync will succeeded, attach it before WAL sync can speed 
up the replication? No?



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