ZooKeeper_branch34_jdk8 - Build # 2167 - Still Failing

2020-05-19 Thread Apache Jenkins Server 
See https://builds.apache.org/job/ZooKeeper_branch34_jdk8/2167/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 43.44 KB...]
[junit] Running org.apache.zookeeper.test.RestoreCommittedLogTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
19.777 sec
[junit] Running org.apache.zookeeper.test.SaslAuthDesignatedClientTest
[junit] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.949 sec
[junit] Running org.apache.zookeeper.test.SaslAuthDesignatedServerTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.389 sec
[junit] Running org.apache.zookeeper.test.SaslAuthFailDesignatedClientTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.345 sec
[junit] Running org.apache.zookeeper.test.SaslAuthFailNotifyTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.809 sec
[junit] Running org.apache.zookeeper.test.SaslAuthFailTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.9 
sec
[junit] Running org.apache.zookeeper.test.SaslAuthMissingClientConfigTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.844 sec
[junit] Running org.apache.zookeeper.test.SaslClientTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.123 sec
[junit] Running org.apache.zookeeper.test.SessionInvalidationTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.807 sec
[junit] Running org.apache.zookeeper.test.SessionTest
[junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
11.285 sec
[junit] Running org.apache.zookeeper.test.SessionTimeoutTest
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.067 sec
[junit] Running org.apache.zookeeper.test.StandaloneTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.926 sec
[junit] Running org.apache.zookeeper.test.StatTest
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.048 sec
[junit] Running org.apache.zookeeper.test.StaticHostProviderTest
[junit] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.313 sec
[junit] Running org.apache.zookeeper.test.SyncCallTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.805 sec
[junit] Running org.apache.zookeeper.test.TruncateTest
[junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
9.628 sec
[junit] Running org.apache.zookeeper.test.UpgradeTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.142 sec
[junit] Running org.apache.zookeeper.test.WatchedEventTest
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.134 sec
[junit] Running org.apache.zookeeper.test.WatcherFuncTest
[junit] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.754 sec
[junit] Running org.apache.zookeeper.test.WatcherTest
[junit] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
29.37 sec
[junit] Running org.apache.zookeeper.test.ZkDatabaseCorruptionTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
11.758 sec
[junit] Running org.apache.zookeeper.test.ZooKeeperQuotaTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.861 sec
[junit] Running org.apache.jute.BinaryInputArchiveTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.116 sec

fail.build.on.test.failure:

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/ZooKeeper_branch34_jdk8/build.xml:1425: 
The following error occurred while executing this line:
/home/jenkins/jenkins-slave/workspace/ZooKeeper_branch34_jdk8/build.xml:1428: 
Tests failed!

Total time: 43 minutes 31 seconds
Build step 'Invoke Ant' marked build as failure
Archiving artifacts
Recording test results
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any



###
## FAILED TESTS (if any) 
##
3 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.auth.QuorumKerberosAuthTest.testValidCredentials

Error Message:
waiting for server 0 being up

Stack Trace:
junit.framework.AssertionFailedError: waiting for server 0 being up
at 
org.apache.zookeeper.server.quorum.auth.QuorumAuthTestBase.startQuorum(QuorumAuthTestBase.java:75)
at 
org.apache.zookeeper.server.quorum.auth.QuorumKerberosAuthTest.testValidCredentials(QuorumKerberosAuthTest.java:114)
at

[GitHub] [zookeeper] belugabehr commented on a change in pull request #1361: ZOOKEEPER-3840: Use JDK 8 Facilities to Synchronize Access to DataTre…

2020-05-19 Thread GitBox 


belugabehr commented on a change in pull request #1361:
URL: https://github.com/apache/zookeeper/pull/1361#discussion_r427454605



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
##
@@ -1407,12 +1391,8 @@ public void deserialize(InputArchive ia, String tag) 
throws IOException {
 } else if (ephemeralType == EphemeralType.TTL) {
 ttls.add(path);
 } else if (eowner != 0) {
-HashSet list = ephemerals.get(eowner);
-if (list == null) {
-list = new HashSet();
-ephemerals.put(eowner, list);
-}
-list.add(path);

Review comment:
   This is `synchronized` in `deleteNode` but not here.  Not sure if it 
matters.





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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr opened a new pull request #1361: ZOOKEEPER-3840: Use JDK 8 Facilities to Synchronize Access to DataTre…

2020-05-19 Thread GitBox 


belugabehr opened a new pull request #1361:
URL: https://github.com/apache/zookeeper/pull/1361


   …e Ephemerals



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:
us...@infra.apache.org

[GitHub] [zookeeper] anmolnar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


anmolnar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427381887



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   Sure. 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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr commented on pull request #1359: ZOOKEEPER-3837: Deprecate StringUtils Join

2020-05-19 Thread GitBox 


belugabehr commented on pull request #1359:
URL: https://github.com/apache/zookeeper/pull/1359#issuecomment-630883360


   > [WARNING] 
/home/travis/build/apache/zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java:[319,24]
 joinStrings(java.util.List,java.lang.String) in 
org.apache.zookeeper.common.StringUtils has been deprecated
   [INFO] 6 warnings 
   [INFO] -
   [INFO] -
   [ERROR] COMPILATION ERROR : 
   [INFO] -
   [ERROR] 
/home/travis/build/apache/zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java:
 warnings found and -Werror specified
   [INFO] 1 error
   
   That's unfortunate that I can't deprecate things.



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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr commented on pull request #1357: ZOOKEEPER-3835: Deprecate IOUtils copyBytes

2020-05-19 Thread GitBox 


belugabehr commented on pull request #1357:
URL: https://github.com/apache/zookeeper/pull/1357#issuecomment-630882722


   > [WARNING] 
/home/travis/build/apache/zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java:[703,20]
 copyBytes(java.io.InputStream,java.io.OutputStream,int,boolean) in 
org.apache.zookeeper.common.IOUtils has been deprecated
   [INFO] 1 warning
   [INFO] -
   [INFO] -
   [ERROR] COMPILATION ERROR : 
   [INFO] -
   [ERROR] 
/home/travis/build/apache/zookeeper/zookeeper-server/src/test/java/org/apache/zookeeper/test/StringUtilTest.java:
 warnings found and -Werror specified
   
   That's unfortunate that I can't deprecate things.



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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr opened a new pull request #1360: ZOOKEEPER-3839: ReconfigBackupTest Remove getFileContent

2020-05-19 Thread GitBox 


belugabehr opened a new pull request #1360:
URL: https://github.com/apache/zookeeper/pull/1360


   



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:
us...@infra.apache.org

Jenkins build became unstable: zookeeper-branch36-java8 #184

2020-05-19 Thread Apache Jenkins Server 
See

[GitHub] [zookeeper] belugabehr opened a new pull request #1359: ZOOKEEPER-3837: Deprecate StringUtils Join

2020-05-19 Thread GitBox 


belugabehr opened a new pull request #1359:
URL: https://github.com/apache/zookeeper/pull/1359


   



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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr opened a new pull request #1358: ZOOKEEPER-3836: Use Commons and JDK Functions in ClientBase

2020-05-19 Thread GitBox 


belugabehr opened a new pull request #1358:
URL: https://github.com/apache/zookeeper/pull/1358


   



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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr opened a new pull request #1357: ZOOKEEPER-3835: Deprecate IOUtils copyBytes

2020-05-19 Thread GitBox 


belugabehr opened a new pull request #1357:
URL: https://github.com/apache/zookeeper/pull/1357


   



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:
us...@infra.apache.org

[GitHub] [zookeeper] belugabehr commented on pull request #1354: ZOOKEEPER-3833: Do Not Override Plugin Versions from Apache Parent POM

2020-05-19 Thread GitBox 


belugabehr commented on pull request #1354:
URL: https://github.com/apache/zookeeper/pull/1354#issuecomment-630806235


   @ctubbsii Great catch.  Thanks.  Change reverted.



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:
us...@infra.apache.org

Jenkins build is back to normal : PreCommit-ZOOKEEPER-github-pr-build-maven #2053

2020-05-19 Thread Apache Jenkins Server 
See

Build failed in Jenkins: zookeeper-master-maven-jdk14 #99

2020-05-19 Thread Apache Jenkins Server 
See 


Changes:


--
[...truncated 198.47 KB...]
 [exec] libtool: link: mv -f ".libs/libzookeeper_mt.expT" 
".libs/libzookeeper_mt.exp"
 [exec] libtool: link: echo "{ global:" > .libs/libzookeeper_mt.ver
 [exec] libtool: link:  cat .libs/libzookeeper_mt.exp | sed -e 
"s/\(.*\)/\1;/" >> .libs/libzookeeper_mt.ver
 [exec] libtool: link:  echo "local: *; };" >> .libs/libzookeeper_mt.ver
 [exec] libtool: link:  gcc -shared  -fPIC -DPIC  -Wl,--whole-archive 
./.libs/libzkmt.a ./.libs/libhashtable.a -Wl,--no-whole-archive  -lssl -lcrypto 
-lsasl2 -lm -lrt -lpthread  -g -O2   -Wl,-soname -Wl,libzookeeper_mt.so.2 
-Wl,-version-script -Wl,.libs/libzookeeper_mt.ver -o 
.libs/libzookeeper_mt.so.2.0.0
 [exec] libtool: link: (cd ".libs" && rm -f "libzookeeper_mt.so.2" && ln -s 
"libzookeeper_mt.so.2.0.0" "libzookeeper_mt.so.2")
 [exec] libtool: link: (cd ".libs" && rm -f "libzookeeper_mt.so" && ln -s 
"libzookeeper_mt.so.2.0.0" "libzookeeper_mt.so")
 [exec] libtool: link: (cd .libs/libzookeeper_mt.lax/libzkmt.a && ar x 
"
 [exec] ar: `u' modifier ignored since `D' is the default (see `U')
 [exec] libtool: link: (cd .libs/libzookeeper_mt.lax/libhashtable.a && ar x 
"
 [exec] libtool: link: ar cru .libs/libzookeeper_mt.a   
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-addrvec.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-mt_adaptor.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-recordio.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-zk_hashtable.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-zk_log.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-zk_sasl.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-zookeeper.jute.o 
.libs/libzookeeper_mt.lax/libzkmt.a/libzkmt_la-zookeeper.o  
.libs/libzookeeper_mt.lax/libhashtable.a/hashtable.o 
.libs/libzookeeper_mt.lax/libhashtable.a/hashtable_itr.o 
 [exec] libtool: link: ranlib .libs/libzookeeper_mt.a
 [exec] libtool: link: rm -fr .libs/libzookeeper_mt.lax
 [exec] libtool: link: ( cd ".libs" && rm -f "libzookeeper_mt.la" && ln -s 
"../libzookeeper_mt.la" "libzookeeper_mt.la" )
 [exec] gcc -DHAVE_CONFIG_H -I. 
-I
  
-I
 
-I
 
-I
  -DHAVE_OPENSSL_H -DHAVE_CYRUS_SASL_H  -Wall -Werror 
-Wdeclaration-after-statement  -g -O2 -D_GNU_SOURCE -MT cli.o -MD -MP -MF 
.deps/cli.Tpo -c -o cli.o `test -f 'src/cli.c' || echo 
'
 [exec] mv -f .deps/cli.Tpo .deps/cli.Po
 [exec] /bin/bash ./libtool  --tag=CC   --mode=link gcc -Wall -Werror 
-Wdeclaration-after-statement  -g -O2 -D_GNU_SOURCE   -o cli_st cli.o 
libzookeeper_st.la -lsasl2 
 [exec] libtool: link: gcc -Wall -Werror -Wdeclaration-after-statement -g 
-O2 -D_GNU_SOURCE -o .libs/cli_st cli.o  ./.libs/libzookeeper_st.so -lsasl2 
-Wl,-rpath 
-Wl,
 [exec] gcc -DHAVE_CONFIG_H -I. 
-I
  
-I
 
-I
 
-I
  -DHAVE_OPENSSL_H -DHAVE_CYRUS_SASL_H  -Wall -Werror 
-Wdeclaration-after-statement  -DTHREADED -g -O2 -D_GNU_SOURCE -MT cli_mt-cli.o 
-MD -MP -MF .deps/cli_mt-cli.Tpo -c -o cli_mt-cli.o `test -f 'src/cli.c' || 
echo 
'
 [exec] mv -f .deps/cli_mt-cli.Tpo .deps/cli_mt-cli.Po
 [exec] /bin/bash ./libtool  --tag=CC   --mode=link gcc -Wall -Werror 
-Wdeclaration-after-statement  -DTHREADED -g -O2 -D_GNU_SOURCE   -o cli_mt 
cli_mt-cli.o libzookeeper_mt.la -lsasl2 
 [exec] libtool: link: gcc -Wall -Werror -Wdeclaration-after-statement 
-DTHREADED -g -O2 -D_GNU_SOURCE -o .libs/cli_mt cli_mt-cli.o  
./.libs/libzookeeper_mt.so

[GitHub] [zookeeper] symat commented on pull request #1356: ZOOKEEPER-3829: backward compatibility fix for rolling restart without dynamic reconfig

2020-05-19 Thread GitBox 


symat commented on pull request #1356:
URL: https://github.com/apache/zookeeper/pull/1356#issuecomment-630747049


   @shralex , @hanm please take a look if you can!



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:
us...@infra.apache.org

[GitHub] [zookeeper] symat commented on a change in pull request #1356: ZOOKEEPER-3829: backward compatibility fix for rolling restart without dynamic reconfig

2020-05-19 Thread GitBox 


symat commented on a change in pull request #1356:
URL: https://github.com/apache/zookeeper/pull/1356#discussion_r427212264



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
##
@@ -1508,20 +1510,25 @@ private synchronized void startZkServer() {
  newLeaderProposal.ackSetsToString(),
  Long.toHexString(zk.getZxid()));
 
-/*
- * ZOOKEEPER-1324. the leader sends the new config it must complete
- *  to others inside a NEWLEADER message (see LearnerHandler where
- *  the NEWLEADER message is constructed), and once it has enough
- *  acks we must execute the following code so that it applies the
- *  config to itself.
- */
-QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
+if (QuorumPeerConfig.isReconfigEnabled()) {
+/*
+ * ZOOKEEPER-1324. the leader sends the new config it must complete
+ *  to others inside a NEWLEADER message (see LearnerHandler where
+ *  the NEWLEADER message is constructed), and once it has enough
+ *  acks we must execute the following code so that it applies the
+ *  config to itself.
+ */
+QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
 
-Long designatedLeader = getDesignatedLeader(newLeaderProposal, 
zk.getZxid());
+Long designatedLeader = getDesignatedLeader(newLeaderProposal, 
zk.getZxid());
 
-self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
-if (designatedLeader != self.getId()) {
-allowedToCommit = false;
+self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
+if (designatedLeader != self.getId()) {
+LOG.warn("This leader is not the designated leader, it will be 
initialized with allowedToCommit = false");
+allowedToCommit = false;
+}
+} else {
+LOG.debug("Reconfig feature is disabled, skip designatedLeader 
calculation and reconfig processing.");

Review comment:
   I agree with the LOG.info (this message will be printed only once when 
the leader starts to lead after an election. Yet, the message can help 
debugging production issues, when debug logs are disabled)
   
   The tests are simulating the "backward compatibility" case when 
`QuorumPeerConfig.isReconfigEnabled()==false`. I don't think that changing the 
config with rolling-restart would be advised when dynamic-reconfig is enabled. 
I think when the reconfig is enabled, the new config can propagate during the 
leader election, so the rolling-restart will be unneccessary.





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:
us...@infra.apache.org

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1356: ZOOKEEPER-3829: backward compatibility fix for rolling restart without dynamic reconfig

2020-05-19 Thread GitBox 


eolivelli commented on a change in pull request #1356:
URL: https://github.com/apache/zookeeper/pull/1356#discussion_r427192704



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
##
@@ -1508,20 +1510,25 @@ private synchronized void startZkServer() {
  newLeaderProposal.ackSetsToString(),
  Long.toHexString(zk.getZxid()));
 
-/*
- * ZOOKEEPER-1324. the leader sends the new config it must complete
- *  to others inside a NEWLEADER message (see LearnerHandler where
- *  the NEWLEADER message is constructed), and once it has enough
- *  acks we must execute the following code so that it applies the
- *  config to itself.
- */
-QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
+if (QuorumPeerConfig.isReconfigEnabled()) {
+/*
+ * ZOOKEEPER-1324. the leader sends the new config it must complete
+ *  to others inside a NEWLEADER message (see LearnerHandler where
+ *  the NEWLEADER message is constructed), and once it has enough
+ *  acks we must execute the following code so that it applies the
+ *  config to itself.
+ */
+QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
 
-Long designatedLeader = getDesignatedLeader(newLeaderProposal, 
zk.getZxid());
+Long designatedLeader = getDesignatedLeader(newLeaderProposal, 
zk.getZxid());
 
-self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
-if (designatedLeader != self.getId()) {
-allowedToCommit = false;
+self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
+if (designatedLeader != self.getId()) {
+LOG.warn("This leader is not the designated leader, it will be 
initialized with allowedToCommit = false");
+allowedToCommit = false;
+}
+} else {
+LOG.debug("Reconfig feature is disabled, skip designatedLeader 
calculation and reconfig processing.");

Review comment:
   LOG.debug -> what about putting it as LOG.info ?
   
   In your new test cases, are you exercising this branch or the "if 
QuorumPeerConfig.isReconfigEnabled()" branch ?





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:
us...@infra.apache.org

Build failed in Jenkins: PreCommit-ZOOKEEPER-github-pr-build-maven #2052

2020-05-19 Thread Apache Jenkins Server 
See 


Changes:


--
GitHub pull request #1,356 to apache/zookeeper
Started by user symat
Rebuilds build #2050
Running as SYSTEM
[EnvInject] - Loading node environment variables.
Building remotely on H1 (Hadoop) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
[WS-CLEANUP] Done
No credentials specified
Wiping out workspace first.
Cloning the remote Git repository
Cloning repository https://github.com/apache/zookeeper
 > git init 
 > 
 >  # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git --version # timeout=10
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse refs/remotes/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse origin/pr/2050/head^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
No credentials specified
Wiping out workspace first.
Cloning the remote Git repository
Cloning repository https://github.com/apache/zookeeper
 > git init 
 > 
 >  # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git --version # timeout=10
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse refs/remotes/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse origin/pr/2050/head^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
No credentials specified
Wiping out workspace first.
Cloning the remote Git repository
Cloning repository https://github.com/apache/zookeeper
 > git init 
 > 
 >  # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git --version # timeout=10
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url https://github.com/apache/zookeeper # timeout=10
Fetching upstream changes from https://github.com/apache/zookeeper
 > git fetch --tags --progress -- https://github.com/apache/zookeeper 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse refs/remotes/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/pr/2050/head^{commit} # timeout=10
 > git rev-parse origin/pr/2050/head^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
[SpotBugs] Skipping execution of recorder since overall result is 'FAILURE'

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


eolivelli commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427181922



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   I would just write something like
   "These test case have been taken from the Apache HttpComponents project."
   Nothing more.
   It is just a test resource (but we are going to "release it", in the source 
release)





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:
us...@infra.apache.org

Jenkins build is back to stable : PreCommit-ZOOKEEPER-github-pr-build-maven #2051

2020-05-19 Thread Apache Jenkins Server 
See

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


nkalmar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427161374



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java
##
@@ -324,8 +324,14 @@ private static HostNameType determineHostFormat(final 
String host) {
 for (List entry : entries) {
 final Integer type = entry.size() >= 2 ? (Integer) 
entry.get(0) : null;
 if (type != null) {
-final String s = (String) entry.get(1);
-result.add(new SubjectName(s, type));
+if (type == SubjectName.DNS || type == SubjectName.IP) {
+final Object o = entry.get(1);
+if (o instanceof String) {
+result.add(new SubjectName((String) o, type));
+} else if (o instanceof byte[]) {
+// TODO ASN.1 DER encoded form

Review comment:
   I agree with Mate here, probably adding a warning until this TODO is not 
implemented would be nice.





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:
us...@infra.apache.org

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


nkalmar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427158878



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   Just checked JVMPauseMonitor out of curiosity. I got that class from 
hadoop, but modified it. I do have a comment indicating the original source. 
Maybe adding something similar?
   
https://github.com/apache/zookeeper/blob/fe940cdd8fb23ba09684cefb73233d570f4a20fa/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/JvmPauseMonitor.java





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:
us...@infra.apache.org

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


nkalmar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427156834



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   AFAIK (and Enrico's question also applies this) if it's from another 
Apache project, the Apache license header is enough.





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:
us...@infra.apache.org

Jenkins build became unstable: PreCommit-ZOOKEEPER-github-pr-build-maven #2050

2020-05-19 Thread Apache Jenkins Server 
See

[GitHub] [zookeeper] anmolnar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


anmolnar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427121077



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java
##
@@ -324,8 +324,14 @@ private static HostNameType determineHostFormat(final 
String host) {
 for (List entry : entries) {
 final Integer type = entry.size() >= 2 ? (Integer) 
entry.get(0) : null;
 if (type != null) {
-final String s = (String) entry.get(1);
-result.add(new SubjectName(s, type));
+if (type == SubjectName.DNS || type == SubjectName.IP) {
+final Object o = entry.get(1);
+if (o instanceof String) {
+result.add(new SubjectName((String) o, type));
+} else if (o instanceof byte[]) {
+// TODO ASN.1 DER encoded form

Review comment:
   I believe it can be done with BouncyCastle ASN1 libraries, but this part 
was missing in the original patch too. I'd be happy to add it as a separate 
ticket, but first I need an example certificate with ASN1 encoded data.





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:
us...@infra.apache.org

[GitHub] [zookeeper] symat opened a new pull request #1356: ZOOKEEPER-3829: backward compatibility fix for rolling restart without dynamic reconfig

2020-05-19 Thread GitBox 


symat opened a new pull request #1356:
URL: https://github.com/apache/zookeeper/pull/1356


   In three different Jira tickets (ZOOKEEPER-3829, ZOOKEEPER-3830, 
ZOOKEEPER-3814) we saw backward-compatibility issues when rolling restart was 
used to change the quorum membership configuration without having dynamic 
reconfig enabled. In worst case it lead to the scenario that we had an elected 
leader which was up but unable to commit any changes.
   
   In this fix I try to fix these issues without breaking anything. I also add 
a few more unit tests to cover these scenarios.
   
   Note: this is still WIP. 



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:
us...@infra.apache.org

[GitHub] [zookeeper] anmolnar commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


anmolnar commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427114702



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   Tests and test resources (this file) have been copied from ASF project 
http client. Name of the project is Apache HttpComponents: 
https://hc.apache.org/
   What do I need to add exactly?





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:
us...@infra.apache.org

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


eolivelli commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427061522



##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java
##
@@ -0,0 +1,569 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Some X509 certificates to test against.
+ * 
+ * Note:  some of these certificates have Japanese Kanji in the "subjectAlt"
+ * field (UTF8).  Not sure how realistic that is since international characters
+ * in DNS names usually get translated into ASCII using "xn--" style DNS
+ * entries.  "xn--i8s592g.co.jp" is what FireFox actually uses when trying to
+ * find .co.jp.  So would the CN in the certificate contain
+ * "xn--i8s592g.co.jp" in ASCII, or ".co.jp" in UTF8?  (Both?)
+ * 
+ *
+ * @since 11-Dec-2006

Review comment:
   Do we have to cite the source of this code ?
   Or did you create this file by yourself ?
   
   If it is not from an ASF project we have to add some line in NOTICE file





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:
us...@infra.apache.org

[GitHub] [zookeeper] symat commented on a change in pull request #1353: ZOOKEEPER-3832 ZKHostnameVerifier rejects valid certificates with subjectAltNames

2020-05-19 Thread GitBox 


symat commented on a change in pull request #1353:
URL: https://github.com/apache/zookeeper/pull/1353#discussion_r427058447



##
File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKHostnameVerifier.java
##
@@ -324,8 +324,14 @@ private static HostNameType determineHostFormat(final 
String host) {
 for (List entry : entries) {
 final Integer type = entry.size() >= 2 ? (Integer) 
entry.get(0) : null;
 if (type != null) {
-final String s = (String) entry.get(1);
-result.add(new SubjectName(s, type));
+if (type == SubjectName.DNS || type == SubjectName.IP) {
+final Object o = entry.get(1);
+if (o instanceof String) {
+result.add(new SubjectName((String) o, type));
+} else if (o instanceof byte[]) {
+// TODO ASN.1 DER encoded form

Review comment:
   I am not sure what ASN.1 DER is or how commonly it is used, but I think 
at least printing out a warning here would make sense (informing the user that 
ASN.1 DER is not supported). (?)

##
File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKHostnameVerifierTest.java
##
@@ -0,0 +1,158 @@
+/**

Review comment:
   this is a great test to have, 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:
us...@infra.apache.org