You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "bbeaudreault (via GitHub)" <gi...@apache.org> on 2023/10/31 17:38:05 UTC
[PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
bbeaudreault opened a new pull request, #5488:
URL: https://github.com/apache/hbase/pull/5488
Uses LambdaMetafactory to create fast functions for reflectively parsing filters and comparators. These are cached for the lifetime of the process.
On startup, a cache will be populated for all of the subclasses of Filter and Comparator in the same package. This covers all of our built-ins. For now, custom filters/comparators are not covered unless the user places them in the same package and are available at startup.
With java9+ we could support updating the cache over time from the dynamic classloader. We could possibly work around that with some language reflection now too, but this patch as-is is a huge optimization. So would rather tackle that in a follow-up.
This is covered by existing unit tests, but I updated them to specifically ensure that the new functionality is working. I also added unit tests for custom filters/comparators to verify the expected behaviour wrt this feature.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1803315656
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 32s | master passed |
| +1 :green_heart: | compile | 1m 17s | master passed |
| +1 :green_heart: | shadedjars | 4m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
| -0 :warning: | patch | 6m 17s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 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 | 4m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 54s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 18s | hbase-client in the patch passed. |
| -1 :x: | unit | 232m 30s | hbase-server in the patch failed. |
| | | 260m 47s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1b8122a9e068 5.4.0-156-generic #173-Ubuntu SMP Tue Jul 11 07:25:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 4b5db21f3f |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/testReport/ |
| Max. process+thread count | 4839 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804823842
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 30s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 6m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | master passed |
| -0 :warning: | patch | 7m 37s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 15s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 3s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 27s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 230m 2s | hbase-server in the patch passed. |
| | | 263m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2d17e04470bb 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7f3921ae40 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/testReport/ |
| Max. process+thread count | 4534 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789873864
Test failures are unrelated.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1795096884
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 51s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 23s | master passed |
| +1 :green_heart: | compile | 4m 5s | master passed |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | spotless | 0m 48s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 21s | master passed |
| -0 :warning: | patch | 2m 11s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 40s | the patch passed |
| +1 :green_heart: | compile | 3m 58s | the patch passed |
| +1 :green_heart: | javac | 3m 58s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 8s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 55s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 1m 3s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 53m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 4d27bbd88806 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 027a119bcf |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1787778601
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 4m 20s | master passed |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | spotless | 0m 49s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 9s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 26s | the patch passed |
| +1 :green_heart: | compile | 4m 2s | the patch passed |
| -0 :warning: | javac | 0m 39s | hbase-common generated 4 new + 36 unchanged - 0 fixed = 40 total (was 36) |
| +1 :green_heart: | checkstyle | 1m 9s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 21s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 56s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 51m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 10bacf221950 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-common.txt |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1800247752
I deployed this to one of our prod servers, which is an extreme case that typically spends about 12% of time deserializing Filters. With the patch, it spends less than 5% time now.
I also instrumented the initialization a bit:
```
ProtobufUtil: Took 377ms to create ClassPath
ReflectedFunctionCache: Took 138ms to initialize ReflectedFunctionCache for 27 Filters
ReflectedFunctionCache: Took 42ms to initialize ReflectedFunctionCache for 9 ByteArrayComparables
```
So it's about 5ms to create one of these, but the ClassPath.from() call is quite expensive and will depend on the size of the classpath (could be worse on clients that do other things).
I'm going to take a look at lazy loading these on demand. Barring that, I'll at the very least re-use one ClassPath object.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1802853945
I just pushed a commit which populates the cache on-demand, negating the need for any classpath traversal. I used our ConcurrentMapUtils.computeIfAbsent, which does a get followed by putIfAbsent. This means that under high concurrency it'd be possible to generate the function more than once, but this is ok -- it'll be GC'd.
I added some timings to the code, logged with debug logging. I ran this in prod, and creating the functions only takes 0-1ms in this context. I think it's faster than the old approach because we don't actually have to load the class (pass `false` to `Class.forName`).
So now we have no startup time problem, and extremely small upfront cost for the first time a filter/comparator is loaded.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789834308
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 45s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 48s | master passed |
| +1 :green_heart: | compile | 1m 27s | master passed |
| +1 :green_heart: | shadedjars | 5m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 40s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 29s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 33s | hbase-client in the patch passed. |
| -1 :x: | unit | 237m 3s | hbase-server in the patch failed. |
| | | 267m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 958a0535cc23 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/testReport/ |
| Max. process+thread count | 4790 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788104930
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 34s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 10s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 9s | the patch passed |
| +1 :green_heart: | compile | 1m 22s | the patch passed |
| +1 :green_heart: | javac | 1m 22s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 1s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 3s | hbase-common in the patch passed. |
| -1 :x: | unit | 1m 11s | hbase-client in the patch failed. |
| -1 :x: | unit | 230m 0s | hbase-server in the patch failed. |
| | | 262m 57s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 51bec871a833 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-client.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/testReport/ |
| Max. process+thread count | 5475 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1803101440
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 36s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 4m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
| -0 :warning: | patch | 6m 20s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 26s | the patch passed |
| +1 :green_heart: | javac | 1m 26s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 29s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 29s | hbase-client in the patch passed. |
| -1 :x: | unit | 224m 26s | hbase-server in the patch failed. |
| | | 254m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9d1a3050eb46 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 954a1f8fc3 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/testReport/ |
| Max. process+thread count | 4669 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1384678667
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -1552,13 +1571,23 @@ public static ComparatorProtos.Comparator toComparator(ByteArrayComparable compa
public static ByteArrayComparable toComparator(ComparatorProtos.Comparator proto)
throws IOException {
String type = proto.getName();
- String funcName = "parseFrom";
byte[] value = proto.getSerializedComparator().toByteArray();
+
try {
+ ByteArrayComparable result = COMPARATORS.getAndCallByName(type, value);
+ if (result != null) {
+ return result;
+ }
+
+ if (!ALLOW_FAST_REFLECTION_FALLTHROUGH) {
+ throw new IllegalStateException("Failed to deserialize comparator " + type
+ + " because fast reflection returned null and fallthrough is disabled");
+ }
Review Comment:
Should we add an else clause that ticks up a metric? Or maybe an emitted-once-per-`type` WARN that notifies the operator that we've fallen back to the slower path? I don't yet understand how often this might occur. Same for Filters.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -304,6 +305,24 @@ public static boolean isClassLoaderLoaded() {
return classLoaderLoaded;
}
+ private static final String PARSE_FROM = "parseFrom";
+
+ // We don't bother using the dynamic CLASS_LOADER above, because currently we can't support
+ // optimizing dynamically loaded classes. We can do it once we build for java9+, see the todo
+ // in ReflectedFunctionCache
+ private static final ReflectedFunctionCache<byte[], Filter> FILTERS = ReflectedFunctionCache
+ .create(ProtobufUtil.class.getClassLoader(), Filter.class, byte[].class, PARSE_FROM);
+ private static final ReflectedFunctionCache<byte[], ByteArrayComparable> COMPARATORS =
+ ReflectedFunctionCache.create(ProtobufUtil.class.getClassLoader(), ByteArrayComparable.class,
+ byte[].class, PARSE_FROM);
+
+ private static volatile boolean ALLOW_FAST_REFLECTION_FALLTHROUGH = true;
Review Comment:
Is there a reason to gate this feature behind a configuration point that's exposed to the operator?
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectedFunctionCache.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.reflect.ClassPath;
+
+/**
+ * Cache to hold resolved Functions generated through reflection. These can be costly to create, but
+ * then are much faster than typical Method.invoke calls when executing. Upon construction, finds
+ * all subclasses in the same package of the passed baseClass. For each found class, creates a
+ * lambda using
+ * {@link ReflectionUtils#getOneArgStaticMethodAsFunction(Class, String, Class, Class)}. These are
+ * added to a hashmap for fast lookup by name later.
+ * @param <I> the input argument type for the resolved functions
+ * @param <R> the return type for the resolved functions
+ */
+@InterfaceAudience.Private
+final public class ReflectedFunctionCache<I, R> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ReflectedFunctionCache.class);
+
+ private final Map<String, Function<I, ? extends R>> lambdasByClass;
+
+ private ReflectedFunctionCache(Map<String, Function<I, ? extends R>> lambdasByClass) {
+ this.lambdasByClass = lambdasByClass;
+ }
+
+ /**
+ * Create a cache of reflected functions using the provided classloader and baseClass. Will find
+ * all subclasses of the provided baseClass (in the same package), and then foreach look for a
+ * static one-arg method with the methodName and argClass. The expectation is that the method
+ * returns a value whose class extends the baseClass. This was primarily designed for use by our
+ * Filter and Comparator parseFrom methods.
+ */
+ public static <I, R> ReflectedFunctionCache<I, R> create(ClassLoader classLoader,
+ Class<R> baseClass, Class<I> argClass, String methodName) {
+ Map<String, Function<I, ? extends R>> lambdasByClass = new HashMap<>();
+ Set<? extends Class<? extends R>> classes = getSubclassesInPackage(classLoader, baseClass);
+ for (Class<? extends R> clazz : classes) {
+ Function<I, ? extends R> func = createFunction(clazz, methodName, argClass, clazz);
+ if (func != null) {
+ lambdasByClass.put(clazz.getName(), func);
+ }
+ }
+ return new ReflectedFunctionCache<>(lambdasByClass);
+ }
+
+ /**
+ * Get and execute the Function for the given className, passing the argument to the function and
+ * returning the result.
+ * @param className the full name of the class to lookup
+ * @param argument the argument to pass to the function, if found.
+ * @return null if a function is not found for classname, otherwise the result of the function.
+ */
+ public R getAndCallByName(String className, I argument) {
+ Function<I, ? extends R> lambda = lambdasByClass.get(className);
+
+ // todo: if we ever make java9+ our lowest supported jdk version, we can
+ // handle generating these for newly loaded classes from our DynamicClassLoader using
+ // MethodHandles.privateLookupIn(). For now this is not possible, because we can't easily
+ // create a privileged lookup in a non-default ClassLoader.
+ if (lambda == null) {
+ return null;
+ }
+
+ return lambda.apply(argument);
+ }
+
+ private static <R> Set<Class<? extends R>> getSubclassesInPackage(ClassLoader classLoader,
+ Class<R> baseClass) {
+ try {
+ return ClassPath.from(classLoader).getAllClasses().stream()
Review Comment:
Reading the code for `ClassPath#getAllClasses`, it seems that it will build an exhaustive set of all classes on the classpath that are loadable. It doesn't actually load the classes. My understanding is that `Stream` operations are lazy, so the only classes loaded should be those that materialize in the final `collect`.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -304,6 +305,24 @@ public static boolean isClassLoaderLoaded() {
return classLoaderLoaded;
}
+ private static final String PARSE_FROM = "parseFrom";
+
+ // We don't bother using the dynamic CLASS_LOADER above, because currently we can't support
+ // optimizing dynamically loaded classes. We can do it once we build for java9+, see the todo
+ // in ReflectedFunctionCache
+ private static final ReflectedFunctionCache<byte[], Filter> FILTERS = ReflectedFunctionCache
+ .create(ProtobufUtil.class.getClassLoader(), Filter.class, byte[].class, PARSE_FROM);
Review Comment:
Is the content of the directory specified in `hbase.dynamic.jars.dir` included in the classpath that is under the domain of this classloader? I think that if there are user-provided Filter classes in the path, we should load them. I guess that we cannot assume that they will be in the `o.a.h.h.filter` package, so we'd have to relax our class selection criteria.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788304130
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 33s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 4m 51s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 49s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 28s | hbase-common in the patch passed. |
| -1 :x: | unit | 1m 13s | hbase-client in the patch failed. |
| -1 :x: | unit | 230m 58s | hbase-server in the patch failed. |
| | | 260m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7a87f8fadcf5 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-client.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/testReport/ |
| Max. process+thread count | 4700 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789832158
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 22s | master passed |
| +1 :green_heart: | compile | 1m 24s | master passed |
| +1 :green_heart: | shadedjars | 6m 6s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 11s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | 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 | 6m 3s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 6s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 29s | hbase-client in the patch passed. |
| -1 :x: | unit | 231m 23s | hbase-server in the patch failed. |
| | | 264m 27s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 62c9edf53239 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/testReport/ |
| Max. process+thread count | 4601 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788306327
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 32s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed |
| +1 :green_heart: | shadedjars | 6m 3s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 3s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 11s | hbase-common in the patch passed. |
| -1 :x: | unit | 1m 11s | hbase-client in the patch failed. |
| -1 :x: | unit | 229m 59s | hbase-server in the patch failed. |
| | | 262m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8b889adcb3f0 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-client.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/testReport/ |
| Max. process+thread count | 4642 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1803106984
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 30s | master passed |
| +1 :green_heart: | compile | 1m 16s | master passed |
| +1 :green_heart: | shadedjars | 4m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | master passed |
| -0 :warning: | patch | 6m 18s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 24s | 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 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 53s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 19s | hbase-client in the patch passed. |
| -1 :x: | unit | 234m 30s | hbase-server in the patch failed. |
| | | 263m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c4aabd564642 5.4.0-156-generic #173-Ubuntu SMP Tue Jul 11 07:25:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 954a1f8fc3 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/testReport/ |
| Max. process+thread count | 4843 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1803308936
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 38s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 4m 56s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
| -0 :warning: | patch | 6m 21s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 46s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 29s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 33s | hbase-client in the patch passed. |
| -1 :x: | unit | 224m 11s | hbase-server in the patch failed. |
| | | 254m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ec30124f2a9a 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 4b5db21f3f |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/testReport/ |
| Max. process+thread count | 4665 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804373350
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 40s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 46s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 6m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 6s | master passed |
| -0 :warning: | patch | 7m 40s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 6s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 3s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 11s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 26s | hbase-client in the patch passed. |
| -1 :x: | unit | 227m 52s | hbase-server in the patch failed. |
| | | 261m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b8cc55d6cb1f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5dc4467e6c |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/testReport/ |
| Max. process+thread count | 4507 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789588375
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 40s | master passed |
| +1 :green_heart: | compile | 4m 9s | master passed |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | spotless | 0m 47s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 18s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 21s | the patch passed |
| +1 :green_heart: | compile | 4m 0s | the patch passed |
| +1 :green_heart: | javac | 4m 0s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 5s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 35s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 56s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 32s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 39s | The patch does not generate ASF License warnings. |
| | | 51m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 7c0306ae088a 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 76 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/4/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789343536
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 20s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 22s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 6m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 5s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 32s | hbase-client in the patch passed. |
| -1 :x: | unit | 232m 1s | hbase-server in the patch failed. |
| | | 264m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 13fff98a6d1d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/testReport/ |
| Max. process+thread count | 4805 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788978133
:broken_heart: **-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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 40s | master passed |
| +1 :green_heart: | compile | 4m 7s | master passed |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| -1 :x: | spotless | 0m 44s | branch has 1 errors when running spotless:check, run spotless:apply to fix. |
| +1 :green_heart: | spotbugs | 3m 6s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 23s | the patch passed |
| +1 :green_heart: | compile | 4m 2s | the patch passed |
| +1 :green_heart: | javac | 4m 2s | the patch passed |
| -0 :warning: | checkstyle | 0m 15s | hbase-common: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 35s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 57s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 51m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 0d089731ff51 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-general-check/output/branch-spotless.txt |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804838158
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 42s | master passed |
| +1 :green_heart: | compile | 1m 28s | master passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
| -0 :warning: | patch | 6m 22s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 26s | the patch passed |
| +1 :green_heart: | javac | 1m 26s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 57s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 30s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 29s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 250m 46s | hbase-server in the patch passed. |
| | | 281m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 18cffe96c7f3 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7f3921ae40 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/testReport/ |
| Max. process+thread count | 4759 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804509255
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 39s | master passed |
| +1 :green_heart: | compile | 4m 13s | master passed |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | spotless | 0m 47s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 7s | master passed |
| -0 :warning: | patch | 2m 6s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| +1 :green_heart: | compile | 4m 7s | the patch passed |
| +1 :green_heart: | javac | 4m 7s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 7s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 30s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 1m 3s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 42s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 51m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux bdd5c473181d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7f3921ae40 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/9/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1384834533
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -1552,13 +1571,23 @@ public static ComparatorProtos.Comparator toComparator(ByteArrayComparable compa
public static ByteArrayComparable toComparator(ComparatorProtos.Comparator proto)
throws IOException {
String type = proto.getName();
- String funcName = "parseFrom";
byte[] value = proto.getSerializedComparator().toByteArray();
+
try {
+ ByteArrayComparable result = COMPARATORS.getAndCallByName(type, value);
+ if (result != null) {
+ return result;
+ }
+
+ if (!ALLOW_FAST_REFLECTION_FALLTHROUGH) {
+ throw new IllegalStateException("Failed to deserialize comparator " + type
+ + " because fast reflection returned null and fallthrough is disabled");
+ }
Review Comment:
I only added this so that I could write tests, since this is all static methods.
I don't think we want a warn or counter here. How often it happens will depend on the usage of custom filters. If they don't use custom filters, this will never fail. If they use exclusively custom filters, then it will fail every time. It's not really a failure mode, more backwards compatibility handling.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1803141538
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 20s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 12s | master passed |
| +1 :green_heart: | compile | 4m 37s | master passed |
| +1 :green_heart: | checkstyle | 1m 33s | master passed |
| +1 :green_heart: | spotless | 1m 4s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 52s | master passed |
| -0 :warning: | patch | 2m 29s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 3s | the patch passed |
| +1 :green_heart: | compile | 4m 22s | the patch passed |
| +1 :green_heart: | javac | 4m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 39s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 58s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 52m 57s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux feb9345380be 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 4b5db21f3f |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/7/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1795951506
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 42s | master passed |
| +1 :green_heart: | compile | 1m 27s | master passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
| -0 :warning: | patch | 6m 23s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 46s | 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 | 4m 56s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 30s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 34s | hbase-client in the patch passed. |
| -1 :x: | unit | 233m 49s | hbase-server in the patch failed. |
| | | 264m 46s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b76a9591c5bc 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 027a119bcf |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/testReport/ |
| Max. process+thread count | 4720 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1383408208
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestComparatorSerialization.java:
##########
@@ -99,4 +129,51 @@ public void testBigDecimalComparator() throws Exception {
ProtobufUtil.toComparator(ProtobufUtil.toComparator(bigDecimalComparator))));
}
+ /**
+ * Test that we can load and deserialize custom comparators. Good to have generally, but also
+ * proves that this still works after HBASE-27276 despite not going through our fast function
+ * caches.
+ */
+ @Test
+ public void testCustomComparator() throws Exception {
+ ByteArrayComparable baseFilter = new BinaryComparator("foo".getBytes());
+ ComparatorProtos.Comparator proto = ProtobufUtil.toComparator(baseFilter);
+ String className = "CustomLoadedComparator" + allowFastReflectionFallthrough;
+ proto = proto.toBuilder().setName(className).build();
+
+ Configuration conf = HBaseConfiguration.create();
+ HBaseTestingUtil testUtil = new HBaseTestingUtil();
+ String dataTestDir = testUtil.getDataTestDir().toString();
+
+ // First make sure the test bed is clean, delete any pre-existing class.
+ // Below toComparator call is expected to fail because the comparator is not loaded now
+ ClassLoaderTestHelper.deleteClass(className, dataTestDir, conf);
+ try {
+ ProtobufUtil.toComparator(proto);
+ fail("expected to fail");
+ } catch (IOException e) {
+ // do nothing, this is expected
+ }
+
+ // Write a jar to be loaded into the classloader
+ String code = StringSubstitutor.replace(
+ IOUtils.toString(getClass().getResourceAsStream("/CustomLoadedComparator.java"),
+ Charset.defaultCharset()),
+ Collections.singletonMap("suffix", allowFastReflectionFallthrough));
+ ClassLoaderTestHelper.buildJar(dataTestDir, className, code,
+ ClassLoaderTestHelper.localDirPath(conf));
+
+ // Disallow fallthrough at first, we expect below to fail
+ ProtobufUtil.setAllowFastReflectionFallthrough(false);
Review Comment:
> Why disable fallthrough will lead to a failure here? It is because the class is not loaded when initializing, so it is not in the ReflectedFunctionCache?
Correct
> So we do not add special test for testing the scenario where ReflectedFunctionCache can work? As it is enabled by default so testing filter usage is enough?
Correct, we try the cache first. All of the tests in this class will run with setAllowFastReflectionFallthrough false and true. The false case is what matters -- If ReflectedFunctionCache were not working, the tests would fail because they can't fallback on the old reflection. So with disallowing fallback, we know that all of our filters and comparators can work with ReflectedFunctionCache.
I added this extra test to specially handle user custom filters, to ensure that they would still work despite not being supported by ReflectionFunctionCache.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1383396372
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectedFunctionCache.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.reflect.ClassPath;
+
+/**
+ * Cache to hold resolved Functions generated through reflection. These can be costly to create, but
+ * then are much faster than typical Method.invoke calls when executing. Upon construction, finds
+ * all subclasses in the same package of the passed baseClass. For each found class, creates a
+ * lambda using
+ * {@link ReflectionUtils#getOneArgStaticMethodAsFunction(Class, String, Class, Class)}. These are
+ * added to a hashmap for fast lookup by name later.
+ * @param <I> the input argument type for the resolved functions
+ * @param <R> the return type for the resolved functions
+ */
+@InterfaceAudience.Private
+final public class ReflectedFunctionCache<I, R> {
Review Comment:
I called it ReflectedFunction, because it holds Functions which were created through Reflection. Once created, I don't think it's considered reflection anymore. I will fix the ordering of `final public`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1789501357
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 44s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 14s | master passed |
| +1 :green_heart: | compile | 2m 14s | master passed |
| +1 :green_heart: | shadedjars | 7m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 18s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 48s | the patch passed |
| +1 :green_heart: | compile | 2m 2s | the patch passed |
| +1 :green_heart: | javac | 2m 2s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 31s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 2m 29s | hbase-client in the patch passed. |
| -1 :x: | unit | 334m 4s | hbase-server in the patch failed. |
| | | 376m 27s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a99b974e8a4a 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / fa4c8960e5 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/testReport/ |
| Max. process+thread count | 4943 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788152431
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 4m 10s | master passed |
| +1 :green_heart: | checkstyle | 1m 11s | master passed |
| +1 :green_heart: | spotless | 0m 47s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 17s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 20s | the patch passed |
| +1 :green_heart: | compile | 3m 54s | the patch passed |
| +1 :green_heart: | javac | 3m 54s | the patch passed |
| -0 :warning: | checkstyle | 0m 14s | hbase-common: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 38s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 1m 2s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
| | | 51m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux df27187ce65e 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/2/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1802933206
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 4m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 25s | master passed |
| +1 :green_heart: | spotless | 1m 3s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 6s | master passed |
| -0 :warning: | patch | 2m 35s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 53s | the patch passed |
| +1 :green_heart: | compile | 4m 26s | the patch passed |
| -0 :warning: | javac | 0m 38s | hbase-common generated 1 new + 36 unchanged - 0 fixed = 37 total (was 36) |
| +1 :green_heart: | checkstyle | 1m 11s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 50s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 54s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
| | | 52m 59s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 42aa67e689a5 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 954a1f8fc3 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/artifact/yetus-general-check/output/diff-compile-javac-hbase-common.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/6/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1383390765
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectionUtils.java:
##########
@@ -208,6 +214,30 @@ private static String getTaskName(long id, String name) {
return id + " (" + name + ")";
}
+ /**
+ * Creates a Function which can be called to performantly execute a reflected static method. The
+ * creation of the Function itself may not be fast, but executing that method thereafter should be
+ * much faster than {@link #invokeMethod(Object, String, Object...)}.
+ * @param lookupClazz the class to find the static method in
+ * @param methodName the method name
+ * @param argumentClazz the type of the argument
+ * @param returnValueClass the type of the return value
+ * @return a function which when called executes the requested static method.
+ * @throws Throwable exception types from the underlying reflection
+ */
+ public static <I, R> Function<I, R> getOneArgStaticMethodAsFunction(Class<?> lookupClazz,
+ String methodName, Class<I> argumentClazz, Class<R> returnValueClass) throws Throwable {
+ MethodHandles.Lookup lookup = MethodHandles.lookup();
+ MethodHandle methodHandle = lookup.findStatic(lookupClazz, methodName,
+ MethodType.methodType(returnValueClass, argumentClazz));
+ CallSite site =
+ LambdaMetafactory.metafactory(lookup, "apply", MethodType.methodType(Function.class),
+ methodHandle.type().generic(), methodHandle, methodHandle.type());
+
+ return (Function<I, R>) site.getTarget().invokeExact();
Review Comment:
Correct, there's a good amount of content on google around LambdaMetafactory. Basically it's a new feature in java 8 which exposes the methods used by java for creating functional interfaces from lambdas, for us to use in reflection. Once created, the invocation time is nearly as fast as native java code.
So the code above is specifying the the signature of a Function's apply method, which takes a byte[] and returns a Filter. I wrote it using generics so that we could possibly plug this into other high-throughput reflection cases we have.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1383374974
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectedFunctionCache.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.reflect.ClassPath;
+
+/**
+ * Cache to hold resolved Functions generated through reflection. These can be costly to create, but
+ * then are much faster than typical Method.invoke calls when executing. Upon construction, finds
+ * all subclasses in the same package of the passed baseClass. For each found class, creates a
+ * lambda using
+ * {@link ReflectionUtils#getOneArgStaticMethodAsFunction(Class, String, Class, Class)}. These are
+ * added to a hashmap for fast lookup by name later.
+ * @param <I> the input argument type for the resolved functions
+ * @param <R> the return type for the resolved functions
+ */
+@InterfaceAudience.Private
+final public class ReflectedFunctionCache<I, R> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ReflectedFunctionCache.class);
+
+ private final Map<String, Function<I, ? extends R>> lambdasByClass;
+
+ private ReflectedFunctionCache(Map<String, Function<I, ? extends R>> lambdasByClass) {
+ this.lambdasByClass = lambdasByClass;
+ }
+
+ /**
+ * Create a cache of reflected functions using the provided classloader and baseClass. Will find
+ * all subclasses of the provided baseClass (in the same package), and then foreach look for a
+ * static one-arg method with the methodName and argClass. The expectation is that the method
+ * returns a value whose class extends the baseClass. This was primarily designed for use by our
+ * Filter and Comparator parseFrom methods.
+ */
+ public static <I, R> ReflectedFunctionCache<I, R> create(ClassLoader classLoader,
+ Class<R> baseClass, Class<I> argClass, String methodName) {
+ Map<String, Function<I, ? extends R>> lambdasByClass = new HashMap<>();
+ Set<? extends Class<? extends R>> classes = getSubclassesInPackage(classLoader, baseClass);
+ for (Class<? extends R> clazz : classes) {
+ Function<I, ? extends R> func = createFunction(clazz, methodName, argClass, clazz);
+ if (func != null) {
+ lambdasByClass.put(clazz.getName(), func);
+ }
+ }
+ return new ReflectedFunctionCache<>(lambdasByClass);
+ }
+
+ /**
+ * Get and execute the Function for the given className, passing the argument to the function and
+ * returning the result.
+ * @param className the full name of the class to lookup
+ * @param argument the argument to pass to the function, if found.
+ * @return null if a function is not found for classname, otherwise the result of the function.
+ */
+ public R getAndCallByName(String className, I argument) {
+ Function<I, ? extends R> lambda = lambdasByClass.get(className);
+
+ // todo: if we ever make java9+ our lowest supported jdk version, we can
+ // handle generating these for newly loaded classes from our DynamicClassLoader using
+ // MethodHandles.privateLookupIn(). For now this is not possible, because we can't easily
+ // create a privileged lookup in a non-default ClassLoader.
+ if (lambda == null) {
+ return null;
+ }
+
+ return lambda.apply(argument);
+ }
+
+ private static <R> Set<Class<? extends R>> getSubclassesInPackage(ClassLoader classLoader,
+ Class<R> baseClass) {
+ try {
+ return ClassPath.from(classLoader).getAllClasses().stream()
Review Comment:
It will cause us to load all of the matching ones, i.e. the ones i call `load` on below after filtering to the correct package. So it will load all of the filters in `org.apache.hadoop.hbase.filter` on startup.
I thought this was preferable because the number of classes is not large. Since I do it on startup, I don't need to worry about synchronization. I could only populate the cache as Filters are accessed, but then I need to handle synchronization. Would you prefer that?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1796230994
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 52s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed |
| +1 :green_heart: | shadedjars | 6m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
| -0 :warning: | patch | 7m 43s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 8s | the patch passed |
| +1 :green_heart: | compile | 1m 32s | the patch passed |
| +1 :green_heart: | javac | 1m 32s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 7s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 32s | hbase-client in the patch passed. |
| -1 :x: | unit | 279m 22s | hbase-server in the patch failed. |
| | | 315m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 48906a604838 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 027a119bcf |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/testReport/ |
| Max. process+thread count | 4651 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1383316307
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectedFunctionCache.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.reflect.ClassPath;
+
+/**
+ * Cache to hold resolved Functions generated through reflection. These can be costly to create, but
+ * then are much faster than typical Method.invoke calls when executing. Upon construction, finds
+ * all subclasses in the same package of the passed baseClass. For each found class, creates a
+ * lambda using
+ * {@link ReflectionUtils#getOneArgStaticMethodAsFunction(Class, String, Class, Class)}. These are
+ * added to a hashmap for fast lookup by name later.
+ * @param <I> the input argument type for the resolved functions
+ * @param <R> the return type for the resolved functions
+ */
+@InterfaceAudience.Private
+final public class ReflectedFunctionCache<I, R> {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ReflectedFunctionCache.class);
+
+ private final Map<String, Function<I, ? extends R>> lambdasByClass;
+
+ private ReflectedFunctionCache(Map<String, Function<I, ? extends R>> lambdasByClass) {
+ this.lambdasByClass = lambdasByClass;
+ }
+
+ /**
+ * Create a cache of reflected functions using the provided classloader and baseClass. Will find
+ * all subclasses of the provided baseClass (in the same package), and then foreach look for a
+ * static one-arg method with the methodName and argClass. The expectation is that the method
+ * returns a value whose class extends the baseClass. This was primarily designed for use by our
+ * Filter and Comparator parseFrom methods.
+ */
+ public static <I, R> ReflectedFunctionCache<I, R> create(ClassLoader classLoader,
+ Class<R> baseClass, Class<I> argClass, String methodName) {
+ Map<String, Function<I, ? extends R>> lambdasByClass = new HashMap<>();
+ Set<? extends Class<? extends R>> classes = getSubclassesInPackage(classLoader, baseClass);
+ for (Class<? extends R> clazz : classes) {
+ Function<I, ? extends R> func = createFunction(clazz, methodName, argClass, clazz);
+ if (func != null) {
+ lambdasByClass.put(clazz.getName(), func);
+ }
+ }
+ return new ReflectedFunctionCache<>(lambdasByClass);
+ }
+
+ /**
+ * Get and execute the Function for the given className, passing the argument to the function and
+ * returning the result.
+ * @param className the full name of the class to lookup
+ * @param argument the argument to pass to the function, if found.
+ * @return null if a function is not found for classname, otherwise the result of the function.
+ */
+ public R getAndCallByName(String className, I argument) {
+ Function<I, ? extends R> lambda = lambdasByClass.get(className);
+
+ // todo: if we ever make java9+ our lowest supported jdk version, we can
+ // handle generating these for newly loaded classes from our DynamicClassLoader using
+ // MethodHandles.privateLookupIn(). For now this is not possible, because we can't easily
+ // create a privileged lookup in a non-default ClassLoader.
+ if (lambda == null) {
+ return null;
+ }
+
+ return lambda.apply(argument);
+ }
+
+ private static <R> Set<Class<? extends R>> getSubclassesInPackage(ClassLoader classLoader,
+ Class<R> baseClass) {
+ try {
+ return ClassPath.from(classLoader).getAllClasses().stream()
Review Comment:
Will this cause we load all classes even if it is not used by now? I used to use guava's ClassPath in a project but it performed differently when executing in IDE and in command line, finally I chose to use ClassPathScanningCandidateComponentProvider in spring for scanning classes...
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectedFunctionCache.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.util;
+
+import java.io.IOException;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.reflect.ClassPath;
+
+/**
+ * Cache to hold resolved Functions generated through reflection. These can be costly to create, but
+ * then are much faster than typical Method.invoke calls when executing. Upon construction, finds
+ * all subclasses in the same package of the passed baseClass. For each found class, creates a
+ * lambda using
+ * {@link ReflectionUtils#getOneArgStaticMethodAsFunction(Class, String, Class, Class)}. These are
+ * added to a hashmap for fast lookup by name later.
+ * @param <I> the input argument type for the resolved functions
+ * @param <R> the return type for the resolved functions
+ */
+@InterfaceAudience.Private
+final public class ReflectedFunctionCache<I, R> {
Review Comment:
Use 'public final'. And is it better to call it 'ReflectionFunctionCache'? Anyway, not a native English speaker, feel free to choose the one you like.
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ReflectionUtils.java:
##########
@@ -208,6 +214,30 @@ private static String getTaskName(long id, String name) {
return id + " (" + name + ")";
}
+ /**
+ * Creates a Function which can be called to performantly execute a reflected static method. The
+ * creation of the Function itself may not be fast, but executing that method thereafter should be
+ * much faster than {@link #invokeMethod(Object, String, Object...)}.
+ * @param lookupClazz the class to find the static method in
+ * @param methodName the method name
+ * @param argumentClazz the type of the argument
+ * @param returnValueClass the type of the return value
+ * @return a function which when called executes the requested static method.
+ * @throws Throwable exception types from the underlying reflection
+ */
+ public static <I, R> Function<I, R> getOneArgStaticMethodAsFunction(Class<?> lookupClazz,
+ String methodName, Class<I> argumentClazz, Class<R> returnValueClass) throws Throwable {
+ MethodHandles.Lookup lookup = MethodHandles.lookup();
+ MethodHandle methodHandle = lookup.findStatic(lookupClazz, methodName,
+ MethodType.methodType(returnValueClass, argumentClazz));
+ CallSite site =
+ LambdaMetafactory.metafactory(lookup, "apply", MethodType.methodType(Function.class),
+ methodHandle.type().generic(), methodHandle, methodHandle.type());
+
+ return (Function<I, R>) site.getTarget().invokeExact();
Review Comment:
So this is the magic here? Somehow we can convert a Method to a Function, so the invocation will be faster?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestComparatorSerialization.java:
##########
@@ -99,4 +129,51 @@ public void testBigDecimalComparator() throws Exception {
ProtobufUtil.toComparator(ProtobufUtil.toComparator(bigDecimalComparator))));
}
+ /**
+ * Test that we can load and deserialize custom comparators. Good to have generally, but also
+ * proves that this still works after HBASE-27276 despite not going through our fast function
+ * caches.
+ */
+ @Test
+ public void testCustomComparator() throws Exception {
+ ByteArrayComparable baseFilter = new BinaryComparator("foo".getBytes());
+ ComparatorProtos.Comparator proto = ProtobufUtil.toComparator(baseFilter);
+ String className = "CustomLoadedComparator" + allowFastReflectionFallthrough;
+ proto = proto.toBuilder().setName(className).build();
+
+ Configuration conf = HBaseConfiguration.create();
+ HBaseTestingUtil testUtil = new HBaseTestingUtil();
+ String dataTestDir = testUtil.getDataTestDir().toString();
+
+ // First make sure the test bed is clean, delete any pre-existing class.
+ // Below toComparator call is expected to fail because the comparator is not loaded now
+ ClassLoaderTestHelper.deleteClass(className, dataTestDir, conf);
+ try {
+ ProtobufUtil.toComparator(proto);
+ fail("expected to fail");
+ } catch (IOException e) {
+ // do nothing, this is expected
+ }
+
+ // Write a jar to be loaded into the classloader
+ String code = StringSubstitutor.replace(
+ IOUtils.toString(getClass().getResourceAsStream("/CustomLoadedComparator.java"),
+ Charset.defaultCharset()),
+ Collections.singletonMap("suffix", allowFastReflectionFallthrough));
+ ClassLoaderTestHelper.buildJar(dataTestDir, className, code,
+ ClassLoaderTestHelper.localDirPath(conf));
+
+ // Disallow fallthrough at first, we expect below to fail
+ ProtobufUtil.setAllowFastReflectionFallthrough(false);
Review Comment:
Why disable fallthrough will lead to a failure here? It is because the class is not loaded when initializing, so it is not in the ReflectedFunctionCache?
So we do not add special test for testing the scenario where ReflectedFunctionCache can work? As it is enabled by default so testing filter usage is enough?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestComparatorSerialization.java:
##########
@@ -99,4 +129,51 @@ public void testBigDecimalComparator() throws Exception {
ProtobufUtil.toComparator(ProtobufUtil.toComparator(bigDecimalComparator))));
}
+ /**
+ * Test that we can load and deserialize custom comparators. Good to have generally, but also
+ * proves that this still works after HBASE-27276 despite not going through our fast function
+ * caches.
+ */
+ @Test
+ public void testCustomComparator() throws Exception {
+ ByteArrayComparable baseFilter = new BinaryComparator("foo".getBytes());
+ ComparatorProtos.Comparator proto = ProtobufUtil.toComparator(baseFilter);
+ String className = "CustomLoadedComparator" + allowFastReflectionFallthrough;
+ proto = proto.toBuilder().setName(className).build();
+
+ Configuration conf = HBaseConfiguration.create();
+ HBaseTestingUtil testUtil = new HBaseTestingUtil();
+ String dataTestDir = testUtil.getDataTestDir().toString();
+
+ // First make sure the test bed is clean, delete any pre-existing class.
+ // Below toComparator call is expected to fail because the comparator is not loaded now
+ ClassLoaderTestHelper.deleteClass(className, dataTestDir, conf);
+ try {
+ ProtobufUtil.toComparator(proto);
+ fail("expected to fail");
+ } catch (IOException e) {
+ // do nothing, this is expected
+ }
+
+ // Write a jar to be loaded into the classloader
+ String code = StringSubstitutor.replace(
+ IOUtils.toString(getClass().getResourceAsStream("/CustomLoadedComparator.java"),
Review Comment:
Better append a suffix like '.template' to the file name
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1788102656
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 49s | master passed |
| +1 :green_heart: | compile | 1m 29s | master passed |
| +1 :green_heart: | shadedjars | 4m 51s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 0s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 48s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 29s | hbase-common in the patch passed. |
| -1 :x: | unit | 1m 13s | hbase-client in the patch failed. |
| -1 :x: | unit | 231m 11s | hbase-server in the patch failed. |
| | | 261m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a1f4dfd34149 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 208e9b1a82 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-client.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/testReport/ |
| Max. process+thread count | 4686 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1384840873
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -304,6 +305,24 @@ public static boolean isClassLoaderLoaded() {
return classLoaderLoaded;
}
+ private static final String PARSE_FROM = "parseFrom";
+
+ // We don't bother using the dynamic CLASS_LOADER above, because currently we can't support
+ // optimizing dynamically loaded classes. We can do it once we build for java9+, see the todo
+ // in ReflectedFunctionCache
+ private static final ReflectedFunctionCache<byte[], Filter> FILTERS = ReflectedFunctionCache
+ .create(ProtobufUtil.class.getClassLoader(), Filter.class, byte[].class, PARSE_FROM);
+ private static final ReflectedFunctionCache<byte[], ByteArrayComparable> COMPARATORS =
+ ReflectedFunctionCache.create(ProtobufUtil.class.getClassLoader(), ByteArrayComparable.class,
+ byte[].class, PARSE_FROM);
+
+ private static volatile boolean ALLOW_FAST_REFLECTION_FALLTHROUGH = true;
Review Comment:
I don't think so. It's explicitly better on all supported versions of Java, with no downside. It seems like one of those unnecessary configs that hangs around forever. Also it'd be complicated to pipe a config in there since it's multiple levels of static only methods (for example recursive toFilter calls in FilterList.parsefrom)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1384836073
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -304,6 +305,24 @@ public static boolean isClassLoaderLoaded() {
return classLoaderLoaded;
}
+ private static final String PARSE_FROM = "parseFrom";
+
+ // We don't bother using the dynamic CLASS_LOADER above, because currently we can't support
+ // optimizing dynamically loaded classes. We can do it once we build for java9+, see the todo
+ // in ReflectedFunctionCache
+ private static final ReflectedFunctionCache<byte[], Filter> FILTERS = ReflectedFunctionCache
+ .create(ProtobufUtil.class.getClassLoader(), Filter.class, byte[].class, PARSE_FROM);
Review Comment:
Correct we'd need to expand our package search, which may lead to increased start times.
I've had this branch hanging around for a long time, I'd like to get this shipped and then we can tackle custom filters in a follow up when I or someone has time
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804022713
: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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 41s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | compile | 4m 11s | master passed |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | spotless | 0m 46s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 15s | master passed |
| -0 :warning: | patch | 2m 9s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 27s | the patch passed |
| +1 :green_heart: | compile | 3m 59s | the patch passed |
| +1 :green_heart: | javac | 3m 59s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 6s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 56s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 1m 5s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 4m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 52m 51s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 71db3c3988ea 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5dc4467e6c |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5488:
URL: https://github.com/apache/hbase/pull/5488#discussion_r1384834533
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -1552,13 +1571,23 @@ public static ComparatorProtos.Comparator toComparator(ByteArrayComparable compa
public static ByteArrayComparable toComparator(ComparatorProtos.Comparator proto)
throws IOException {
String type = proto.getName();
- String funcName = "parseFrom";
byte[] value = proto.getSerializedComparator().toByteArray();
+
try {
+ ByteArrayComparable result = COMPARATORS.getAndCallByName(type, value);
+ if (result != null) {
+ return result;
+ }
+
+ if (!ALLOW_FAST_REFLECTION_FALLTHROUGH) {
+ throw new IllegalStateException("Failed to deserialize comparator " + type
+ + " because fast reflection returned null and fallthrough is disabled");
+ }
Review Comment:
I only added this so that I could write tests, since this is all static methods.
I don't think we want a warn or counter here. How often it happens will depend on the usage of custom filters. If they don't use custom filters, this will never fail. If they use exclusively custom filters, then it will fail every time. It's not really a failure mode, more backwards compatibility handling.
In fact, this specific exception will never fire outside of tests. The above call to getAndCallByName will "fail" (return null) for custom filters, and cleanly fallback to the below older handling.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804131667
I think this is ready for merge, if anyone has any other feedback since the move to on-demand loading.
Note I decided to use a ConcurrentHashMap rather than LoadingCache or Caffeine Cache, because we don't need any of the extra features of those. The total number of cached items is small and constant, with no eviction or expiration.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5488:
URL: https://github.com/apache/hbase/pull/5488#issuecomment-1804361882
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 47s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 4m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | master passed |
| -0 :warning: | patch | 6m 23s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 26s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 29s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 1m 28s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 227m 13s | hbase-server in the patch passed. |
| | | 257m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5488 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6e2cc5f02825 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5dc4467e6c |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/testReport/ |
| Max. process+thread count | 4722 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5488/8/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-27276 Reduce reflection overhead in Filter deserialization [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault merged PR #5488:
URL: https://github.com/apache/hbase/pull/5488
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org