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/12/14 10:26:40 UTC

[GitHub] [hbase] ramkrish86 opened a new pull request #2776: Using ContiguousCellFormat as a marker alone

ramkrish86 opened a new pull request #2776:
URL: https://github.com/apache/hbase/pull/2776


   This is much simpler version. Instead of adding the APIs to ContiguousCellFormat we are just using as a marker. If so we directly use the package private methods of those cells - currently Only KV and BBKV are marked as ContiguousCellFormat . So this is simple and much easier to understand. 


----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 17s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 11s |  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  |   4m 57s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 22s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 134m 25s |  hbase-server in the patch passed.  |
   |  |   | 160m 50s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 588791b843e3 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 | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/testReport/ |
   | Max. process+thread count | 3895 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  8s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 10s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 30s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 51s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 20s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 49s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  | 124m  3s |  hbase-server in the patch failed.  |
   |  |   | 154m 40s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b94154583c5f 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 | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/testReport/ |
   | Max. process+thread count | 4274 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 20s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  7s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 46s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 47s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 17s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 41s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 130m 59s |  hbase-server in the patch passed.  |
   |  |   | 161m 32s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 3bbafd226a32 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/testReport/ |
   | Max. process+thread count | 3844 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   4m 59s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 17s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  2s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 30s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  | 142m  7s |  hbase-server in the patch failed.  |
   |  |   | 168m 24s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 31da3e807b0b 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 | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/testReport/ |
   | Max. process+thread count | 4083 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] ramkrish86 commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   All clear in this now. I will push this by EOD IST. 


----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 54s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 18s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 33s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 40s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 36s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 14s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 12s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  5s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 22s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 24s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 136m 43s |  hbase-server in the patch passed.  |
   |  |   | 167m 59s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c6c677b2ef0a 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 | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/testReport/ |
   | Max. process+thread count | 3443 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 47s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 21s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 29s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 55s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 19s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 54s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 17s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 40s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  |   7m 10s |  hbase-server in the patch failed.  |
   |  |   |  36m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f6d6bf6fba0d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/testReport/ |
   | Max. process+thread count | 741 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 49s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 19s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 57s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   2m  1s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   3m 39s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 31s |  hbase-common: The patch generated 6 new + 118 unchanged - 0 fixed = 124 total (was 118)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 53s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 52s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 23s |  The patch does not generate ASF License warnings.  |
   |  |   |  49m 41s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux ee3190b725ad 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 | branch-2.3 / 94ce48b173 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 46s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   5m  4s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 45s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   7m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 46s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 42s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 49s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 49s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 31s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 42s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 47s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  | 153m 36s |  hbase-server in the patch failed.  |
   |  |   | 189m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 0b874ebb9ca1 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/testReport/ |
   | Max. process+thread count | 3734 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 10s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  4s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 49s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 46s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 17s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 43s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 147m 46s |  hbase-server in the patch passed.  |
   |  |   | 178m  6s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux e62c90c27972 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/testReport/ |
   | Max. process+thread count | 3906 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  8s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 12s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 21s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   4m 57s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 12s |  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  |   5m  0s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 20s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 133m 25s |  hbase-server in the patch passed.  |
   |  |   | 159m 24s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f981c66c1208 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 | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/testReport/ |
   | Max. process+thread count | 4298 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 14s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  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  |   5m  1s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 22s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 32s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 144m 23s |  hbase-server in the patch passed.  |
   |  |   | 171m 11s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux e34481899464 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 | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/testReport/ |
   | Max. process+thread count | 4759 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 16s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 32s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 46s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 26s |  hbase-common: The patch generated 6 new + 118 unchanged - 0 fixed = 124 total (was 118)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 35s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 59s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m 24s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 159e821a5179 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 | branch-2.3 / 94ce48b173 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 38s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  5s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m  6s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 14s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  3s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 21s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  |   7m 10s |  hbase-server in the patch failed.  |
   |  |   |  32m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c9bdfa767d8b 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 | branch-2.3 / b612260bec |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/testReport/ |
   | Max. process+thread count | 787 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  6s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 15s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 12s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 13s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 17s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 36s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 140m 31s |  hbase-server in the patch passed.  |
   |  |   | 172m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 2f57c6f0e828 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/testReport/ |
   | Max. process+thread count | 3721 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 35s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 45s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 26s |  hbase-common: The patch generated 6 new + 193 unchanged - 0 fixed = 199 total (was 193)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m 25s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 55s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  47m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux dbab62577179 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 | branch-2.3 / 94ce48b173 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/5/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  5s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 12s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 58s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  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  |   5m  3s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 31s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  |   7m 24s |  hbase-server in the patch failed.  |
   |  |   |  33m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux a046bbe32cad 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 | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/2/testReport/ |
   | Max. process+thread count | 767 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] huaxiangsun commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -82,6 +86,337 @@ public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
     return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
   }
 
+  /**
+   * Compares cells that are of type {@link ContiguousCellFormat}. It is basically to peel off the common
+   * comparisons that happen and on top of it make the parsing of individual cell items only once.
+   * @param l the left cell
+   * @param r the right cell
+   * @param ignoreSequenceid if to ignore the sequence id
+   * @return return > 0 if left cell is bigger, < 0 if right is bigger, == 0 if both cells are same
+   */
+  public final static int compare(final ContiguousCellFormat l, final ContiguousCellFormat r, boolean ignoreSequenceid) {
+    int diff = 0;
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      KeyValue left = (KeyValue) l;
+      KeyValue right = (KeyValue) r;
+      // Compare Rows. Cache row length.
+      int leftRowLength = left.getRowLength();
+      int rightRowLength = right.getRowLength();
+      diff = Bytes.compareTo(left.getRowArray(), left.getRowOffset(), leftRowLength,
+        right.getRowArray(), right.getRowOffset(), rightRowLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // If the column is not specified, the "minimum" key type appears as latest in the sorted
+      // order, regardless of the timestamp. This is used for specifying the last key/value in a
+      // given row, because there is no "lexicographically last column" (it would be infinitely
+      // long).
+      // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+      // that
+      // we can't do memcmp w/ special rules like this.
+      // TODO: Is there a test for this behavior?
+      int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+      int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+      int leftKeyLength = left.getKeyLength();
+      int leftQualifierLength =
+          left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+      // No need of left row length below here.
+
+      byte leftType = left.getTypeByte(leftKeyLength);
+      if (leftFamilyLength + leftQualifierLength == 0
+          && leftType == KeyValue.Type.Minimum.getCode()) {
+        // left is "bigger", i.e. it appears later in the sorted order
+        return 1;
+      }
+
+      int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+      int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+      int rightKeyLength = right.getKeyLength();
+      int rightQualifierLength =
+          right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+      // No need of right row length below here.
+
+      byte rightType = right.getTypeByte(rightKeyLength);
+      if (rightFamilyLength + rightQualifierLength == 0
+          && rightType == KeyValue.Type.Minimum.getCode()) {
+        return -1;
+      }
+
+      // Compare families.
+      int leftFamilyPosition = left.getFamilyOffset(leftFamilyLengthPosition);
+      int rightFamilyPosition = right.getFamilyOffset(rightFamilyLengthPosition);
+      diff = Bytes.compareTo(left.getFamilyArray(), leftFamilyPosition, leftFamilyLength,
+        right.getFamilyArray(), rightFamilyPosition, rightFamilyLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare qualifiers
+      diff = Bytes.compareTo(left.getQualifierArray(),
+        left.getQualifierOffset(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+        right.getQualifierArray(), right.getQualifierOffset(rightFamilyPosition, rightFamilyLength),
+        rightQualifierLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Timestamps.
+      // Swap order we pass into compare so we get DESCENDING order.
+      diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare types. Let the delete types sort ahead of puts; i.e. types
+      // of higher numbers sort before those of lesser numbers. Maximum (255)
+      // appears ahead of everything, and minimum (0) appears after
+      // everything.
+      return (0xff & rightType) - (0xff & leftType);
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      KeyValue left = (KeyValue) l;
+      ByteBufferKeyValue right = (ByteBufferKeyValue) r;
+      // Compare Rows. Cache row length.
+      int leftRowLength = left.getRowLength();
+      int rightRowLength = right.getRowLength();
+      diff = ByteBufferUtils.compareTo(left.getRowArray(), left.getRowOffset(), leftRowLength,
+        right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // If the column is not specified, the "minimum" key type appears as latest in the sorted
+      // order, regardless of the timestamp. This is used for specifying the last key/value in a
+      // given row, because there is no "lexicographically last column" (it would be infinitely
+      // long).
+      // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+      // that
+      // we can't do memcmp w/ special rules like this.
+      // TODO: Is there a test for this behavior?
+      int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+      int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+      int leftKeyLength = left.getKeyLength();
+      int leftQualifierLength =
+          left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+      // No need of left row length below here.
+
+      byte leftType = left.getTypeByte(leftKeyLength);
+      if (leftFamilyLength + leftQualifierLength == 0
+          && leftType == KeyValue.Type.Minimum.getCode()) {
+        // left is "bigger", i.e. it appears later in the sorted order
+        return 1;
+      }
+
+      int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+      int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+      int rightKeyLength = right.getKeyLength();
+      int rightQualifierLength =
+          right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+      // No need of right row length below here.
+
+      byte rightType = right.getTypeByte(rightKeyLength);
+      if (rightFamilyLength + rightQualifierLength == 0
+          && rightType == KeyValue.Type.Minimum.getCode()) {
+        return -1;
+      }
+
+      // Compare families.
+      int leftFamilyPosition = left.getFamilyOffset(leftFamilyLengthPosition);
+      int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+      diff = ByteBufferUtils.compareTo(left.getFamilyArray(), leftFamilyPosition, leftFamilyLength,
+        right.getFamilyByteBuffer(), rightFamilyPosition, rightFamilyLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare qualifiers
+      diff = ByteBufferUtils.compareTo(left.getQualifierArray(),
+        left.getQualifierOffset(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+        right.getQualifierByteBuffer(),
+        right.getQualifierPosition(rightFamilyPosition, rightFamilyLength), rightQualifierLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Timestamps.
+      // Swap order we pass into compare so we get DESCENDING order.
+      diff = Long.compare(left.getTimestamp(rightKeyLength), right.getTimestamp(leftKeyLength));
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare types. Let the delete types sort ahead of puts; i.e. types
+      // of higher numbers sort before those of lesser numbers. Maximum (255)
+      // appears ahead of everything, and minimum (0) appears after
+      // everything.
+      return (0xff & rightType) - (0xff & leftType);
+
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      ByteBufferKeyValue left = (ByteBufferKeyValue) l;
+      KeyValue right = (KeyValue) r;
+      // Compare Rows. Cache row length.
+      int leftRowLength = left.getRowLength();
+      int rightRowLength = right.getRowLength();
+      diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+        leftRowLength, right.getRowArray(), right.getRowOffset(), rightRowLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // If the column is not specified, the "minimum" key type appears as latest in the sorted
+      // order, regardless of the timestamp. This is used for specifying the last key/value in a
+      // given row, because there is no "lexicographically last column" (it would be infinitely
+      // long).
+      // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+      // that
+      // we can't do memcmp w/ special rules like this.
+      // TODO: Is there a test for this behavior?
+      int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+      int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+      int leftKeyLength = left.getKeyLength();
+      int leftQualifierLength =
+          left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+      // No need of left row length below here.
+
+      byte leftType = left.getTypeByte(leftKeyLength);
+      if (leftFamilyLength + leftQualifierLength == 0
+          && leftType == KeyValue.Type.Minimum.getCode()) {
+        // left is "bigger", i.e. it appears later in the sorted order
+        return 1;
+      }
+
+      int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+      int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+      int rightKeyLength = right.getKeyLength();
+      int rightQualifierLength =
+          right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+      // No need of right row length below here.
+
+      byte rightType = right.getTypeByte(rightKeyLength);
+      if (rightFamilyLength + rightQualifierLength == 0
+          && rightType == KeyValue.Type.Minimum.getCode()) {
+        return -1;
+      }
+
+      // Compare families.
+      int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+      int rightFamilyPosition = right.getFamilyOffset(rightFamilyLengthPosition);
+      diff = ByteBufferUtils.compareTo(left.getFamilyByteBuffer(), leftFamilyPosition,
+        leftFamilyLength, right.getFamilyArray(), rightFamilyPosition, rightFamilyLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare qualifiers
+      diff = ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+        left.getQualifierPosition(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+        right.getQualifierArray(), right.getQualifierOffset(rightFamilyPosition, rightFamilyLength),
+        rightQualifierLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Timestamps.
+      // Swap order we pass into compare so we get DESCENDING order.
+      diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare types. Let the delete types sort ahead of puts; i.e. types
+      // of higher numbers sort before those of lesser numbers. Maximum (255)
+      // appears ahead of everything, and minimum (0) appears after
+      // everything.
+      return (0xff & rightType) - (0xff & leftType);
+
+    } else {

Review comment:
       Ditto here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 33s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 56s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   3m 19s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  5s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 45s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  20m 15s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  48m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 8372bd036ab1 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 | branch-2.3 / f490913764 |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/9/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] saintstack commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   @ramkrish86 I think there is an @anoopsjohn  ask in the above and I have a question on how the test was run but otherwise, looks good to me.


----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  8s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 30s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  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  |   4m 58s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 29s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 141m  2s |  hbase-server in the patch passed.  |
   |  |   | 167m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 569df1b004f3 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 | branch-2.3 / 94ce48b173 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/testReport/ |
   | Max. process+thread count | 3346 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] ramkrish86 commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   @saintstack  - finished all comments. Will push this unless objections. 


----------------------------------------------------------------
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] anoopsjohn commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0
+        && leftType == KeyValue.Type.Minimum.getCode()) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightFamilyLength + rightQualifierLength == 0
+        && rightType == KeyValue.Type.Minimum.getCode()) {
+      return -1;
+    }
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+    diff = ByteBufferUtils.compareTo(left.getFamilyByteBuffer(), leftFamilyPosition,
+      leftFamilyLength, right.getFamilyByteBuffer(), rightFamilyPosition, rightFamilyLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare qualifiers
+    diff = ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      left.getQualifierPosition(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+      right.getQualifierByteBuffer(),
+      right.getQualifierPosition(rightFamilyPosition, rightFamilyLength), rightQualifierLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Timestamps.
+    // Swap order we pass into compare so we get DESCENDING order.
+    diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));

Review comment:
       Not suggesting for doing in this patch. 
   But may be we can avoid the long decodes and do compare but instead bytes level compare we can do.  We are not interested in the exact diff here anyways. A compare value of 0 or +ve number of -ve number. Add a TODO so that we can come back and do some JMH tests and if that helps, we can incorporate in all places. WDYT?




----------------------------------------------------------------
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 #2776: HBASE-24850 CellComparator perf improvement

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 38s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 26s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 32s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 36s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 11s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  16m 42s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 55s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  41m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 113e02276611 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 | branch-2.3 / 2bcab163b6 |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/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] ramkrish86 merged pull request #2776: HBASE-24850 CellComparator perf improvement

Posted by GitBox <gi...@apache.org>.
ramkrish86 merged pull request #2776:
URL: https://github.com/apache/hbase/pull/2776


   


----------------------------------------------------------------
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] saintstack commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
##########
@@ -920,6 +930,14 @@ public static boolean matchingColumn(final Cell left, final Cell right) {
     return matchingQualifier(left, right);
   }
 
+  public static boolean matchingColumn(final Cell left, final byte lFamLen, final int lQualLength,

Review comment:
       These have to be public? They are in same package as comparator and Cell types.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
##########
@@ -1598,16 +1621,22 @@ public static boolean matchingRows(final Cell left, final Cell right) {
    * @return True if same row and column.
    */
   public static boolean matchingRowColumn(final Cell left, final Cell right) {
-    if ((left.getRowLength() + left.getFamilyLength()
-        + left.getQualifierLength()) != (right.getRowLength() + right.getFamilyLength()
-            + right.getQualifierLength())) {
+    short lrowlength = left.getRowLength();
+    short rrowlength = right.getRowLength();
+    byte lfamlength = left.getFamilyLength();
+    byte rfamlength = right.getFamilyLength();
+    int lqlength = left.getQualifierLength();
+    int rqlength = right.getQualifierLength();

Review comment:
       We are decoding ints, bytes and shorts that we might end up not using at all.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,285 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {

Review comment:
       This if/else is a lot of code. Would be good to break it out... check for it at top of the compare.... For a follow-on.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,285 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareKeyValues(final KeyValue left, final KeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = Bytes.compareTo(left.getRowArray(), left.getRowOffset(), leftRowLength,
+      right.getRowArray(), right.getRowOffset(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftType == KeyValue.Type.Minimum.getCode()
+        && leftFamilyLength + leftQualifierLength == 0) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightType == KeyValue.Type.Minimum.getCode()
+        && rightFamilyLength + rightQualifierLength == 0) {
+      return -1;
+    }
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyOffset(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyOffset(rightFamilyLengthPosition);
+    diff = Bytes.compareTo(left.getFamilyArray(), leftFamilyPosition, leftFamilyLength,
+      right.getFamilyArray(), rightFamilyPosition, rightFamilyLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare qualifiers
+    diff = Bytes.compareTo(left.getQualifierArray(),
+      left.getQualifierOffset(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+      right.getQualifierArray(), right.getQualifierOffset(rightFamilyPosition, rightFamilyLength),
+      rightQualifierLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Timestamps.
+    // Swap order we pass into compare so we get DESCENDING order.
+    diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare types. Let the delete types sort ahead of puts; i.e. types
+    // of higher numbers sort before those of lesser numbers. Maximum (255)
+    // appears ahead of everything, and minimum (0) appears after
+    // everything.
+    return (0xff & rightType) - (0xff & leftType);
+  }

Review comment:
       In follow-on, we should look at produced byte code. Might get ideas on how to make this go faster.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {

Review comment:
       The perf tests were PE @ramkrish86 ? Yeah, we need a JMH test so can do the combinations @anoopsjohn suggests. I can work on that...

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
##########
@@ -1396,7 +1403,14 @@ public int getQualifierOffset() {
    * @return Qualifier offset
    */
   private int getQualifierOffset(int foffset) {
-    return foffset + getFamilyLength(foffset);
+    return getQualifierOffset(foffset, getFamilyLength());
+  }
+
+  /**
+   * @return Qualifier offset
+   */
+  int getQualifierOffset(int foffset, int flength) {

Review comment:
       Looks good




----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 26s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 32s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 41s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 42s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 27s |  hbase-common: The patch generated 4 new + 193 unchanged - 0 fixed = 197 total (was 193)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 29s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 28s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux ede5ecc22cca 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 | branch-2.3 / b612260bec |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/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] anoopsjohn commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
##########
@@ -1598,16 +1621,21 @@ public static boolean matchingRows(final Cell left, final Cell right) {
    * @return True if same row and column.
    */
   public static boolean matchingRowColumn(final Cell left, final Cell right) {
-    if ((left.getRowLength() + left.getFamilyLength()
-        + left.getQualifierLength()) != (right.getRowLength() + right.getFamilyLength()
-            + right.getQualifierLength())) {
+    short lrowlength = left.getRowLength();
+    short rrowlength = right.getRowLength();
+    byte lfamlength = left.getFamilyLength();
+    byte rfamlength = right.getFamilyLength();
+    int lqlength = left.getQualifierLength();
+    int rqlength = right.getQualifierLength();
+    // match length
+    if ((lrowlength + lfamlength + lqlength) != (rrowlength + rfamlength + rqlength)) {

Review comment:
       More efficient way is ((lrowlength == rrowlength) && (lfamlength == rfamlength) && (lqlength == rqlength) ?

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());

Review comment:
       Is this line needed here? All other branches fall back to the last line in this method.  Any specific reason for doing it here?

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0
+        && leftType == KeyValue.Type.Minimum.getCode()) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightFamilyLength + rightQualifierLength == 0
+        && rightType == KeyValue.Type.Minimum.getCode()) {
+      return -1;
+    }
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+    diff = ByteBufferUtils.compareTo(left.getFamilyByteBuffer(), leftFamilyPosition,
+      leftFamilyLength, right.getFamilyByteBuffer(), rightFamilyPosition, rightFamilyLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare qualifiers
+    diff = ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      left.getQualifierPosition(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+      right.getQualifierByteBuffer(),
+      right.getQualifierPosition(rightFamilyPosition, rightFamilyLength), rightQualifierLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Timestamps.
+    // Swap order we pass into compare so we get DESCENDING order.
+    diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));

Review comment:
       Not suggesting for doing in this patch. 
   But may be we can avoid the long decodes and do compare but instead bytes level compare we can do. That too 1st with lower order bytes.  We are not interested in the exact diff here anyways. A compare value of 0 or +ve number of -ve number. Add a TODO so that we can come back and do some JMH tests and if that helps, we can incorporate in all places. WDYT?

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -125,38 +442,174 @@ public final int compareFamilies(Cell left, Cell right) {
         right.getFamilyArray(), right.getFamilyOffset(), right.getFamilyLength());
   }
 
+  static int compareQualifiers(KeyValue left, KeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyOffset = left.getFamilyOffset(leftFamilyLengthPosition);
+    int rightFamilyOffset = right.getFamilyOffset(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return Bytes.compareTo(left.getQualifierArray(), leftFamilyOffset + leftFamilyLength,
+      leftQualifierLength, right.getQualifierArray(), rightFamilyOffset + rightFamilyLength,
+      rightQualifierLength);
+  }
+
+  static int compareQualifiers(KeyValue left, ByteBufferKeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyOffset = left.getFamilyOffset(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierArray(),
+      leftFamilyOffset + leftFamilyLength, leftQualifierLength, right.getQualifierByteBuffer(),
+      rightFamilyPosition + rightFamilyLength, rightQualifierLength);
+  }
+
+  static int compareQualifiers(ByteBufferKeyValue left, KeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyOffset = right.getFamilyOffset(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      leftFamilyPosition + leftFamilyLength, leftQualifierLength, right.getQualifierArray(),
+      rightFamilyOffset + rightFamilyLength, rightQualifierLength);
+  }
+
+  static int compareQualifiers(ByteBufferKeyValue left, ByteBufferKeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      leftFamilyPosition + leftFamilyLength, leftQualifierLength, right.getQualifierByteBuffer(),
+      rightFamilyPosition + rightFamilyLength, rightQualifierLength);
+  }
+
   /**
    * Compare the qualifiers part of the left and right cells.
    * @return 0 if both cells are equal, 1 if left cell is bigger than right, -1 otherwise
    */
   @Override
   public final int compareQualifiers(Cell left, Cell right) {
-    if (left instanceof ByteBufferExtendedCell && right instanceof ByteBufferExtendedCell) {
-      return ByteBufferUtils
-          .compareTo(((ByteBufferExtendedCell) left).getQualifierByteBuffer(),
-              ((ByteBufferExtendedCell) left).getQualifierPosition(),
-              left.getQualifierLength(), ((ByteBufferExtendedCell) right).getQualifierByteBuffer(),
-              ((ByteBufferExtendedCell) right).getQualifierPosition(),
-              right.getQualifierLength());
-    }
-    if (left instanceof ByteBufferExtendedCell) {
-      return ByteBufferUtils.compareTo(((ByteBufferExtendedCell) left).getQualifierByteBuffer(),
+    if ((left instanceof ByteBufferKeyValue) && (right instanceof ByteBufferKeyValue)) {
+      return compareQualifiers((ByteBufferKeyValue) left, (ByteBufferKeyValue) right);
+    } else if ((left instanceof KeyValue) && (right instanceof KeyValue)) {
+      return compareQualifiers((KeyValue) left, (KeyValue) right);
+    } else if ((left instanceof KeyValue) && (right instanceof ByteBufferKeyValue)) {
+      return compareQualifiers((KeyValue) left, (ByteBufferKeyValue) right);
+    } else if ((left instanceof ByteBufferKeyValue) && (right instanceof KeyValue)) {
+      return compareQualifiers((ByteBufferKeyValue) left, (KeyValue) right);
+    } else {
+      if (left instanceof ByteBufferExtendedCell && right instanceof ByteBufferExtendedCell) {

Review comment:
       We have ByteBufferKeyValue which extends ByteBufferExtendedCell  and other sub types also.  May be its time to reduce these types. I see we have KeyOnlyByteBufferExtendedCell..  But there is some ways.. Same way will see how we can have the DBE seekers also returning Cells of type KV or BBKV.  So the optimizations will be used in case of scan over DBEed table:cf.   Will do it in a followup issue. I can do that.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);

Review comment:
       Can we extract this also to a private method pls?   

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0

Review comment:
       We can reverse the check. Mostly leftType wont be Minimum type and so we can skip the '+' operator eval.  These were like this before.. But can we focus on these small small optimization also now?  That would be good.  

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {

Review comment:
       In read path mostly both cells will be either KV or BBKV.  In write path we have BBKV now? I think some patches did that for master branch already. we create the ChunkCell or so. Is that reverted? Pls check.  
   So better to do both KV check and then both BBKV followed by other 2 options?
   Te perf tests u did now was giving all cells are KVs right?  is it possible to test for both cells as BBKV also?  That will be better.  May be a JMH test?
   
   The split of private methods came good now.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0
+        && leftType == KeyValue.Type.Minimum.getCode()) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightFamilyLength + rightQualifierLength == 0

Review comment:
       Same here also




----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 14s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 57s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 31s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 19s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 45s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 12s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 19s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 17s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 46s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  |  17m 12s |  hbase-server in the patch failed.  |
   |  |   |  48m 20s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 54b070daf02c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / b612260bec |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/testReport/ |
   | Max. process+thread count | 955 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/6/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] ramkrish86 commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
##########
@@ -1396,7 +1403,14 @@ public int getQualifierOffset() {
    * @return Qualifier offset
    */
   private int getQualifierOffset(int foffset) {
-    return foffset + getFamilyLength(foffset);
+    return getQualifierOffset(foffset, getFamilyLength());
+  }
+
+  /**
+   * @return Qualifier offset
+   */
+  int getQualifierOffset(int foffset, int flength) {

Review comment:
       Done 




----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 13s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 32s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 45s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 17s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 26s |  hbase-common: The patch generated 6 new + 118 unchanged - 0 fixed = 124 total (was 118)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m  8s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 25s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux feaac7433d49 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 | branch-2.3 / 94ce48b173 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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] Apache-HBase commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 32s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 34s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 40s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 14s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 26s |  hbase-common: The patch generated 6 new + 118 unchanged - 0 fixed = 124 total (was 118)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  16m 41s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 58s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 4fd483e8dd4f 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 | branch-2.3 / 94ce48b173 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 54s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 14s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 49s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   3m 10s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  4s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 31s |  hbase-common: The patch generated 4 new + 193 unchanged - 0 fixed = 197 total (was 193)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  21m 23s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 43s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 54e7694cc171 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 | branch-2.3 / f490913764 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/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 #2776: Using ContiguousCellFormat as a marker alone

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 23s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  6s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 53s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 30s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   6m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 35s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 47s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 47s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 44s |  hbase-common in the patch passed.  |
   | -1 :x: |  unit  | 140m 19s |  hbase-server in the patch failed.  |
   |  |   | 172m 35s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c4142fefe815 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / f490913764 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/testReport/ |
   | Max. process+thread count | 3716 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/7/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 #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -82,6 +86,337 @@ public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
     return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
   }
 
+  /**
+   * Compares cells that are of type {@link ContiguousCellFormat}. It is basically to peel off the common
+   * comparisons that happen and on top of it make the parsing of individual cell items only once.
+   * @param l the left cell
+   * @param r the right cell
+   * @param ignoreSequenceid if to ignore the sequence id
+   * @return return > 0 if left cell is bigger, < 0 if right is bigger, == 0 if both cells are same
+   */
+  public final static int compare(final ContiguousCellFormat l, final ContiguousCellFormat r, boolean ignoreSequenceid) {
+    int diff = 0;
+    if (l instanceof KeyValue && r instanceof KeyValue) {

Review comment:
       Can be abstract this into a separate method, say KVCompare? Per discussion in the jira, this size of this method is too big to be inlined.




----------------------------------------------------------------
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] ramkrish86 commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   @huaxiangsun , @saintstack 
   I have updated as per the comments. Pls review. 


----------------------------------------------------------------
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] saintstack commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
##########
@@ -1396,7 +1403,14 @@ public int getQualifierOffset() {
    * @return Qualifier offset
    */
   private int getQualifierOffset(int foffset) {
-    return foffset + getFamilyLength(foffset);
+    return getQualifierOffset(foffset, getFamilyLength());
+  }
+
+  /**
+   * @return Qualifier offset
+   */
+  int getQualifierOffset(int foffset, int flength) {

Review comment:
       So, these methods are not part of the Cell Interface. They are package private. The CellComparator is in same package. It makes use of these package private methods. I think that is fine. I don't think we need the marker interface in this case given all current Cell implementations are 'contiguous'? I say this because there are already too many Cell Interfaces. Lets worry about adding a Marker interface later, when we are adding a new Cell type?

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ContiguousCellFormat.java
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ *  A marker interface that indicates that the cells follow the KV serialization pattern.

Review comment:
       Say a bit more what you mean here... if you make a new patch (nit)




----------------------------------------------------------------
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] saintstack commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -125,38 +442,174 @@ public final int compareFamilies(Cell left, Cell right) {
         right.getFamilyArray(), right.getFamilyOffset(), right.getFamilyLength());
   }
 
+  static int compareQualifiers(KeyValue left, KeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyOffset = left.getFamilyOffset(leftFamilyLengthPosition);
+    int rightFamilyOffset = right.getFamilyOffset(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return Bytes.compareTo(left.getQualifierArray(), leftFamilyOffset + leftFamilyLength,
+      leftQualifierLength, right.getQualifierArray(), rightFamilyOffset + rightFamilyLength,
+      rightQualifierLength);
+  }
+
+  static int compareQualifiers(KeyValue left, ByteBufferKeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyOffset = left.getFamilyOffset(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierArray(),
+      leftFamilyOffset + leftFamilyLength, leftQualifierLength, right.getQualifierByteBuffer(),
+      rightFamilyPosition + rightFamilyLength, rightQualifierLength);
+  }
+
+  static int compareQualifiers(ByteBufferKeyValue left, KeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyOffset = right.getFamilyOffset(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      leftFamilyPosition + leftFamilyLength, leftQualifierLength, right.getQualifierArray(),
+      rightFamilyOffset + rightFamilyLength, rightQualifierLength);
+  }
+
+  static int compareQualifiers(ByteBufferKeyValue left, ByteBufferKeyValue right) {
+    // NOTE: Same method is in CellComparatorImpl, also private, not shared, intentionally. Not
+    // sharing gets us a few percent more throughput in compares. If changes here or there, make
+    // sure done in both places.
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    byte leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    byte rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+
+    // Compare qualifiers
+    return ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      leftFamilyPosition + leftFamilyLength, leftQualifierLength, right.getQualifierByteBuffer(),
+      rightFamilyPosition + rightFamilyLength, rightQualifierLength);
+  }
+
   /**
    * Compare the qualifiers part of the left and right cells.
    * @return 0 if both cells are equal, 1 if left cell is bigger than right, -1 otherwise
    */
   @Override
   public final int compareQualifiers(Cell left, Cell right) {
-    if (left instanceof ByteBufferExtendedCell && right instanceof ByteBufferExtendedCell) {
-      return ByteBufferUtils
-          .compareTo(((ByteBufferExtendedCell) left).getQualifierByteBuffer(),
-              ((ByteBufferExtendedCell) left).getQualifierPosition(),
-              left.getQualifierLength(), ((ByteBufferExtendedCell) right).getQualifierByteBuffer(),
-              ((ByteBufferExtendedCell) right).getQualifierPosition(),
-              right.getQualifierLength());
-    }
-    if (left instanceof ByteBufferExtendedCell) {
-      return ByteBufferUtils.compareTo(((ByteBufferExtendedCell) left).getQualifierByteBuffer(),
+    if ((left instanceof ByteBufferKeyValue) && (right instanceof ByteBufferKeyValue)) {
+      return compareQualifiers((ByteBufferKeyValue) left, (ByteBufferKeyValue) right);
+    } else if ((left instanceof KeyValue) && (right instanceof KeyValue)) {
+      return compareQualifiers((KeyValue) left, (KeyValue) right);
+    } else if ((left instanceof KeyValue) && (right instanceof ByteBufferKeyValue)) {
+      return compareQualifiers((KeyValue) left, (ByteBufferKeyValue) right);
+    } else if ((left instanceof ByteBufferKeyValue) && (right instanceof KeyValue)) {
+      return compareQualifiers((ByteBufferKeyValue) left, (KeyValue) right);
+    } else {
+      if (left instanceof ByteBufferExtendedCell && right instanceof ByteBufferExtendedCell) {

Review comment:
       +1. Not in this issue but file a follow-on.




----------------------------------------------------------------
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] ramkrish86 commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   @saintstack  - The test I used to verify this was a standalone test (not JMH) where I measure the E2E time for adding millions of cells to the tree map. Some thing that you see in https://issues.apache.org/jira/browse/HBASE-24754.
   Apart from that a cluster test was also performed and I can ask @pankaj72981  to share that report. 


----------------------------------------------------------------
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 #2776: HBASE-24850 CellComparator perf improvement

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 30s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 56s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 29s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 43s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in branch-2.3 failed.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server in branch-2.3 failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 55s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 36s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 131m 13s |  hbase-server in the patch passed.  |
   |  |   | 161m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 1f2576671ddb 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / 2bcab163b6 |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/testReport/ |
   | Max. process+thread count | 3747 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/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 #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -82,6 +86,337 @@ public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
     return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
   }
 
+  /**
+   * Compares cells that are of type {@link ContiguousCellFormat}. It is basically to peel off the common
+   * comparisons that happen and on top of it make the parsing of individual cell items only once.
+   * @param l the left cell
+   * @param r the right cell
+   * @param ignoreSequenceid if to ignore the sequence id
+   * @return return > 0 if left cell is bigger, < 0 if right is bigger, == 0 if both cells are same
+   */
+  public final static int compare(final ContiguousCellFormat l, final ContiguousCellFormat r, boolean ignoreSequenceid) {
+    int diff = 0;
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      KeyValue left = (KeyValue) l;
+      KeyValue right = (KeyValue) r;
+      // Compare Rows. Cache row length.
+      int leftRowLength = left.getRowLength();
+      int rightRowLength = right.getRowLength();
+      diff = Bytes.compareTo(left.getRowArray(), left.getRowOffset(), leftRowLength,
+        right.getRowArray(), right.getRowOffset(), rightRowLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // If the column is not specified, the "minimum" key type appears as latest in the sorted
+      // order, regardless of the timestamp. This is used for specifying the last key/value in a
+      // given row, because there is no "lexicographically last column" (it would be infinitely
+      // long).
+      // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+      // that
+      // we can't do memcmp w/ special rules like this.
+      // TODO: Is there a test for this behavior?
+      int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+      int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+      int leftKeyLength = left.getKeyLength();
+      int leftQualifierLength =
+          left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+      // No need of left row length below here.
+
+      byte leftType = left.getTypeByte(leftKeyLength);
+      if (leftFamilyLength + leftQualifierLength == 0
+          && leftType == KeyValue.Type.Minimum.getCode()) {
+        // left is "bigger", i.e. it appears later in the sorted order
+        return 1;
+      }
+
+      int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+      int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+      int rightKeyLength = right.getKeyLength();
+      int rightQualifierLength =
+          right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+      // No need of right row length below here.
+
+      byte rightType = right.getTypeByte(rightKeyLength);
+      if (rightFamilyLength + rightQualifierLength == 0
+          && rightType == KeyValue.Type.Minimum.getCode()) {
+        return -1;
+      }
+
+      // Compare families.
+      int leftFamilyPosition = left.getFamilyOffset(leftFamilyLengthPosition);
+      int rightFamilyPosition = right.getFamilyOffset(rightFamilyLengthPosition);
+      diff = Bytes.compareTo(left.getFamilyArray(), leftFamilyPosition, leftFamilyLength,
+        right.getFamilyArray(), rightFamilyPosition, rightFamilyLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare qualifiers
+      diff = Bytes.compareTo(left.getQualifierArray(),
+        left.getQualifierOffset(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+        right.getQualifierArray(), right.getQualifierOffset(rightFamilyPosition, rightFamilyLength),
+        rightQualifierLength);
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Timestamps.
+      // Swap order we pass into compare so we get DESCENDING order.
+      diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));
+      if (diff != 0) {
+        return diff;
+      }
+
+      // Compare types. Let the delete types sort ahead of puts; i.e. types
+      // of higher numbers sort before those of lesser numbers. Maximum (255)
+      // appears ahead of everything, and minimum (0) appears after
+      // everything.
+      return (0xff & rightType) - (0xff & leftType);
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {

Review comment:
       For     } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
   and } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) { cases, essentially, there are same.
   Can we have a method for BBKVCompare() and swap parameters for these two cases?




----------------------------------------------------------------
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] saintstack commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
##########
@@ -1598,16 +1621,21 @@ public static boolean matchingRows(final Cell left, final Cell right) {
    * @return True if same row and column.
    */
   public static boolean matchingRowColumn(final Cell left, final Cell right) {
-    if ((left.getRowLength() + left.getFamilyLength()
-        + left.getQualifierLength()) != (right.getRowLength() + right.getFamilyLength()
-            + right.getQualifierLength())) {
+    short lrowlength = left.getRowLength();
+    short rrowlength = right.getRowLength();
+    byte lfamlength = left.getFamilyLength();
+    byte rfamlength = right.getFamilyLength();
+    int lqlength = left.getQualifierLength();
+    int rqlength = right.getQualifierLength();
+    // match length
+    if ((lrowlength + lfamlength + lqlength) != (rrowlength + rfamlength + rqlength)) {

Review comment:
       I like the @anoopsjohn suggestion




----------------------------------------------------------------
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] saintstack commented on pull request #2776: Using ContiguousCellFormat as a marker alone

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


   Be careful @ramkrish86 .. you are missing the JIRA from this PR summary.


----------------------------------------------------------------
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 #2776: Using ContiguousCellFormat as a marker alone

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 39s |  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.  |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 20s |  branch-2.3 passed  |
   | +1 :green_heart: |  checkstyle  |   1m 40s |  branch-2.3 passed  |
   | +1 :green_heart: |  spotbugs  |   2m 45s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 39s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 45s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 23s |  The patch does not generate ASF License warnings.  |
   |  |   |  47m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 78308c3f5fc0 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.3 / f490913764 |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/8/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 #2776: HBASE-24850 CellComparator perf improvement

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  8s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.3 Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 36s |  branch-2.3 passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  branch-2.3 passed  |
   | +1 :green_heart: |  shadedjars  |   5m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  branch-2.3 passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 14s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  1s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 21s |  hbase-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 21s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 134m  5s |  hbase-server in the patch passed.  |
   |  |   | 160m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2776 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 2e12dc3d7afa 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 | branch-2.3 / 2bcab163b6 |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/artifact/yetus-jdk8-hadoop2-check/output/diff-javadoc-javadoc-hbase-common.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/testReport/ |
   | Max. process+thread count | 4019 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2776/10/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] saintstack commented on a change in pull request #2776: Using ContiguousCellFormat as a marker alone

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, (ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the last key/value in a
+    // given row, because there is no "lexicographically last column" (it would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0
+        && leftType == KeyValue.Type.Minimum.getCode()) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightFamilyLength + rightQualifierLength == 0
+        && rightType == KeyValue.Type.Minimum.getCode()) {
+      return -1;
+    }
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = right.getFamilyPosition(rightFamilyLengthPosition);
+    diff = ByteBufferUtils.compareTo(left.getFamilyByteBuffer(), leftFamilyPosition,
+      leftFamilyLength, right.getFamilyByteBuffer(), rightFamilyPosition, rightFamilyLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare qualifiers
+    diff = ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      left.getQualifierPosition(leftFamilyPosition, leftFamilyLength), leftQualifierLength,
+      right.getQualifierByteBuffer(),
+      right.getQualifierPosition(rightFamilyPosition, rightFamilyLength), rightQualifierLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Timestamps.
+    // Swap order we pass into compare so we get DESCENDING order.
+    diff = Long.compare(right.getTimestamp(rightKeyLength), left.getTimestamp(leftKeyLength));

Review comment:
       Nice idea. Wonder if we can do this elsewhere?




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