brfrn169 commented on a change in pull request #2094:
URL: https://github.com/apache/hbase/pull/2094#discussion_r459957143
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4272,43 +4275,96 @@ protected Durability getEffectiveDurability(Durability
d) {
}
@Override
+ @Deprecated
public boolean checkAndMutate(byte[] row, byte[] family, byte[] qualifier,
CompareOperator op,
ByteArrayComparable comparator, TimeRange timeRange, Mutation mutation)
throws IOException {
- return doCheckAndRowMutate(row, family, qualifier, op, comparator, null,
timeRange, null,
- mutation);
+ CheckAndMutate.Builder builder = CheckAndMutate.newBuilder(row)
+ .ifMatches(family, qualifier, op,
comparator.getValue()).timeRange(timeRange);
+ try {
+ if (mutation instanceof Put) {
+ return checkAndMutate(builder.build((Put) mutation)).isSuccess();
+ } else if (mutation instanceof Delete) {
+ return checkAndMutate(builder.build((Delete) mutation)).isSuccess();
+ } else {
+ throw new DoNotRetryIOException(
+ "Unsupported mutate type: " +
mutation.getClass().getSimpleName().toUpperCase());
+ }
+ } catch (IllegalArgumentException e) {
Review comment:
CheckAndMutate.Builder.build() calls preCheck() internally and it can
throw IllegalArgumentException if the parameters are invalid:
https://github.com/apache/hbase/blob/09e7ccd42dc0b41241c193e27a7c24e53c8408d4/hbase-client/src/main/java/org/apache/hadoop/hbase/client/CheckAndMutate.java#L141-L151
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4335,32 +4391,14 @@ private boolean doCheckAndRowMutate(byte[] row, byte[]
family, byte[] qualifier,
checkRow(row, "doCheckAndRowMutate");
RowLock rowLock = getRowLockInternal(get.getRow(), false, null);
try {
- if (mutation != null && this.getCoprocessorHost() != null) {
- // Call coprocessor.
- Boolean processed = null;
- if (mutation instanceof Put) {
- if (filter != null) {
- processed = this.getCoprocessorHost()
Review comment:
The old coprocessor methods will be called after this change because the
default implementations of the new coprocessor methods call the old methods for
backward compatibility:
RegionObserver#preCheckAndMutate:
https://github.com/brfrn169/hbase/blob/HBASE-24680/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java#L837-L865
RegionObserver#preCheckAndMutateAfterRowLock:
https://github.com/brfrn169/hbase/blob/HBASE-24680/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java#L885-L914
RegionObserver#postCheckAndMutate:
https://github.com/brfrn169/hbase/blob/HBASE-24680/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java#L927-L955
This change doesn't break the existing tests for the old coprocessor methods.
##########
File path:
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java
##########
@@ -86,6 +86,12 @@
*/
void updateCheckAndPut(long t);
+ /**
+ * Update checkAndMutate histogram
+ * @param t time it took
+ */
+ void updateCheckAndMutate(long t);
Review comment:
Yes. This change keeps the existing metrics for checkAndPut and
checkAndDelete for backward compatibility. This change doesn't break the
existing tests for the metrics for checkAndPut and checkAndDelete.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
