[GitHub] [hbase] comnetwork commented on a diff in pull request #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872196855
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##
@@ -4142,15 +4147,58 @@ private static long getLongValue(final Cell cell)
throws DoNotRetryIOException {
@Override
public WriteEntry writeMiniBatchOperationsToMemStore(
- final MiniBatchOperationInProgress miniBatchOp, @Nullable
WriteEntry writeEntry)
- throws IOException {
+ final MiniBatchOperationInProgress miniBatchOp, @Nullable
WriteEntry writeEntry,
+ long now) throws IOException {
+ boolean newWriteEntry = false;
if (writeEntry == null) {
writeEntry = region.mvcc.begin();
+newWriteEntry = true;
}
super.writeMiniBatchOperationsToMemStore(miniBatchOp,
writeEntry.getWriteNumber());
+ if (newWriteEntry) {
Review Comment:
@Apache9 ，There are two cases:
case 1 is partial Mutations are Durability#SKIP_WAL , which is processed in
`HRegion.doWALAppend`
case 2 is all Mutations are Durability#SKIP_WAL, which is processed here.
--
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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872196855
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##
@@ -4142,15 +4147,58 @@ private static long getLongValue(final Cell cell)
throws DoNotRetryIOException {
@Override
public WriteEntry writeMiniBatchOperationsToMemStore(
- final MiniBatchOperationInProgress miniBatchOp, @Nullable
WriteEntry writeEntry)
- throws IOException {
+ final MiniBatchOperationInProgress miniBatchOp, @Nullable
WriteEntry writeEntry,
+ long now) throws IOException {
+ boolean newWriteEntry = false;
if (writeEntry == null) {
writeEntry = region.mvcc.begin();
+newWriteEntry = true;
}
super.writeMiniBatchOperationsToMemStore(miniBatchOp,
writeEntry.getWriteNumber());
+ if (newWriteEntry) {
Review Comment:
There are two cases:
case 1 is partial Mutations are Durability#SKIP_WAL , which is processed in
`HRegion.doWALAppend`
case 2 is all Mutations are Durability#SKIP_WAL, which is processed here.
--
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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872190752
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MiniBatchOperationInProgress.java:
##
@@ -182,4 +195,25 @@ public int getNumOfAppends() {
public void incrementNumOfAppends() {
this.numOfAppends += 1;
}
+
+ public void addSkipWALMutation(NonceKey nonceKey, Map>
columnFamilyToCells) {
+if (columnFamilyToCells == null) {
+ return;
+}
+if (this.nonceKeyToSkipWALMutations == null) {
+ this.nonceKeyToSkipWALMutations = new HashMap<>();
+}
+List>> skipWALMuations =
+ this.nonceKeyToSkipWALMutations.computeIfAbsent(nonceKey, (key) -> new
ArrayList<>());
Review Comment:
That code was removed.
--
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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872190471
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##
@@ -7947,6 +8000,58 @@ private WriteEntry doWALAppend(WALEdit walEdit,
Durability durability, List
batchOp,
+MiniBatchOperationInProgress miniBatchOp,
+NonceKey nonceKey, WALKeyImpl walKey, WALEdit walEdit, WriteEntry
writeEntry)
+throws IOException {
+if (!regionReplicationSink.isPresent()) {
+ return;
+}
+final WALEdit walEditToUse =
+ getWALEditForReplicateRegionReplica(batchOp, miniBatchOp, nonceKey,
walEdit);
+final ServerCall rpcCall =
RpcServer.getCurrentServerCallWithCellScanner().orElse(null);
+regionReplicationSink.ifPresent(sink ->
writeEntry.attachCompletionAction(() -> {
+ sink.add(walKey, walEditToUse, rpcCall);
+}));
+ }
+
+ /**
+ * Here is for HBASE-26993 case 1,partial {@link Mutation}s are {@link
Durability#SKIP_WAL}.In
+ * order to make the new framework for region replication could work for
SKIP_WAL, we must add the
+ * {@link Mutation} which {@link Mutation#getDurability} is {@link
Durability#SKIP_WAL} to the
+ * {@link WALEdit} used for replicating to region replica.
+ */
+ private WALEdit getWALEditForReplicateRegionReplica(BatchOperation
batchOp,
+MiniBatchOperationInProgress miniBatchOp,
+NonceKey nonceKey, WALEdit walEdit) throws IOException {
+/**
+ * Here there is no need to consider there are SKIP_WAL {@link Mutation}s
which are not covered
+ * by {@link HRegion#doWALAppend} because for primary region,only {@link
MutationBatchOperation}
+ * is used and {@link NonceKey} is all the same for {@link Mutation}s in
+ * {@link MutationBatchOperation}.
+ */
+List>> columnFamilyToCellsList =
miniBatchOp.getSkipMutations(nonceKey);
+if(columnFamilyToCellsList == null || columnFamilyToCellsList.isEmpty()) {
+ return walEdit;
+}
+
+/**
+ * Create a new WALEdit for replicating to region replica,and add the
{@link Mutation} which
+ * {@link Mutation#getDurability} is {@link Durability#SKIP_WAL}
+ */
+WALEdit newWALEdit = new WALEdit(walEdit);
+for (Map> columnFamilyToCells :
columnFamilyToCellsList) {
Review Comment:
@Apache9 , thank you very much for point out it , that may cause the
secondary replica inconsistent with primary. I have fixed it.
--
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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872189343
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##
@@ -7947,6 +8000,58 @@ private WriteEntry doWALAppend(WALEdit walEdit,
Durability durability, List
batchOp,
+MiniBatchOperationInProgress miniBatchOp,
+NonceKey nonceKey, WALKeyImpl walKey, WALEdit walEdit, WriteEntry
writeEntry)
+throws IOException {
+if (!regionReplicationSink.isPresent()) {
+ return;
+}
+final WALEdit walEditToUse =
+ getWALEditForReplicateRegionReplica(batchOp, miniBatchOp, nonceKey,
walEdit);
+final ServerCall rpcCall =
RpcServer.getCurrentServerCallWithCellScanner().orElse(null);
+regionReplicationSink.ifPresent(sink ->
writeEntry.attachCompletionAction(() -> {
+ sink.add(walKey, walEditToUse, rpcCall);
+}));
+ }
+
+ /**
+ * Here is for HBASE-26993 case 1,partial {@link Mutation}s are {@link
Durability#SKIP_WAL}.In
+ * order to make the new framework for region replication could work for
SKIP_WAL, we must add the
+ * {@link Mutation} which {@link Mutation#getDurability} is {@link
Durability#SKIP_WAL} to the
+ * {@link WALEdit} used for replicating to region replica.
+ */
+ private WALEdit getWALEditForReplicateRegionReplica(BatchOperation
batchOp,
+MiniBatchOperationInProgress miniBatchOp,
+NonceKey nonceKey, WALEdit walEdit) throws IOException {
+/**
+ * Here there is no need to consider there are SKIP_WAL {@link Mutation}s
which are not covered
Review Comment:
@Apache9 , yes , we no need to pass in the nonceKey, I have removed it and
add a
`assert batchOp instanceof MutationBatchOperation`
--
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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392: URL: https://github.com/apache/hbase/pull/4392#discussion_r872186781 ## hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java: ## @@ -3372,8 +3372,8 @@ public abstract void prepareMiniBatchOperations( * Write mini-batch operations to MemStore */ public abstract WriteEntry writeMiniBatchOperationsToMemStore( - final MiniBatchOperationInProgress miniBatchOp, final WriteEntry writeEntry) - throws IOException; + final MiniBatchOperationInProgress miniBatchOp, final WriteEntry writeEntry, + long now) throws IOException; Review Comment: Because we may need the `now` to create WALKeyImpl, so here I add a `now` parameter. -- 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 #4392: HBASE-26993 Make the new framework for region replication could work …
comnetwork commented on code in PR #4392:
URL: https://github.com/apache/hbase/pull/4392#discussion_r872183114
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##
@@ -3606,24 +3610,25 @@ protected MiniBatchOperationInProgress
createMiniBatch(final int lastI
@Override
public boolean visit(int index) throws IOException {
Mutation m = getMutation(index);
+ // the batch may contain multiple nonce keys (replay case). If so,
write WALEdit for each.
+ // Given how nonce keys are originally written, these should be
contiguous.
+ // They don't have to be, it will still work, just write more
WALEdits than needed.
+ long nonceGroup = getNonceGroup(index);
+ long nonce = getNonce(index);
// we use durability of the original mutation for the mutation
passed by CP.
if (region.getEffectiveDurability(m.getDurability()) ==
Durability.SKIP_WAL) {
region.recordMutationWithoutWal(m.getFamilyCellMap());
+miniBatchOp.addSkipWALMutation(new NonceKey(nonceGroup, nonce),
familyCellMaps[index]);
Review Comment:
@Apache9 , yes, thank you very much for reminding, I have fixed.
--
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
