Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-27303 Unnecessary replication to secondary region replicas shou… #4707

Merged
merged 1 commit into from
Aug 24, 2022
Merged

HBASE-27303 Unnecessary replication to secondary region replicas shou… #4707

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For most cases the sync will succeeded, attach it before WAL sync can speed up the replication? No?

Copy link
Contributor Author

@comnetwork comnetwork Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 #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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we will complete the MVCC entry after calling this method.

return writeEntry;
} catch (IOException ioe) {
if (walKey.getWriteEntry() != null) {
Expand Down