You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/11/30 19:41:21 UTC

[GitHub] [accumulo] ctubbsii opened a new pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

ctubbsii opened a new pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817


   * Obtain a SingletonReservation as a client for using ZooSession (via
     ZooReader) in ZooTraceClient (the tracer span receiver) and clean it
     up when the ZooTraceClient instance is closed or garbage collected
   * Remove a few warnings about unrecognized event states for ZooKeeper's
     new Closed event state, which is generated by the ZooKeeper client
     code when a connection is terminated
   * Remove a few unnecessary `log.isTraceEnabled()` checks in related code
   * Reduce visibility of `ZooSession.connect()` method to package-private


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

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

[GitHub] [accumulo] jmark99 edited a comment on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 edited a comment on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-736675131


   I've been running one particular IT test while reviewing this PR:
   
   ```shell
   mvn clean verify -Dtest=NoSuchTestExists -Dit.test=org.apache.accumulo.test.ShellIT#insertDeleteScanTest
   ```
   
   After each run I have been checking the failsafe directory for output. Although I've seen runs without the exception on occasion, I'm still seeing it in the output logs.
   
   Another interesting thing of note is that when starting and stopping accumulo, I'm seeing the following exception show up with the change:
   
   ```java
   2020-12-01T11:40:18,104 [zookeeper.ClientCnxn] ERROR: Error while calling watcher 
   java.lang.IllegalStateException: null
           at com.google.common.base.Preconditions.checkState(Preconditions.java:492) ~[guava-28.2-jre.jar:?]
           at org.apache.accumulo.fate.zookeeper.ZooCache.clear(ZooCache.java:484) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.accumulo.fate.zookeeper.ZooCache$ZCacheWatcher.process(ZooCache.java:174) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535) ~[zookeeper-3.5.8.jar:3.5.8]
           at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) ~[zookeeper-3.5.8.jar:3.5.8]
   ```
   
   I've been re-running the IT test, but I'm still seeing the exception appear the majority of the runs.
   
   Now I'm trying to verify that I'm testing everything correctly :) I added a debug statement to the ShellIT test just to verify that I was running the correct build.


----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738606098


   Okay @jmark99, I think I've tracked it all down now. :smiley_cat: 


----------------------------------------------------------------
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] [accumulo] jmark99 commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-736822279


   That particular IT also passes, it's just that the ZooSessionShutdown exception is still occurring when I run the IT (most of the time).


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

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

[GitHub] [accumulo] jmark99 commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738844434


   @ctubbsii, I successfully verified by running the ShellIT tests multiple times without seeing the exception appear. I also started and stopped the Accumulo process multiple times and did not see the precondition error either. LGTM!


----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-736802019


   > I've been running one particular IT test while reviewing this PR:
   > 
   > `mvn clean verify -Dtest=NoSuchTestExists -Dit.test=org.apache.accumulo.test.ShellIT#insertDeleteScanTest`
   
   All the ITs passed when I ran them, but I didn't pay attention to this log message. I will investigate.


----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738266412


   Okay, so when I "fixed" it, I verified the fix by running a real shell, and did not check the log messages in the ITs. After running the above ShellIT command, I can the same error message that @jmark99 was seeing. From a previous run of the `main` branch, I see the error in the following tests, so I will use those to try to verify a subsequent fix: `ShellIT,ShellServerIT,ShellConfigIT,TracerRecoversAfterOfflineTableIT,ConditionalWriterIT`
   
   As for the ZooCache precondition... I didn't see that. However, I'd be curious to know what you are doing to reproduce that. It shouldn't be possible based on the `IllegalStateException.toString()` implementation in Java: if the detail message is null, it should just print the class name, without the `null` part. I don't see how it could even print what you saw. So, I would want a reliable way to reproduce it to test it out.


----------------------------------------------------------------
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] [accumulo] jmark99 commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-736675131


   I've been running one particular IT test while reviewing this PR:
   
   <code>mvn clean verify -Dtest=NoSuchTestExists -Dit.test=org.apache.accumulo.test.ShellIT#insertDeleteScanTest</code>
   
   After each run I have been checking the failsafe directory for output. Although I've seen runs without the exception on occasion, I'm still seeing it in the output logs.
   
   Another interesting thing of note is that when starting and stopping accumulo, I'm seeing the following exception show up with the change:
   
   <code>
   2020-12-01T11:40:18,104 [zookeeper.ClientCnxn] ERROR: Error while calling watcher 
   java.lang.IllegalStateException: null
           at com.google.common.base.Preconditions.checkState(Preconditions.java:492) ~[guava-28.2-jre.jar:?]
           at org.apache.accumulo.fate.zookeeper.ZooCache.clear(ZooCache.java:484) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.accumulo.fate.zookeeper.ZooCache$ZCacheWatcher.process(ZooCache.java:174) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535) ~[zookeeper-3.5.8.jar:3.5.8]
           at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) ~[zookeeper-3.5.8.jar:3.5.8]
   </code>
   
   I've been re-running the IT test, but I'm still seeing the exception appear the majority of the runs.
   
   Now I'm trying to verify that I'm testing everything correctly :) I added a debug statement to the ShellIT test just to verify that I was running the correct build.


----------------------------------------------------------------
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] [accumulo] ctubbsii merged pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817


   


----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738342570


   For the ITs, it looks like ZooTraceClient is seeing Closed and/or Disconnected events when the tests finish and shut down, and assuming they are node changed events. The fix I made initially only addressed the message I was seeing in the shell, but did not address the messages in the tests.


----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738336328


   @jmark99 Thanks, I will try to reproduce and experiment a bit.


----------------------------------------------------------------
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] [accumulo] jmark99 edited a comment on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 edited a comment on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-736675131


   I've been running one particular IT test while reviewing this PR:
   
   ```shell
   mvn clean verify -Dtest=NoSuchTestExists -Dit.test='org.apache.accumulo.test.ShellIT#insertDeleteScanTest'
   ```
   
   After each run I have been checking the failsafe directory for output. Although I've seen runs without the exception on occasion, I'm still seeing it in the output logs.
   
   Another interesting thing of note is that when starting and stopping accumulo, I'm seeing the following exception show up with the change:
   
   ```java
   2020-12-01T11:40:18,104 [zookeeper.ClientCnxn] ERROR: Error while calling watcher 
   java.lang.IllegalStateException: null
           at com.google.common.base.Preconditions.checkState(Preconditions.java:492) ~[guava-28.2-jre.jar:?]
           at org.apache.accumulo.fate.zookeeper.ZooCache.clear(ZooCache.java:484) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.accumulo.fate.zookeeper.ZooCache$ZCacheWatcher.process(ZooCache.java:174) ~[accumulo-core-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
           at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:535) ~[zookeeper-3.5.8.jar:3.5.8]
           at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:510) ~[zookeeper-3.5.8.jar:3.5.8]
   ```
   
   I've been re-running the IT test, but I'm still seeing the exception appear the majority of the runs.
   
   Now I'm trying to verify that I'm testing everything correctly :) I added a debug statement to the ShellIT test just to verify that I was running the correct build.


----------------------------------------------------------------
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] [accumulo] jmark99 commented on pull request #1817: Fix #1800 Remove ZooTraceClient singleton warning

Posted by GitBox <gi...@apache.org>.
jmark99 commented on pull request #1817:
URL: https://github.com/apache/accumulo/pull/1817#issuecomment-738280361


   @ctubbsii I see the ZooCache precondition error after the first time I stop a running instance of Accumulo via Fluo-Uno. I was using Hadoop 3.3.0, ZK 3.5.8, and Accumulo 2.1.0-SNAPSHOT. I had the following workflow:
   <pre>
   uno fetch accumulo
   uno setup accumulo 
   ... do stuff  with accumulo...
   </pre>
   At this point, once I execute <code>uno stop accumulo</code> it appears.
   It continues to occur anytime I execute <code>uno start accumulo</code> or <code>uno stop accumulo</code> after this point.
   Note that it is not necessary to 'do stuff'. Simply stopping that initial instance of Accumulo is enough for me to start seeing the error.
   


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