You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/02/02 09:19:32 UTC
[GitHub] [hbase] brfrn169 opened a new pull request #1114: HBASE-23146
Support CheckAndMutate with multi conditions
brfrn169 opened a new pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r379977251
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9 Thank you very much for the reply.
```
we are still fighting with 2019-nCov so... Hope Japan will be safe...
```
Oh okay.. I hope you are well... I'm okay now, but 2019-nCov is spreading little by little in Japan actually.
```
I still prefer that we introduce a new method which accepts a row and filter as parameters to construct a Builder, this is more natural. Enforcing usage by method signature is better than javadoc.
```
Thank you for commenting on this. I will change the API as you mentioned.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 merged pull request #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 merged pull request #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r379951779
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9 Could you please reply this when you get a chance?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-588228889
It looks like the test failures in the last QA is unrelated to this patch. I ran the failed tests locally and they were successful.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r379952183
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
Actually, this feature is needed for our use case. If we can, I want to include it in hbase-2.3.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r384181018
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -289,6 +290,60 @@ default CheckAndMutateBuilder ifEquals(byte[] value) {
CompletableFuture<Boolean> thenMutate(RowMutations mutation);
}
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateWithFilterBuilder} to construct your request and then
+ * execute it. This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row, filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateWithFilterBuilder checkAndMutate(byte[] row, Filter filter);
+
+ /**
+ * A helper class for sending checkAndMutate request with a filter.
+ */
+ interface CheckAndMutateWithFilterBuilder {
+
+ /**
+ * @param timeRange time range to check.
+ */
+ CheckAndMutateWithFilterBuilder timeRange(TimeRange timeRange);
Review comment:
Thank you very much for your review and approval!
> Maybe we could have a CheckAndMutateBuilderBase to palce these methods.
Yes, we can have CheckAndMutateBuilderBase to place the methods.
> Anyway, can be a follow issue as I do not think the change will break compatibility.
Sure. Maybe we can do it at a next time when we add another type of checkAndMutate.
Will commit this. Thank you very much again!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373894914
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
The API here is a bit strange. We introduce the above family & row method is because that family and row both can not be null, but here since we allow matching multiple qualifiers acrossing different families, we can accept null families as we can pass it through the filter implementation? So here, either we deprecated the old checkAndMutate(family, row) method and introduce a new family method, or we introduce a new checkAndMutate(row, filter)?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r379977251
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9 Thank you very much for the reply.
> we are still fighting with 2019-nCov so... Hope Japan will be safe...
Oh okay.. I hope you are well... I'm okay now, but 2019-nCov is spreading little by little in Japan actually.
> I still prefer that we introduce a new method which accepts a row and filter as parameters to construct a Builder, this is more natural. Enforcing usage by method signature is better than javadoc.
Thank you for commenting on this. I will change the API as you mentioned.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r383734231
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -289,6 +290,60 @@ default CheckAndMutateBuilder ifEquals(byte[] value) {
CompletableFuture<Boolean> thenMutate(RowMutations mutation);
}
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateWithFilterBuilder} to construct your request and then
+ * execute it. This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row, filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateWithFilterBuilder checkAndMutate(byte[] row, Filter filter);
+
+ /**
+ * A helper class for sending checkAndMutate request with a filter.
+ */
+ interface CheckAndMutateWithFilterBuilder {
+
+ /**
+ * @param timeRange time range to check.
+ */
+ CheckAndMutateWithFilterBuilder timeRange(TimeRange timeRange);
Review comment:
I think these 4 methods are also in the CheckAndMutateBuilder? Maybe we could have a CheckAndMutateBuilderBase to palce these methods. Anyway, can be a follow issue as I do not think the change will break compatibility.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r379956417
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
Sorry, we are still fighting with 2019-nCov so... Hope Japan will be safe...
I still prefer that we introduce a new method which accepts a row and filter as parameters to construct a Builder, this is more natural. Enforcing usage by method signature is better than javadoc.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r376649523
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9
One of the reasons why I decided the design of the new API here is that I wanted to keep the compatibility of the existing API. This patch also involves coprocessor changes, and I didn't want to change the existing methods and I just wanted to add new methods.
If you still think the API is strange, I can change it.
Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373895018
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java
##########
@@ -921,22 +924,28 @@ public static RegionSpecifier buildRegionSpecifier(
* @param row
* @param family
* @param qualifier
- * @param comparator
- * @param compareType
+ * @param op
Review comment:
Ditto.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 edited a comment on issue #1114: HBASE-23146
Support CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 edited a comment on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-588228889
It looks like the test failures in the last QA are unrelated to this patch. I ran the failed tests locally and they were successful.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-581382054
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 52s | master passed |
| +1 :green_heart: | compile | 3m 39s | master passed |
| +1 :green_heart: | checkstyle | 3m 13s | master passed |
| +1 :green_heart: | shadedjars | 5m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 21s | master passed |
| +0 :ok: | spotbugs | 1m 37s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 12m 22s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 36s | the patch passed |
| +1 :green_heart: | compile | 3m 43s | the patch passed |
| +1 :green_heart: | cc | 3m 43s | the patch passed |
| +1 :green_heart: | javac | 3m 43s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 9s | The patch passed checkstyle in hbase-protocol-shaded |
| +1 :green_heart: | checkstyle | 0m 11s | The patch passed checkstyle in hbase-protocol |
| +1 :green_heart: | checkstyle | 0m 33s | hbase-client: The patch generated 0 new + 85 unchanged - 26 fixed = 85 total (was 111) |
| +1 :green_heart: | checkstyle | 1m 18s | hbase-server: The patch generated 0 new + 449 unchanged - 10 fixed = 449 total (was 459) |
| +1 :green_heart: | checkstyle | 0m 41s | The patch passed checkstyle in hbase-thrift |
| +1 :green_heart: | checkstyle | 0m 15s | The patch passed checkstyle in hbase-rest |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 39s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | hbaseprotoc | 3m 22s | the patch passed |
| +1 :green_heart: | javadoc | 2m 23s | the patch passed |
| +1 :green_heart: | findbugs | 13m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 45s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 0m 27s | hbase-protocol in the patch passed. |
| +1 :green_heart: | unit | 2m 3s | hbase-client in the patch passed. |
| -1 :x: | unit | 161m 27s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 3m 47s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 5m 12s | hbase-rest in the patch passed. |
| +1 :green_heart: | asflicense | 2m 36s | The patch does not generate ASF License warnings. |
| | | 273m 14s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.quotas.TestClusterScopeQuotaThrottle |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1114 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux d30055d5e41c 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1114/out/precommit/personality/provided.sh |
| git revision | master / ab0039974d |
| Default Java | 1.8.0_181 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/2/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/2/testReport/ |
| Max. process+thread count | 4821 (vs. ulimit of 10000) |
| modules | C: hbase-protocol-shaded hbase-protocol hbase-client hbase-server hbase-thrift hbase-rest U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r376649523
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9
One of the reason why I decided the design of the new API here is that I wanted to keep the compatibility of the existing API. This patch also involves coprocessor changes, and I didn't want to change the existing methods and I just wanted to add new methods.
If you still think the API is strange, I can change it.
Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373894999
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java
##########
@@ -212,54 +214,55 @@ public static MutateRequest buildMutateRequest(
* @param row
* @param family
* @param qualifier
- * @param comparator
- * @param compareType
+ * @param op
+ * @param value
* @param delete
* @return a mutate request
* @throws IOException
*/
public static MutateRequest buildMutateRequest(
- final byte[] regionName, final byte[] row, final byte[] family,
- final byte [] qualifier, final ByteArrayComparable comparator,
- final CompareType compareType, TimeRange timeRange, final Delete delete) throws IOException {
- return buildMutateRequest(regionName, row, family, qualifier, comparator, compareType, timeRange
- , delete, MutationType.DELETE);
+ final byte[] regionName, final byte[] row, final byte[] family,
+ final byte [] qualifier, final CompareOperator op, final byte[] value, final Filter filter,
+ final TimeRange timeRange, final Delete delete) throws IOException {
+ return buildMutateRequest(regionName, row, family, qualifier, op, value, filter, timeRange,
+ delete, MutationType.DELETE);
}
public static MutateRequest buildMutateRequest(final byte[] regionName, final byte[] row,
- final byte[] family, final byte[] qualifier, final ByteArrayComparable comparator,
- final CompareType compareType, TimeRange timeRange, final Mutation mutation,
+ final byte[] family, final byte[] qualifier, final CompareOperator op, final byte[] value,
+ final Filter filter, final TimeRange timeRange, final Mutation mutation,
final MutationType type) throws IOException {
return MutateRequest.newBuilder()
.setRegion(buildRegionSpecifier(RegionSpecifierType.REGION_NAME, regionName))
.setMutation(ProtobufUtil.toMutation(type, mutation))
- .setCondition(buildCondition(row, family, qualifier, comparator, compareType, timeRange))
+ .setCondition(buildCondition(row, family, qualifier, op, value, filter, timeRange))
.build();
}
+
/**
* Create a protocol buffer MutateRequest for conditioned row mutations
*
* @param regionName
* @param row
* @param family
* @param qualifier
- * @param comparator
- * @param compareType
+ * @param op
Review comment:
Ditto.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-590649505
It looks like Duo is very busy.
@mistyhacks @joshelser Could you please review it when you get a chance? I would like to include it in hbase-2.3 if possible.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-587946687
I just force-pushed the new patch with changing the new API. I added a new method which accepts a row and filter as parameters to construct a Builder.
@Apache9 Could you please review it when you get a chance?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on issue #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-581389004
The test failures in the last QA are not related to the patch. I ran the failed tests locally and they were successful.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-581138604
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 18s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 1s | The patch appears to include 8 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 51s | master passed |
| +1 :green_heart: | compile | 3m 36s | master passed |
| +1 :green_heart: | checkstyle | 3m 8s | master passed |
| +1 :green_heart: | shadedjars | 5m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 20s | master passed |
| +0 :ok: | spotbugs | 1m 37s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 12m 29s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 34s | the patch passed |
| +1 :green_heart: | compile | 3m 39s | the patch passed |
| +1 :green_heart: | cc | 3m 39s | the patch passed |
| +1 :green_heart: | javac | 3m 39s | the patch passed |
| -1 :x: | checkstyle | 0m 32s | hbase-client: The patch generated 9 new + 103 unchanged - 8 fixed = 112 total (was 111) |
| -1 :x: | checkstyle | 1m 20s | hbase-server: The patch generated 11 new + 455 unchanged - 4 fixed = 466 total (was 459) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 36s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | hbaseprotoc | 3m 30s | the patch passed |
| +1 :green_heart: | javadoc | 2m 17s | the patch passed |
| +1 :green_heart: | findbugs | 13m 11s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 44s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 0m 28s | hbase-protocol in the patch passed. |
| +1 :green_heart: | unit | 2m 0s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 158m 7s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 3m 45s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 5m 16s | hbase-rest in the patch passed. |
| +1 :green_heart: | asflicense | 2m 39s | The patch does not generate ASF License warnings. |
| | | 273m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1114 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux 9ff26235c8c8 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1114/out/precommit/personality/provided.sh |
| git revision | master / 5a3ad6f08f |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/1/artifact/out/diff-checkstyle-hbase-client.txt |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/1/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/1/testReport/ |
| Max. process+thread count | 4744 (vs. ulimit of 10000) |
| modules | C: hbase-protocol-shaded hbase-protocol hbase-client hbase-server hbase-thrift hbase-rest U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-588005398
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 8m 10s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 50s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 7m 29s | master passed |
| +1 :green_heart: | compile | 4m 22s | master passed |
| +1 :green_heart: | checkstyle | 3m 18s | master passed |
| +1 :green_heart: | shadedjars | 5m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 28s | master passed |
| +0 :ok: | spotbugs | 1m 43s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 13m 20s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 46s | the patch passed |
| +1 :green_heart: | compile | 3m 52s | the patch passed |
| +1 :green_heart: | cc | 3m 52s | the patch passed |
| +1 :green_heart: | javac | 3m 52s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 11s | The patch passed checkstyle in hbase-protocol-shaded |
| +1 :green_heart: | checkstyle | 0m 9s | The patch passed checkstyle in hbase-protocol |
| +1 :green_heart: | checkstyle | 0m 34s | hbase-client: The patch generated 0 new + 85 unchanged - 26 fixed = 85 total (was 111) |
| -1 :x: | checkstyle | 1m 22s | hbase-server: The patch generated 1 new + 449 unchanged - 10 fixed = 450 total (was 459) |
| +1 :green_heart: | checkstyle | 0m 47s | The patch passed checkstyle in hbase-thrift |
| +1 :green_heart: | checkstyle | 0m 17s | The patch passed checkstyle in hbase-rest |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 55s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | hbaseprotoc | 3m 26s | the patch passed |
| +1 :green_heart: | javadoc | 2m 20s | the patch passed |
| +1 :green_heart: | findbugs | 13m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 43s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 0m 27s | hbase-protocol in the patch passed. |
| +1 :green_heart: | unit | 1m 0s | hbase-client in the patch passed. |
| -1 :x: | unit | 96m 49s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 3m 16s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 4m 16s | hbase-rest in the patch passed. |
| +1 :green_heart: | asflicense | 2m 18s | The patch does not generate ASF License warnings. |
| | | 217m 46s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.replication.TestNamespaceReplication |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1114 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux 016c0766d222 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1114/out/precommit/personality/provided.sh |
| git revision | master / c4fb1cd234 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/3/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/3/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/3/testReport/ |
| Max. process+thread count | 6837 (vs. ulimit of 10000) |
| modules | C: hbase-protocol-shaded hbase-protocol hbase-client hbase-server hbase-thrift hbase-rest U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/3/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1114: HBASE-23146 Support
CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-588114858
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 50s | master passed |
| +1 :green_heart: | compile | 3m 39s | master passed |
| +1 :green_heart: | checkstyle | 3m 11s | master passed |
| +1 :green_heart: | shadedjars | 5m 5s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 16s | master passed |
| +0 :ok: | spotbugs | 1m 46s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 12m 30s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 52s | the patch passed |
| +1 :green_heart: | compile | 3m 45s | the patch passed |
| +1 :green_heart: | cc | 3m 45s | the patch passed |
| +1 :green_heart: | javac | 3m 45s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 10s | The patch passed checkstyle in hbase-protocol-shaded |
| +1 :green_heart: | checkstyle | 0m 9s | The patch passed checkstyle in hbase-protocol |
| +1 :green_heart: | checkstyle | 0m 33s | hbase-client: The patch generated 0 new + 85 unchanged - 26 fixed = 85 total (was 111) |
| +1 :green_heart: | checkstyle | 1m 21s | hbase-server: The patch generated 0 new + 449 unchanged - 10 fixed = 449 total (was 459) |
| +1 :green_heart: | checkstyle | 0m 41s | The patch passed checkstyle in hbase-thrift |
| +1 :green_heart: | checkstyle | 0m 15s | The patch passed checkstyle in hbase-rest |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 27s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | hbaseprotoc | 3m 24s | the patch passed |
| +1 :green_heart: | javadoc | 2m 16s | the patch passed |
| +1 :green_heart: | findbugs | 13m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 43s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 0m 27s | hbase-protocol in the patch passed. |
| +1 :green_heart: | unit | 1m 0s | hbase-client in the patch passed. |
| -1 :x: | unit | 95m 3s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 2m 50s | hbase-thrift in the patch passed. |
| +1 :green_heart: | unit | 3m 53s | hbase-rest in the patch passed. |
| +1 :green_heart: | asflicense | 2m 21s | The patch does not generate ASF License warnings. |
| | | 203m 24s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.util.TestHBaseFsckCleanReplicationBarriers |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1114 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux 4757529ac9ad 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1114/out/precommit/personality/provided.sh |
| git revision | master / be4b6d6824 |
| Default Java | 1.8.0_181 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/4/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/4/testReport/ |
| Max. process+thread count | 6247 (vs. ulimit of 10000) |
| modules | C: hbase-protocol-shaded hbase-protocol hbase-client hbase-server hbase-thrift hbase-rest U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1114/4/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373902670
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9 Thank you for reviewing this!
> but here since we allow matching multiple qualifiers acrossing different families, we can accept null families as we can pass it through the filter implementation?
Yes. With this new feature, we can specify multiple conditions across different families by setting a filter. So the family can be null.
My intention was if we want to specify a filter by ifMatches(filter), we can use checkAndMutate(row). And if we want to specify a qualifier and a condition by qualifier()/ifNotExists()/ifEquals()/ifMatches(op, value), we can use checkAndMutate(row, family) in the existing way.
And, if we call ifMatches(filter) after calling checkAndMutate(row, family), it will throw UnsupportedOperationException with the message "Please use checkAndMutate(row)":
```
table.checkAndMutate(row, family).ifMatches(filter).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row)
```
And vice versa:
```
table.checkAndMutate(row).qualifier(qualifier).ifMatches(op, value).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row, family)
```
Please see the following test for this:
https://github.com/brfrn169/hbase/blob/HBASE-23146/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java#L380-L416
I think, you are right, it might be a little bit confusing but I'm still feeling it's okay. What do you think?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache9 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373894959
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java
##########
@@ -191,18 +193,18 @@ public static GetRequest buildGetRequest(final byte[] regionName,
* @param row
* @param family
* @param qualifier
- * @param comparator
- * @param compareType
+ * @param op
Review comment:
Just remove all the empty param javadoc?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r376072388
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
Ping @Apache9
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 edited a comment on issue #1114: HBASE-23146
Support CheckAndMutate with multiple conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 edited a comment on issue #1114: HBASE-23146 Support CheckAndMutate with multiple conditions
URL: https://github.com/apache/hbase/pull/1114#issuecomment-590649505
It looks like Duo is very busy.
@saintstack @joshelser Could you please review it when you get a chance? I would like to include it in hbase-2.3 if possible.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #1114:
HBASE-23146 Support CheckAndMutate with multi conditions
Posted by GitBox <gi...@apache.org>.
brfrn169 commented on a change in pull request #1114: HBASE-23146 Support CheckAndMutate with multi conditions
URL: https://github.com/apache/hbase/pull/1114#discussion_r373902670
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncTable.java
##########
@@ -233,6 +234,28 @@
*/
CheckAndMutateBuilder checkAndMutate(byte[] row, byte[] family);
+ /**
+ * Atomically checks if a row matches the specified filter. If it does, it adds the
+ * Put/Delete/RowMutations.
+ * <p>
+ * Use the returned {@link CheckAndMutateBuilder} to construct your request and then execute it.
+ * This is a fluent style API, the code is like:
+ *
+ * <pre>
+ * <code>
+ * table.checkAndMutate(row).ifMatches(filter).thenPut(put)
+ * .thenAccept(succ -> {
+ * if (succ) {
+ * System.out.println("Check and put succeeded");
+ * } else {
+ * System.out.println("Check and put failed");
+ * }
+ * });
+ * </code>
+ * </pre>
+ */
+ CheckAndMutateBuilder checkAndMutate(byte[] row);
Review comment:
@Apache9 Thank you for reviewing this!
> but here since we allow matching multiple qualifiers acrossing different families, we can accept null families as we can pass it through the filter implementation?
Yes. With this new feature, we can specify multiple conditions across different families by setting a filter. So the family can be null.
My intention was if we want to specify a filter by ifMatches(filter), we can use checkAndMutate(row). And if we want to specify a qualifier and a condition by qualifier()/ifNotExists()/ifEquals()/ifMatches(op, value), we can use checkAndMutate(row, family) in the existing way.
And, if we call ifMatches(filter) after calling checkAndMutate(row, family) wrongly, it will throw UnsupportedOperationException with the message "Please use checkAndMutate(row)":
```
table.checkAndMutate(row, family).ifMatches(filter).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row)
```
And vice versa:
```
table.checkAndMutate(row).qualifier(qualifier).ifMatches(op, value).thenPut(put); // UnsupportedOperationException: Please use checkAndMutate(row, family)
```
Please see the following test for this:
https://github.com/brfrn169/hbase/blob/HBASE-23146/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java#L380-L416
I think, you are right, it might be a little bit confusing but I'm still feeling it's okay. What do you think?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services