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/02/25 01:24:52 UTC
[GitHub] [accumulo] ctubbsii opened a new pull request #1531: WIP - bump to
ZK 3.5.7
ctubbsii opened a new pull request #1531: WIP - bump to ZK 3.5.7
URL: https://github.com/apache/accumulo/pull/1531
This work is stalled due to https://issues.apache.org/jira/browse/ZOOKEEPER-3737
It is possible to do this, but one of the benefits of this was to try to
eliminate the transitive dependency on log4j 1.2. ZOOKEEPER-3737 seems
to prohibit that, at least for now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1531: WIP - bump to ZK 3.5.7
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1531: WIP - bump to ZK 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#issuecomment-591225446
A workaround was suggested, and works for the log4j 1.2 jmx ClassNotFoundException. So, this PR is now ready for review (though I'm still waiting for tests to finish). I think there might be some issue with a new port being listened to on 8080 by ZK servers... I haven't looked into it, but it seems likely to be something we'd want to set the port explicitly to some other random port instead of using 8080.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii merged pull request #1531: Bump ZooKeeper
dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1531: Bump ZooKeeper dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#discussion_r384757398
##########
File path: test/src/test/java/org/apache/accumulo/test/fate/zookeeper/ZooLockTest.java
##########
@@ -274,42 +274,43 @@ public void testUnexpectedEvent() throws Exception {
String parent = "/zltest-" + this.hashCode() + "-l" + pdCount.incrementAndGet();
ConnectedWatcher watcher = new ConnectedWatcher();
- ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher);
- zk.addAuthInfo("digest", "accumulo:secret".getBytes(UTF_8));
+ try (ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher)) {
Review comment:
Would be good to make more use of this being autoclosable but that could be a follow on ticket.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1531: Bump ZooKeeper
dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#issuecomment-591236647
It turns out the thing listening on port 8080 was the new admin server (a REST-based alternative to the 4lw interface), and it's both optional and trivial to disable.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1531:
Bump ZooKeeper dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#discussion_r384778200
##########
File path: test/src/test/java/org/apache/accumulo/test/fate/zookeeper/ZooLockTest.java
##########
@@ -274,42 +274,43 @@ public void testUnexpectedEvent() throws Exception {
String parent = "/zltest-" + this.hashCode() + "-l" + pdCount.incrementAndGet();
ConnectedWatcher watcher = new ConnectedWatcher();
- ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher);
- zk.addAuthInfo("digest", "accumulo:secret".getBytes(UTF_8));
+ try (ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher)) {
Review comment:
I think these might be the only places we could use try-with-resources (I only fixed these because they generated warnings). But yes, separate issue, if there's anything else to be done with this.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1531: WIP - bump to ZK 3.5.7
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1531: WIP - bump to ZK 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#issuecomment-590959324
> @ctubbsii do you know if Zookeepers hard coded dependency on log4j 1 in its server and/or client code? Wondering this impact Accumulo clients or if it only impact mini accumulo.
As far as I can tell, it's only in the server code, and only affects mini. Either way, bumping doesn't remove our dependency on log4j1 on the class path, so there's no good reason to bump to it right now (unless there's some 3.5-only feature we want). Hence, the stall.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1531: Bump ZooKeeper dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#discussion_r384496579
##########
File path: minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
##########
@@ -404,6 +404,7 @@ public MiniAccumuloClusterImpl(MiniAccumuloConfigImpl config) throws IOException
zooCfg.setProperty("maxClientCnxns", "1000");
zooCfg.setProperty("dataDir", config.getZooKeeperDir().getAbsolutePath());
zooCfg.setProperty("4lw.commands.whitelist", "ruok");
+ zooCfg.setProperty("admin.enableServer", "false");
Review comment:
Does disabling this avoid executing the code that calls log4j v1?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1531: Bump ZooKeeper dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#discussion_r384780972
##########
File path: test/src/test/java/org/apache/accumulo/test/fate/zookeeper/ZooLockTest.java
##########
@@ -274,42 +274,43 @@ public void testUnexpectedEvent() throws Exception {
String parent = "/zltest-" + this.hashCode() + "-l" + pdCount.incrementAndGet();
ConnectedWatcher watcher = new ConnectedWatcher();
- ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher);
- zk.addAuthInfo("digest", "accumulo:secret".getBytes(UTF_8));
+ try (ZooKeeper zk = new ZooKeeper(accumulo.getZooKeepers(), 30000, watcher)) {
Review comment:
I looked again and it looks like this is the only use of this object other than ZooSession (where we are manually closing) so disregard.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1531: Bump ZooKeeper dependency to 3.5.7
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1531: Bump ZooKeeper dependency to 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#discussion_r384498103
##########
File path: minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java
##########
@@ -404,6 +404,7 @@ public MiniAccumuloClusterImpl(MiniAccumuloConfigImpl config) throws IOException
zooCfg.setProperty("maxClientCnxns", "1000");
zooCfg.setProperty("dataDir", config.getZooKeeperDir().getAbsolutePath());
zooCfg.setProperty("4lw.commands.whitelist", "ruok");
+ zooCfg.setProperty("admin.enableServer", "false");
Review comment:
Nevermind I see the discussion on the zookeeper issue.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on issue #1531: WIP - bump to ZK
3.5.7
Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #1531: WIP - bump to ZK 3.5.7
URL: https://github.com/apache/accumulo/pull/1531#issuecomment-590892877
@ctubbsii do you know if Zookeepers hard coded dependency on log4j 1 in its server and/or client code? Wondering this impact Accumulo clients or if it only impact mini accumulo.
----------------------------------------------------------------
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
With regards,
Apache Git Services