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/18 07:10:16 UTC

[GitHub] [phoenix] virajjasani opened a new pull request #1096: PHOENIX-5296 : refCount leak checks

virajjasani opened a new pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096


   


----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

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


   Let me also upload patch over Jira in the meanwhile.


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r563503258



##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       Could you solve this without Guava ?
   I know that it doesn't really matter, but it pains me to see anther Guava dependency added.




----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolTimeRangeIT.java
##########
@@ -226,7 +225,8 @@ public void tickTime() {
     }
 
     @AfterClass
-    public static synchronized void teardown() {
+    public static synchronized void teardown() throws Exception {
+        confirmStoreRefCountLeak();

Review comment:
       Oh btw we don't need to worry about putting anything in finally after calling BaseTest#confirmStoreRefCountLeak() because BaseTest is taking care of shutting down miniCluster:
   ```
       protected synchronized static void confirmStoreRefCountLeak()
               throws Exception {
           if (getUtility() != null) {
               try {
                   CompatUtil.confirmStoreRefCountLeak(
                       getUtility().getAdmin());
               } catch (IOException e) {
                   LOGGER.error("StoreFile refCount is leaked", e);
                   freeResources(false);
                   fail("StoreFile refCount is leaked");
               }
           }
       }
   ```
   freeResources() is doing the job.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762396870


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 33s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  12m 52s |  master passed  |
   | +0 |  hbaserecompile  |  37m 12s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 38s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m  1s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m  3s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 44s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 30s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 43s |  root: The patch generated 13 new + 124 unchanged - 2 fixed = 137 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   2m  6s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   6m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 115m 48s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  6s |  The patch does not generate ASF License warnings.  |
   |  |   | 196m 49s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.ViewTTLIT |
   
   
   | 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-1096/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
   | uname | Linux cf75a3ddf5bb 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | 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-1096/4/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/4/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/4/testReport/ |
   | Max. process+thread count | 10342 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/4/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-767449453


   One more nit: Please update the commit message to match the JIRA description.


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559497588



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       I think that throwing an exception here may interfere with the cluster shutdown logic that this is called from.
   
   Either catch and log it (and fail the test), or make sure that it doesn't preclude cluster shutdown at every invocation point.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-763929219


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 29s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m  9s |  master passed  |
   | +0 |  hbaserecompile  |  22m 14s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 37s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m  3s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m 18s |  the patch passed  |
   | +0 |  hbaserecompile  |  15m 42s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 42s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.2.1 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 16s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 127m 38s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  6s |  The patch does not generate ASF License warnings.  |
   |  |   | 194m 28s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.IndexScrutinyToolIT |
   
   
   | 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-1096/14/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 36126ff454f0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 772ff2c |
   | 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-1096/14/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/14/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/14/testReport/ |
   | Max. process+thread count | 13647 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/14/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 #1096: PHOENIX-5296 : refCount leak checks

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


   @stoty Thanks for the suggestion, i have updated PR for `hbase-compat` modules to get rid of guava dependency. Any other suggestions you have in mind?


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2015,21 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     */
+    protected synchronized static void confirmStoreRefCountLeak()
+            throws Exception {
+        if (getUtility() != null) {
+            try {
+                CompatUtil.confirmStoreRefCountLeak(
+                    getUtility().getAdmin());
+            } catch (IOException e) {
+                LOGGER.error("StoreFile refCount is leaked", e);

Review comment:
       Okk I think we better let cluster shutdown happen the way it does today. We better use `finally` with catching storeFile refCount leak assertion catch so that not only we fail test but also let resources be freed up in individual test's `finally` block. I was thinking of taking care of cluster shutdown in `BaseTest` only as generic check, but now it seems better let individual test decide what they want to free up - cluster or just drop table or anything as per it's current behaviour.
   The only problem with current approach (generic freeup resource in `BaseTest`) is that if refCount leakage is encountered in one test and if we shutdown miniCluster in `@After`, other tests in that class will not run anyways.
   
   @stoty you are also fine with having `finally` in each test right?




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-765559907


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 34s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  13m  9s |  master passed  |
   | +0 |  hbaserecompile  |  24m 16s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 28s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 40s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  9s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 32s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 58s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m  8s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 39s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 28s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 43s |  root: The patch generated 17 new + 108 unchanged - 0 fixed = 125 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  6s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 13s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 106m 28s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m 10s |  The patch does not generate ASF License warnings.  |
   |  |   | 174m 33s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.UpsertSelectIT |
   
   
   | 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-1096/20/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux e66790ac296b 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / d5e93b3 |
   | 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-1096/20/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/20/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/20/testReport/ |
   | Max. process+thread count | 10814 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/20/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] stoty commented on pull request #1096: PHOENIX-5296 : Ensure store file reader refcount is zero at end of relevant unit tests

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-767522020


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 34s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m  6s |  master passed  |
   | +0 |  hbaserecompile  |  22m 35s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 42s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  6s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 59s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m 23s |  the patch passed  |
   | +0 |  hbaserecompile  |  15m  8s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 41s |  root: The patch generated 23 new + 108 unchanged - 0 fixed = 131 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  5s |  the patch passed  |
   | -1 :x: |  spotbugs  |   0m 48s |  phoenix-hbase-compat-2.4.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | -1 :x: |  spotbugs  |   0m 48s |  phoenix-hbase-compat-2.3.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 15s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 118m 36s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  9s |  The patch does not generate ASF License warnings.  |
   |  |   | 181m 49s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-hbase-compat-2.4.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 109] |
   | FindBugs | module:phoenix-hbase-compat-2.3.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 108] |
   
   
   | 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-1096/27/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 4af870731b1a 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / ba47233 |
   | 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-1096/27/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/27/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.4.0.html |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/27/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.3.0.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/27/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/27/testReport/ |
   | Max. process+thread count | 11156 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/27/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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       Let me update 4.x PR in the meanwhile.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

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


   Build is timing out, not sure if relevant.


----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762291894


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 49s |  master passed  |
   | +0 |  hbaserecompile  |  27m 55s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 42s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  5s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 56s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | -1 :x: |  mvninstall  |   1m  6s |  root in the patch failed.  |
   | +0 |  hbaserecompile  |   7m 28s |  HBase recompiled.  |
   | -1 :x: |  compile  |   1m  9s |  root in the patch failed.  |
   | -1 :x: |  javac  |   1m  9s |  root in the patch failed.  |
   | -1 :x: |  checkstyle  |   0m 41s |  root: The patch generated 13 new + 124 unchanged - 2 fixed = 137 total (was 126)  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch 31 line(s) with tabs.  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  the patch passed  |
   | -1 :x: |  spotbugs  |   0m 59s |  phoenix-core in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   1m 15s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  8s |  The patch does not generate ASF License warnings.  |
   |  |   |  59m 11s |   |
   
   
   | 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-1096/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
   | uname | Linux 9b885b9fd88e 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | mvninstall | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
   | compile | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/patch-compile-root.txt |
   | javac | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/patch-compile-root.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | whitespace | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/whitespace-tabs.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/patch-spotbugs-phoenix-core.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/3/testReport/ |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-763388825


   Yout may want to rebase these on HEAD, in case the Xmx setting is the culprit.


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2015,21 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     */
+    protected synchronized static void confirmStoreRefCountLeak()
+            throws Exception {
+        if (getUtility() != null) {
+            try {
+                CompatUtil.confirmStoreRefCountLeak(
+                    getUtility().getAdmin());
+            } catch (IOException e) {
+                LOGGER.error("StoreFile refCount is leaked", e);

Review comment:
       Okk I think we better let cluster shutdown happen the way it does today. We better use `finally` with catching storeFile refCount leak assertion catch so that not only we fail test but also let resources be freed up in individual test's `finally` block. I was thinking of taking care of cluster shutdown in `BaseTest` only as generic check, but now it seems better let individual test decide what they want to free up - cluster or just drop table or anything as per it's current behaviour.
   The only problem with current approach is that if refCount leakage is encountered in one test and if we shutdown miniCluster in `@After`, other tests will not run anyways.
   
   @stoty you are also fine with having `finally` in each test right?




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762537875


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 56s |  master passed  |
   | +0 |  hbaserecompile  |  23m  1s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 29s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m  1s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   7m 58s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 18s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 31s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 43s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  6s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.2.1 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 22s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m 27s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  6s |  The patch does not generate ASF License warnings.  |
   |  |   | 179m 21s |   |
   
   
   | 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-1096/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 62a9abc0c81b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | 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-1096/8/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/8/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/8/testReport/ |
   | Max. process+thread count | 5761 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/8/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762420273


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 54s |  master passed  |
   | +0 |  hbaserecompile  |  23m  9s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 44s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 39s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m 43s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | -1 :x: |  mvninstall  |   0m 23s |  root in the patch failed.  |
   | +0 |  hbaserecompile  |   8m  2s |  HBase recompiled.  |
   | -1 :x: |  compile  |   0m 22s |  root in the patch failed.  |
   | -1 :x: |  javac  |   0m 22s |  root in the patch failed.  |
   | -1 :x: |  checkstyle  |   0m 45s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | -1 :x: |  javadoc  |   0m 18s |  phoenix-hbase-compat-2.4.0 generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   | -1 :x: |  javadoc  |   0m 20s |  phoenix-hbase-compat-2.3.0 generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)  |
   | -1 :x: |  spotbugs  |   0m 21s |  phoenix-hbase-compat-2.4.0 in the patch failed.  |
   | -1 :x: |  spotbugs  |   0m 22s |  phoenix-hbase-compat-2.3.0 in the patch failed.  |
   | -1 :x: |  spotbugs  |   1m  5s |  phoenix-core in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 36s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  8s |  The patch does not generate ASF License warnings.  |
   |  |   |  53m 57s |   |
   
   
   | 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-1096/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
   | uname | Linux ad091ddfbce4 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | mvninstall | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
   | compile | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-compile-root.txt |
   | javac | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-compile-root.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | javadoc | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/diff-javadoc-javadoc-phoenix-hbase-compat-2.4.0.txt |
   | javadoc | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/diff-javadoc-javadoc-phoenix-hbase-compat-2.3.0.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-spotbugs-phoenix-hbase-compat-2.4.0.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-spotbugs-phoenix-hbase-compat-2.3.0.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-spotbugs-phoenix-core.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/testReport/ |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/5/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] stoty commented on a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559476681



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+

Review comment:
       Please move this to CompatUtil, the HbaseCompatCapabilities is for bool flags only.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

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


   Let me resolve the conflicts


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559608940



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2015,21 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     */
+    protected synchronized static void confirmStoreRefCountLeak()
+            throws Exception {
+        if (getUtility() != null) {
+            try {
+                CompatUtil.confirmStoreRefCountLeak(
+                    getUtility().getAdmin());
+            } catch (IOException e) {
+                LOGGER.error("StoreFile refCount is leaked", e);

Review comment:
       This is a rather unexpected side effect. I am not a fan of having the freeResources logic called from a different place in the normal and the error path.
   
   Maybe call this method freeResourcesAndCheckRefCountLeak, and 
   call freeResources unconditionally from from finally here ?




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

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



[GitHub] [phoenix] virajjasani commented on pull request #1096: PHOENIX-5296 : refCount leak checks

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


   @stoty could you please take a look while we have 2nd QA results coming up?
   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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-766756455


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m  2s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 36s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  15m  5s |  master passed  |
   | +0 |  hbaserecompile  |  28m  0s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 54s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 50s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 37s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 40s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 40s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   4m  9s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |  11m 52s |  the patch passed  |
   | +0 |  hbaserecompile  |  20m 19s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 58s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 58s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 53s |  root: The patch generated 23 new + 108 unchanged - 0 fixed = 131 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m 23s |  the patch passed  |
   | -1 :x: |  spotbugs  |   0m 59s |  phoenix-hbase-compat-2.4.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | -1 :x: |  spotbugs  |   1m  1s |  phoenix-hbase-compat-2.3.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 59s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   1m  1s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   4m 30s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 143m 13s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  0s |  The patch does not generate ASF License warnings.  |
   |  |   | 224m  9s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-hbase-compat-2.4.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 109] |
   | FindBugs | module:phoenix-hbase-compat-2.3.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 108] |
   
   
   | 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-1096/25/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 395b0b1822d8 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / ba47233 |
   | 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-1096/25/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.4.0.html |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.3.0.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/testReport/ |
   | Max. process+thread count | 7781 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       For 4.x `Uninterruptibles` is in beta version based on guava version that we use so i used this: https://github.com/apache/phoenix/pull/1097/files#diff-d12329d4796498b9880a54bb4b6d681f7ee42b924536bf5247791f372329cd9fR61
   However, i feel using `Uninterruptibles.sleepUninterruptibly` more comfortably as we don't have to catch `InterruptedException` and it is not interruptible. But i agree with your point on not introducing more dependency so let me replicate same logic as 4.x.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-766756455


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m  2s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 36s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  15m  5s |  master passed  |
   | +0 |  hbaserecompile  |  28m  0s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 54s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 50s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 37s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 40s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 40s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   4m  9s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |  11m 52s |  the patch passed  |
   | +0 |  hbaserecompile  |  20m 19s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 58s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 58s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 53s |  root: The patch generated 23 new + 108 unchanged - 0 fixed = 131 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m 23s |  the patch passed  |
   | -1 :x: |  spotbugs  |   0m 59s |  phoenix-hbase-compat-2.4.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | -1 :x: |  spotbugs  |   1m  1s |  phoenix-hbase-compat-2.3.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 59s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   1m  1s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   4m 30s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 143m 13s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  0s |  The patch does not generate ASF License warnings.  |
   |  |   | 224m  9s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-hbase-compat-2.4.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 109] |
   | FindBugs | module:phoenix-hbase-compat-2.3.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 108] |
   
   
   | 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-1096/25/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 395b0b1822d8 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / ba47233 |
   | 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-1096/25/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.4.0.html |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.3.0.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/testReport/ |
   | Max. process+thread count | 7781 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/25/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] stoty commented on a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559497588



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       I think that throwing an exception here may interfere with the cluster shutdown logic that this is called from.
   
   Either catch and log it (and fail the test), or make sure that it doesn't preclude cluster shutdown every invocation point.




----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559588560



##########
File path: phoenix-core/src/it/java/org/apache/hadoop/hbase/regionserver/wal/WALReplayWithIndexWritesAndCompressedWALIT.java
##########
@@ -155,6 +156,15 @@ protected void startCluster() throws Exception {
 
   @After
   public void tearDown() throws Exception {
+    try {
+      CompatUtil.confirmStoreRefCountLeak(UTIL.getAdmin());
+    } catch (IOException e) {
+      LOGGER.error("StoreFile refCount is leaked", e);
+      UTIL.shutdownMiniHBaseCluster();

Review comment:
       Should just put these statements into a `finally`  instead.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/DerivedTableIT.java
##########
@@ -101,8 +100,9 @@ public void initTable() throws Exception {
     }
 
     @After
-    public void cleanUp(){
-        tableName=null;
+    public void cleanUp() throws Exception {
+        confirmStoreRefCountLeak();
+        tableName = null;

Review comment:
       switch the statement order ?

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
##########
@@ -110,6 +117,18 @@ public synchronized void doSetup() throws Exception {
     
     @After
     public void cleanUpAfterTest() throws Exception {
+        try {
+            CompatUtil.confirmStoreRefCountLeak(
+                hbaseTestUtil.getAdmin());
+        } catch (IOException e) {
+            LOGGER.error("StoreFile refCount is leaked", e);
+            try {

Review comment:
       this can also go into finally.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolTimeRangeIT.java
##########
@@ -226,7 +225,8 @@ public void tickTime() {
     }
 
     @AfterClass
-    public static synchronized void teardown() {
+    public static synchronized void teardown() throws Exception {
+        confirmStoreRefCountLeak();

Review comment:
       this should also be
   ```
   try {
     confirmStore...
   } catch (Exception e) {
     Assert.fail()}
   } finally {
     tearDownMiniCluster...
   }
   ```

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -73,7 +74,8 @@ public void setup() throws Exception {
     }
 
     @After
-    public void cleanUp() throws SQLException {
+    public void cleanUp() throws Exception {
+        confirmStoreRefCountLeak();
         deleteTenantData(descViewName);

Review comment:
       The should probably go into a finally as well.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

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






----------------------------------------------------------------
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 #1096: PHOENIX-5296 : Ensure store file reader refcount is zero at end of relevant unit tests

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-768124093


   Committed with update commit message.


----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-765682926


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 42s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m  4s |  master passed  |
   | +0 |  hbaserecompile  |  22m 31s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 29s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 42s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 37s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 58s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m  9s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 51s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 31s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 44s |  root: The patch generated 21 new + 108 unchanged - 0 fixed = 129 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 14s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 126m 46s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m 14s |  The patch does not generate ASF License warnings.  |
   |  |   | 189m 38s |   |
   
   
   | 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-1096/23/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 81edc6f2ae73 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / d5e93b3 |
   | 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-1096/23/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/23/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/23/testReport/ |
   | Max. process+thread count | 9821 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/23/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-765262831


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m  0s |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m  3s |  https://github.com/apache/phoenix/pull/1096 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/18/console |
   | versions | git=2.17.1 |
   | 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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-763929219






----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762256959


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 21s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 47s |  master passed  |
   | +0 |  hbaserecompile  |  22m 41s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 39s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  8s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 32s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m  8s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m  0s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 15s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 30s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 40s |  root: The patch generated 15 new + 34 unchanged - 2 fixed = 49 total (was 36)  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch 31 line(s) with tabs.  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   6m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 118m 16s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  7s |  The patch does not generate ASF License warnings.  |
   |  |   | 183m 47s |   |
   
   
   | 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-1096/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
   | uname | Linux 6254e78c0b14 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | 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-1096/2/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | whitespace | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/2/artifact/yetus-general-check/output/whitespace-tabs.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/2/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/2/testReport/ |
   | Max. process+thread count | 13658 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/2/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] stoty closed pull request #1096: PHOENIX-5296 : Ensure store file reader refcount is zero at end of relevant unit tests

Posted by GitBox <gi...@apache.org>.
stoty closed pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096


   


----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-767428915


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 14s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 53s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m 17s |  master passed  |
   | +0 |  hbaserecompile  |  22m 22s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 32s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 53s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m  9s |  the patch passed  |
   | +0 |  hbaserecompile  |  14m 30s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 33s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 42s |  root: The patch generated 23 new + 108 unchanged - 0 fixed = 131 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  the patch passed  |
   | -1 :x: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.4.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | -1 :x: |  spotbugs  |   0m 49s |  phoenix-hbase-compat-2.3.0 generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 13s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 118m  3s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m 10s |  The patch does not generate ASF License warnings.  |
   |  |   | 180m 30s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-hbase-compat-2.4.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 109] |
   | FindBugs | module:phoenix-hbase-compat-2.3.0 |
   |  |  org.apache.phoenix.compat.hbase.CompatUtil.isAnyStoreRefCountLeaked(Admin) calls Thread.sleep() with a lock held  At CompatUtil.java:lock held  At CompatUtil.java:[line 108] |
   
   
   | 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-1096/26/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 089759f1ce69 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / ba47233 |
   | 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-1096/26/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/26/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.4.0.html |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/26/artifact/yetus-general-check/output/new-spotbugs-phoenix-hbase-compat-2.3.0.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/26/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/26/testReport/ |
   | Max. process+thread count | 10892 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/26/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-763720186


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 45s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 29s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  13m 53s |  master passed  |
   | +0 |  hbaserecompile  |  26m 41s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   2m 10s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 53s |  master passed  |
   | +1 :green_heart: |  javadoc  |   3m 18s |  master passed  |
   | +0 :ok: |  spotbugs  |   1m  1s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 57s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 56s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 56s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   4m 56s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |  12m 51s |  the patch passed  |
   | +0 |  hbaserecompile  |  23m  0s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   2m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 16s |  the patch passed  |
   | -1 :x: |  checkstyle  |   1m  4s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  4s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   3m 16s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   1m 14s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   1m 16s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   1m 14s |  phoenix-hbase-compat-2.2.1 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   1m  8s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   5m 10s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 142m 19s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   2m 35s |  The patch does not generate ASF License warnings.  |
   |  |   | 233m 15s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.PermissionsCacheIT |
   |   | phoenix.end2end.PermissionNSEnabledIT |
   
   
   | 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-1096/13/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 49dbe3e8a828 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 772ff2c |
   | 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-1096/13/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/13/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/13/testReport/ |
   | Max. process+thread count | 6267 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/13/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 #1096: PHOENIX-5296 : refCount leak checks

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


   @stoty last 2 builds have `phoenix-core` built successfully (as both runs took place after PHOENIX-6329 was checked-in).


----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r563503258



##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       Could you solve this without Guava ?
   I know that it doesn't really matter, but it pains me to see anther Guava dependency added.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-765257053


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 31s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 40s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  13m 13s |  master passed  |
   | +0 |  hbaserecompile  |  25m  8s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 37s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 45s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  8s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 32s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m 16s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |  10m 33s |  the patch passed  |
   | +0 |  hbaserecompile  |  17m 40s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 41s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 41s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 40s |  root: The patch generated 17 new + 108 unchanged - 0 fixed = 125 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   1m 59s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 43s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 28s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 130m 31s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m 55s |  The patch does not generate ASF License warnings.  |
   |  |   | 206m 17s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.IndexScrutinyToolIT |
   
   
   | 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-1096/17/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux b61b7e5f96ef 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / e19cc3f |
   | 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-1096/17/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/17/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/17/testReport/ |
   | Max. process+thread count | 5808 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/17/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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       Done




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

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



[GitHub] [phoenix] virajjasani commented on a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       For 4.x `Uninterruptibles` is in beta version based on guava version that we use so i used this: https://github.com/apache/phoenix/pull/1097/files#diff-d12329d4796498b9880a54bb4b6d681f7ee42b924536bf5247791f372329cd9fR61
   However, i feel using `Uninterruptibles.sleepUninterruptibly` more comfortably as we don't have to catch `InterruptedException` and it is not interruptible. But i agree with your point on not introducing more dependency so let me replicate same logic as 4.x.

##########
File path: phoenix-hbase-compat-2.3.0/pom.xml
##########
@@ -97,6 +97,18 @@
       <version>${hbase23.compat.version}</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4j.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>

Review comment:
       Done




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

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



[GitHub] [phoenix] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-765374870


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 39s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 36s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  16m  9s |  master passed  |
   | +0 |  hbaserecompile  |  29m 20s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   2m 12s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 58s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 53s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 43s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 50s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   4m 38s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |  12m 55s |  the patch passed  |
   | +0 |  hbaserecompile  |  20m 41s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 38s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 41s |  root: The patch generated 17 new + 108 unchanged - 0 fixed = 125 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  0s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 41s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 43s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 33s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 109m  5s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   2m 23s |  The patch does not generate ASF License warnings.  |
   |  |   | 194m  9s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.index.IndexMetadataIT |
   
   
   | 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-1096/19/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 156716bd3d6b 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 11b37e6 |
   | 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-1096/19/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/19/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/19/testReport/ |
   | Max. process+thread count | 10036 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/19/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 #1096: PHOENIX-5296 : refCount leak checks

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


   > given the amount of trouble miniCluster shutdown/startup has already caused for it, I'd like to make sure that we do not skip it anywhere.
   
   Definitely good point. Let me take a pass through all tests.
   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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       I think better catch and log it and also throw AssertionError with explanation.




----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559638890



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2015,21 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     */
+    protected synchronized static void confirmStoreRefCountLeak()
+            throws Exception {
+        if (getUtility() != null) {
+            try {
+                CompatUtil.confirmStoreRefCountLeak(
+                    getUtility().getAdmin());
+            } catch (IOException e) {
+                LOGGER.error("StoreFile refCount is leaked", e);

Review comment:
       Sounds good, @virajjasani 




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-764041076


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m 12s |  master passed  |
   | +0 |  hbaserecompile  |  22m 45s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 32s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 42s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 10s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 39s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 34s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 58s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m 11s |  the patch passed  |
   | +0 |  hbaserecompile  |  15m 26s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 30s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 41s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.2.1 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 19s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m 10s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  7s |  The patch does not generate ASF License warnings.  |
   |  |   | 180m 21s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | 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-1096/15/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 921734e0bb6f 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 772ff2c |
   | 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-1096/15/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/15/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/15/testReport/ |
   | Max. process+thread count | 5262 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/15/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 #1096: PHOENIX-5296 : refCount leak checks

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


   @stoty i have done small change in `CompatUtil` to ensure tests have enough time before we fail with refCount leak. Could you please take a look again?


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

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



[GitHub] [phoenix] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762154340


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 19s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 32s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  12m 43s |  master passed  |
   | +0 |  hbaserecompile  |  24m 13s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 36s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 39s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  0s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 32s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 31s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m 14s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   9m 40s |  the patch passed  |
   | +0 |  hbaserecompile  |  16m 36s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 37s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 39s |  root: The patch generated 4 new + 36 unchanged - 0 fixed = 40 total (was 36)  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch 31 line(s) with tabs.  |
   | +1 :green_heart: |  javadoc  |   1m 56s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   6m 25s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 133m 36s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   2m 15s |  The patch does not generate ASF License warnings.  |
   |  |   | 202m 39s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | phoenix.end2end.QueryWithOffsetIT |
   |   | phoenix.util.IndexScrutinyIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
   | uname | Linux 1cf29e17ff22 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | 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-1096/1/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | whitespace | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/1/artifact/yetus-general-check/output/whitespace-tabs.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/1/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/1/testReport/ |
   | Max. process+thread count | 10054 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/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 commented on a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -73,7 +74,8 @@ public void setup() throws Exception {
     }
 
     @After
-    public void cleanUp() throws SQLException {
+    public void cleanUp() throws Exception {
+        confirmStoreRefCountLeak();
         deleteTenantData(descViewName);

Review comment:
       This too should be taken care of by BaseTest#confirmStoreRefCountLeak() because it internally frees up resources and while doing so, drops all tables before shutting down cluster.




----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#discussion_r559504498



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       Throwing any exception would also stop the minicluster shutdown process, unless you catch it at the caller.
   
   We should make sure that shutdownMiniCluster() will get called even if confirmStoreRefCountLeak() fails.
   
   ```
   @AfterClass
   public static void shutdown() {
     confirmStoreRefCountLeak();
     shutdownMiniCluster();
   }
   ```
   




----------------------------------------------------------------
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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       Updated PR should work. It does free up resources including shutting down mini or distributed cluster as per `freeResources()`.




----------------------------------------------------------------
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 #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-762489335


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 20s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m  2s |  master passed  |
   | +0 |  hbaserecompile  |  22m  6s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 31s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 43s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  7s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 38s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 39s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 37s |  phoenix-hbase-compat-2.2.1 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 37s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   3m 35s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   9m 35s |  the patch passed  |
   | +0 |  hbaserecompile  |  16m 45s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 39s |  root: The patch generated 17 new + 124 unchanged - 2 fixed = 141 total (was 126)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  3s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.2.1 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 12s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m  4s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   3m  7s |  The patch does not generate ASF License warnings.  |
   |  |   | 181m 22s |   |
   
   
   | 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-1096/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux 18deb9cb44c9 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 50df995 |
   | 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-1096/7/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/7/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/7/testReport/ |
   | Max. process+thread count | 13926 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.1 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/7/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] stoty commented on pull request #1096: PHOENIX-5296 : refCount leak checks

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1096:
URL: https://github.com/apache/phoenix/pull/1096#issuecomment-766329849


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  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 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   4m 46s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  11m 18s |  master passed  |
   | +0 |  hbaserecompile  |  22m 33s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  9s |  master passed  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.4.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 36s |  phoenix-hbase-compat-2.3.0 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 33s |  phoenix-hbase-compat-2.2.5 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   0m 35s |  phoenix-hbase-compat-2.1.6 in master has 1 extant spotbugs warnings.  |
   | +0 :ok: |  spotbugs  |   2m 58s |  phoenix-core in master has 955 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   8m 41s |  the patch passed  |
   | +0 |  hbaserecompile  |  16m  5s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m 43s |  root: The patch generated 21 new + 108 unchanged - 0 fixed = 129 total (was 108)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   0m 48s |  phoenix-hbase-compat-2.4.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 48s |  phoenix-hbase-compat-2.3.0 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  phoenix-hbase-compat-2.2.5 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  phoenix-hbase-compat-2.1.6 in the patch passed.  |
   | +1 :green_heart: |  spotbugs  |   3m 30s |  phoenix-core generated 0 new + 954 unchanged - 1 fixed = 954 total (was 955)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 232m 31s |  root in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m  6s |  The patch does not generate ASF License warnings.  |
   |  |   | 296m 28s |   |
   
   
   | 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-1096/24/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1096 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile xml |
   | uname | Linux fc421217da95 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 2c6bd1c |
   | 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-1096/24/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/24/artifact/yetus-general-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/24/testReport/ |
   | Max. process+thread count | 13599 (vs. ulimit of 30000) |
   | modules | C: phoenix-hbase-compat-2.4.0 phoenix-hbase-compat-2.3.0 phoenix-hbase-compat-2.2.5 phoenix-hbase-compat-2.1.6 phoenix-core U: . |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1096/24/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 a change in pull request #1096: PHOENIX-5296 : refCount leak checks

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



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
##########
@@ -2006,4 +2007,17 @@ public void run() {
         thread.setDaemon(true);
         thread.start();
     }
+
+    /**
+     * Confirms that no storeFile has refCount leakage
+     *
+     * @throws IOException if refCount is leaked
+     */
+    protected synchronized static void confirmStoreRefCountLeak()

Review comment:
       Updated PR should work




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