You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/01/04 12:09:04 UTC
[GitHub] [phoenix] virajjasani opened a new pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
virajjasani opened a new pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754695198
:broken_heart: **-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. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 14m 16s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | checkstyle | 3m 16s | master passed |
| +1 :green_heart: | javadoc | 0m 45s | master passed |
| +0 :ok: | spotbugs | 2m 55s | phoenix-core in master has 973 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 29s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| -1 :x: | checkstyle | 3m 30s | phoenix-core: The patch generated 22 new + 7179 unchanged - 26 fixed = 7201 total (was 7205) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 0s | phoenix-core generated 0 new + 957 unchanged - 16 fixed = 957 total (was 973) |
||| _ Other Tests _ |
| -1 :x: | unit | 107m 0s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. |
| | | 148m 31s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1053 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 8442a2837389 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 0bac803 |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/3/testReport/ |
| Max. process+thread count | 6521 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/3/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754439718
Yeah sounds better, having the class implement `Serializable` is not going to harm anyways.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-755900715
@gjacoby126 @stoty Could you please help merge this PR and 4.x backport #1054 (and 4.16 backport too as 4.x and 4.16 are exactly same as of now)?
After approval, the only change done is to let `MutationComparator` implement `Serializable` also (to silence spotbug warning we saw earlier).
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754617107
My bad, I think it's different, build did not procceed beyond `mvn install` which is failing with:
```
error waiting for container: unexpected EOF
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754088899
Pushed new version to fix RAT confusion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754436022
As we don't seem to use the java serialization framework anywhere, and your patch doesn't change anything WRT the java serialization situation, It doesn't really matter.
You may as well make the class Serializable, as it has no state, and it wouldn't make any difference to us, and silence spotbugs, but it's good either way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754016163
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 20s | 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. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 14m 24s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | checkstyle | 3m 27s | master passed |
| +1 :green_heart: | javadoc | 0m 46s | master passed |
| +0 :ok: | spotbugs | 2m 55s | phoenix-core in master has 973 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 29s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| -1 :x: | checkstyle | 3m 30s | phoenix-core: The patch generated 69 new + 7132 unchanged - 73 fixed = 7201 total (was 7205) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 44s | the patch passed |
| -1 :x: | spotbugs | 3m 9s | phoenix-core generated 1 new + 957 unchanged - 16 fixed = 958 total (was 973) |
||| _ Other Tests _ |
| -1 :x: | unit | 110m 14s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 29s | The patch does not generate ASF License warnings. |
| | | 156m 11s | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:phoenix-core |
| | org.apache.phoenix.hbase.index.covered.update.IndexUpdateManager$MutationComparator implements Comparator but not Serializable At IndexUpdateManager.java:Serializable At IndexUpdateManager.java:[lines 47-94] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1053 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 8334d017343d 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 4d5449f |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/testReport/ |
| Max. process+thread count | 6686 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1053/1/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani edited a comment on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754615238
Weird, recent builds for both PRs master and 4.x did not succeed to post build status on github:
```
Could not update commit status, please check if your scan credentials belong to a member of the organization or a collaborator of the repository and repo:status scope is selected
GitHub has been notified of this commit’s build result
```
@stoty seems like similar message to what we used to get on PR 945
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi merged pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
yanxinyi merged pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani edited a comment on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-755900715
@gjacoby126 @stoty Could you please help merge this PR and 4.x backport #1054 (and 4.16 backport as 4.x and 4.16 are exactly same as of today. If required, I can raise 4.16 PR )?
After approval, the only change done is to let `MutationComparator` implement `Serializable`(to silence spotbug warning we saw earlier).
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754429561
Although findbugs has valid point that we should let `MutationComparator` implement `Serializable` because TreeMaps are serializable, but our treeMap that uses this comparator doesn't seem to be getting (de)serialized in any flow and hence we can ignore this findbug warning? Thoughts @gjacoby126 @stoty ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754615238
Weird, recent builds for both PRs master and 4.x did not succeed to post build status on github:
```
Could not update commit status, please check if your scan credentials belong to a member of the organization or a collaborator of the repository and repo:status scope is selected
GitHub has been notified of this commit’s build result
```
@stoty seems like similar message to what we used to get on PR #945
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-754618515
We also had multibranch failures today, there seems to a problem or maintanence on the Jenkins infra.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1053: PHOENIX-6295 : Fix non-static inner classes for better memory utilization
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1053:
URL: https://github.com/apache/phoenix/pull/1053#issuecomment-756564131
Thanks for merging this @yanxinyi . Could you please also backport 4.x commit to 4.16 as I was expecting to include this in 4.16. Let me know if you want me to create PR, I can keep it ready.
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org