-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24757 : ReplicationSink should limit row count in batch mutation based on hbase.rpc.rows.warning.threshold #2127
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.
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
Conversation
… based on hbase.rpc.rows.warning.threshold
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| * Default value of {@link #BATCH_ROWS_THRESHOLD_NAME} | ||
| */ | ||
| public static final int BATCH_ROWS_THRESHOLD_DEFAULT = 5000; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the move? HConstants is often considered an anti-pattern; better to have constants beside where they are used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want RsRpcServices as well as ReplicationSink both to use these constants. Would you still recommend having duplicate copies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Two very disparate contexts. This is probably only case for HConstants.
| * @throws IOException thrown when HDFS goes bad or bad file name | ||
| */ | ||
| public ReplicationSink(Configuration conf, Stoppable stopper) | ||
| public ReplicationSink(Configuration conf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the Stoppable unused? Usually its a chain for a Service to pull on when it meets a condition it can't deal with... one that is so bad it wants to stop the process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree for stoppable usage, but here it is anyways unused :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
| batchRows = Collections.singletonList(rows); | ||
| } | ||
| futures.addAll(batchRows.stream().map(table::batchAll).collect(Collectors.toList())); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the edits that are beyond the limit, they get handled in next batch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, they are handled in next batch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UT testLargeEditsPutDelete() as part of this PR will always ensure that this is handled.
|
🎊 +1 overall
This message was automatically generated. |
saintstack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…n based on hbase.rpc.rows.warning.threshold Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (#2136) Closes #2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (#2137) Closes #2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (#2139) Closes #2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (#2146) Closes #2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (apache#2136) Closes apache#2127 Signed-off-by: stack <[email protected]>
…n based on hbase.rpc.rows.warning.threshold (apache#2139) Closes apache#2127 Signed-off-by: stack <[email protected]> (cherry picked from commit 603d2b6) Change-Id: I27416e5ee65bc2cbcb776e7f83487cb2c06e147a
No description provided.