[GitHub] [hbase] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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 fired 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` or `ProtobufLogWriter.sync` ,and it may
not abort the regionServer, HBASE-27231 try to solve this problem), attaching
the region replication after sync success could try to avoid data inconsistent
between primary and secondary replicas,and if `WAL.sync `throws exception, we
have no need to replicate to secondary region replicas whether RegionServer is
aborted or not.
--
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] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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 fired 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` or `ProtobufLogWriter.sync` ,and it may
not abort the regionServer, HBASE-27231 try to solve this problem), attaching
the region replication after sync success could try to avoid data inconsistent
between primary and secondary replicas,and if `WAL.sync `throws exception, we
have no need to replicate to secondary region replicas whether RegionServer is
aborted or not.
--
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] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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 fired 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` or `ProtobufLogWriter.sync` ,and
it may not abort the regionServer, HBASE-27231 try to solve this problem),
attaching the region replication after sync success could 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
[GitHub] [hbase] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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` or `ProtobufLogWriter.sync` ,and
it may not abort the regionServer, HBASE-27231 try to solve this problem),
attaching the region replication after sync success could 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
[GitHub] [hbase] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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
[GitHub] [hbase] comnetwork commented on a diff in pull request #4707: HBASE-27303 Unnecessary replication to secondary region replicas shou…
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
