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/11/18 10:14:30 UTC

[GitHub] [hbase] thangTang opened a new pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

thangTang opened a new pull request #2674:
URL: https://github.com/apache/hbase/pull/2674


   


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-731212305


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 39s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 52s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 11s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 36s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 36s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m  1s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 20s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 150m 25s |  hbase-server in the patch passed.  |
   |  |   | 183m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c3e6fb1e49f3 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / eca904e0fb |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/testReport/ |
   | Max. process+thread count | 4334 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] virajjasani commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730230041


   @thangTang can you please also write a unit test with the same scenarios that you mentioned over Jira? i.e 
   startRow="112", rowPrefixFilter "11" and Scan can contain "111". 
   Although you commented, but having clear unit test will be really good reference.
   Thanks for identifying the issue.


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-729599030


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   1m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 15s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   1m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  36m  2s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux b2157df68cb5 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9419c78d26 |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-731141056


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  6s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 40s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 35s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 58s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m  9s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 27s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux c9806ae9b494 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / eca904e0fb |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] thangTang commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730248807


   > @thangTang can you please also write a unit test with the same scenarios that you mentioned over Jira? i.e
   > startRow="112", rowPrefixFilter "11" and Scan can contain "111".
   > Although you commented, but having clear unit test will be really good reference.
   > Thanks for identifying the issue.
   
   I'd like to, but should i write the UT in the patch? I think it would be failure and the CI -1.


----------------------------------------------------------------
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



[GitHub] [hbase] thangTang commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r526559462



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,15 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through other methods,

Review comment:
       Tks for comment, updated.




----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-732792234


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 36s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 38s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 45s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 23s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 48s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  5s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 148m 31s |  hbase-server in the patch passed.  |
   |  |   | 182m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux d2fd9b013a4a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1cd8f3cf94 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/testReport/ |
   | Max. process+thread count | 4390 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] huaxiangsun commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r528837177



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       +1 to remove. Sorry, it is just I did not know the alternative way to archive the goal.




----------------------------------------------------------------
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



[GitHub] [hbase] virajjasani commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r528852663



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       Yeah, this is a tough one. Ideally we can provide the link in comment but the class here is IA.Private.




----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-729592865


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 50s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 33s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 28s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 30s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  1s |  hbase-client in the patch passed.  |
   |  |   |  24m 49s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 09b3f8ef9dae 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9419c78d26 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/testReport/ |
   | Max. process+thread count | 346 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] thangTang commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r526559665



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,15 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through other methods,

Review comment:
       > Can you provide more details about the other methods?
   
   Tks for comment, updated.




----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-732720759


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   7m 46s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 32s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 56s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m  9s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 27s |  The patch does not generate ASF License warnings.  |
   |  |   |  49m 45s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 6fc3aedf8d48 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1cd8f3cf94 |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] thangTang commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r527632665



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       > `ClientUtil` is IA.Private, hence not really recommended for direct use by downstreamers.
   
   @huaxiangsun suggest me to give more detail, so i put this link. What i mean is just user can do this things by themself. Should i remove this link?




----------------------------------------------------------------
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



[GitHub] [hbase] virajjasani commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r527669124



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       I think we can remove just this statement: 
   ```
      *   Users can compose this logic externally through
      *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
      *   so there is no need to implement it inside.
   ```
   because `ClientUtil` is IA.Private, hence we should not recommend it in Javadoc.
   @huaxiangsun Thought?




----------------------------------------------------------------
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



[GitHub] [hbase] virajjasani closed pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani closed pull request #2674:
URL: https://github.com/apache/hbase/pull/2674


   


----------------------------------------------------------------
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



[GitHub] [hbase] huaxiangsun commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730134700


   Looks good to me, but let's wait for what people say from dev mailing list.


----------------------------------------------------------------
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



[GitHub] [hbase] virajjasani commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r526688894



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.

Review comment:
       nit: `scene` can be replaced by `scenario`

##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       `ClientUtil` is IA.Private, hence not really recommended for direct use by downstreamers.




----------------------------------------------------------------
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



[GitHub] [hbase] huaxiangsun commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r526598878



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,15 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through other methods,

Review comment:
       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



[GitHub] [hbase] infraio commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r528438994



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through
+   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
+   *   so there is no need to implement it inside.

Review comment:
       >    *   Users can compose this logic externally through
   >   *   {@link ClientUtil#calculateTheClosestNextRowKeyForPrefix(byte[])},
   >   *   so there is no need to implement it inside.
   
   +1 for remove these comment.
   




----------------------------------------------------------------
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



[GitHub] [hbase] huaxiangsun commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r526308915



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,15 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.
+   *   Users can compose this logic externally through other methods,

Review comment:
       Can you provide more details about the other methods?




----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-729593688


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 21s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 39s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  8s |  hbase-client in the patch passed.  |
   |  |   |  26m 14s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux be363fb55e95 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9419c78d26 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/testReport/ |
   | Max. process+thread count | 295 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730103169


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 30s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 55s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 13s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 55s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  8s |  hbase-client in the patch passed.  |
   |  |   |  27m 20s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux aa0b8735590b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 8c1e4763b3 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/testReport/ |
   | Max. process+thread count | 285 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] thangTang commented on a change in pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on a change in pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#discussion_r527637456



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
##########
@@ -447,7 +450,16 @@ public Scan withStopRow(byte[] stopRow, boolean inclusive) {
    * after this method will yield undefined results.</b></p>
    * @param rowPrefix the prefix all rows must start with. (Set <i>null</i> to remove the filter.)
    * @return this
+   * @deprecated since 3.0.0. The scan result might be unexpected in some cases.
+   *   e.g. startRow : "112" and rowPrefixFilter : "11"
+   *   The Result of this scan might contains : "111"
+   *   This method implements the filter by setting startRow and stopRow,
+   *   but does not take care of the scene where startRow has been set.

Review comment:
       Updated.




----------------------------------------------------------------
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



[GitHub] [hbase] virajjasani edited a comment on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730317424


   > > @thangTang can you please also write a unit test with the same scenarios that you mentioned over Jira? i.e
   > > startRow="112", rowPrefixFilter "11" and Scan can contain "111".
   > > Although you commented, but having clear unit test will be really good reference.
   > > Thanks for identifying the issue.
   > 
   > I'd like to, but should i write the UT in the patch? I think it would be failure and the CI -1.
   
   I didn't get why it would fail. What I mean to say is you can write unit test such that the condition you provided with "111" and "112" holds true. 
   If you are worrying about CI giving -1 for tests which are not relevant to this PR, that's fine, you don't have to worry. As long as we are certain that CI gave -1 for something which is irrelevant to this PR's changes, we can merge the PR.
   Thanks @thangTang 
   
   Please let me know if you still have doubts :)


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730105916


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +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.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 46s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   1m  1s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 27s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 17s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   1m  8s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  36m 14s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux de912c43370a 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 8c1e4763b3 |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-731216901


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 35s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 24s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 30s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 36s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 34s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  5s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 161m 29s |  hbase-server in the patch passed.  |
   |  |   | 191m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 5c9653eec533 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / eca904e0fb |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/testReport/ |
   | Max. process+thread count | 4118 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] thangTang commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-731121624


   > > > @thangTang can you please also write a unit test with the same scenarios that you mentioned over Jira? i.e
   > > > startRow="112", rowPrefixFilter "11" and Scan can contain "111".
   > > > Although you commented, but having clear unit test will be really good reference.
   > > > Thanks for identifying the issue.
   > > 
   > > 
   > > I'd like to, but should i write the UT in the patch? I think it would be failure and the CI -1.
   > 
   > I didn't get why it would fail. What I mean to say is you can write unit test such that the condition you provided with "111" and "112" holds true.
   > If you are worrying about CI giving -1 for tests which are not relevant to this PR, that's fine, you don't have to worry. As long as we are certain that CI gave -1 for something which is irrelevant to this PR's changes, we can merge the PR.
   > Thanks @thangTang
   > 
   > Please let me know if you still have doubts :)
   
   UT updated.


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-732786082


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 31s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 39s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 14s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 41s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 41s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 57s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  9s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 139m 52s |  hbase-server in the patch passed.  |
   |  |   | 172m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux daccfbaf79b6 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1cd8f3cf94 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/testReport/ |
   | Max. process+thread count | 4512 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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



[GitHub] [hbase] virajjasani commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730317424


   > > @thangTang can you please also write a unit test with the same scenarios that you mentioned over Jira? i.e
   > > startRow="112", rowPrefixFilter "11" and Scan can contain "111".
   > > Although you commented, but having clear unit test will be really good reference.
   > > Thanks for identifying the issue.
   > 
   > I'd like to, but should i write the UT in the patch? I think it would be failure and the CI -1.
   
   I didn't get why it would fail. What I mean to say is you can write unit test such that the condition you provided with "111" and "112" holds true. 
   If you are worrying about CI giving -1 for tests which are not relevant to this PR, that's fine, you don't have to worry. As long as we are certain that CI gave -1 for something which is irrelevant to this PR's changes, we can merge the PR.
   Thanks @thangTang 


----------------------------------------------------------------
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



[GitHub] [hbase] thangTang commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
thangTang commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-729604358


   @infraio Hi, could u help me to review this patch?
   I just deprecated this method because modifying it might cause some compatibility issues for old user, and users can compose this logic externally through other methods, so there is no need to implement it inside.
   Is this solution acceptable? Hope to get some suggestions, tks.
   


----------------------------------------------------------------
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



[GitHub] [hbase] Apache-HBase commented on pull request #2674: HBASE-25299 Scan#setRowPrefixFilter Unexpected behavior

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2674:
URL: https://github.com/apache/hbase/pull/2674#issuecomment-730102385


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 50s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 24s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 27s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 23s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 33s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 22s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  3s |  hbase-client in the patch passed.  |
   |  |   |  24m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2674 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f6236979ffc7 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 8c1e4763b3 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/testReport/ |
   | Max. process+thread count | 340 (vs. ulimit of 30000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2674/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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