[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230539608
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
--- End diff --

Thanks for defining states. 


---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230539129
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
+synchronized (watcherThread) {
+while (!watcherThread.started) {
+watcherThread.wait();
+}
+}
+}
+
+/**
+ * Tells the background thread to stop. Does not wait for it to exit.
+ */
+public void stop() {
+watcherThread.shouldStop = true;
+watcherThread.interrupt();
+}
+
+/**
+ * Tells the background thread to stop and waits for it to exit. Only 
used by unit tests, which is why it is package
+ 

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230538126
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,191 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+public enum State {
+NEW,  // object created but start() not called yet
+STARTING, // start() called but background thread has not entered 
main loop
+RUNNING,  // background thread is running
+STOPPING, // stop() called but background thread has not exited 
main loop
+STOPPED   // stop() called and background thread has exited, or 
background thread crashed
+}
+
+private final WatcherThread watcherThread;
+private State state; // protected by synchronized(this)
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+state = State.NEW;
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+}
+
+public synchronized State getState() {
+return state;
+}
+
+private synchronized void setState(State newState) {
+state = newState;
+this.notifyAll();
+}
+
+/**
+ * Tells the background thread to start. Does not wait for it to be 
running.
+ * Calling this method more than once has no effect.
+ */
+public synchronized void start() {
+if (state != State.NEW) {
+   

Failed: ZOOKEEPER- PreCommit Build #2575

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2575/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 76.07 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 20 minutes 40 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 93ce9c429353fd061c1cdd07aba89c93d227c20c to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2575/ and 
message: 'FAILURE
 2309 tests run, 1 skipped, 0 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2575/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
All tests passed

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2575/



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/681
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2576/



---

Failed: ZOOKEEPER- PreCommit Build #2576

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2576/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 85.27 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=257EEA6BEE15AB8DE7D07AF682E4683C.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
‘/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess’
 and 
‘/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess’
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 16 minutes 12 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3176
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 75dc064b8717647f0238ad3e92c5ee00b6ef9a35 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2576/ and 
message: 'FAILURE
 2357 tests run, 1 skipped, 1 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2576/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
1 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[0]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

Failed: ZOOKEEPER- PreCommit Build #2573

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2573/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 85.68 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=B906CFEE22095A6FC09749824E7732F3.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 16 minutes 37 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 2e0865f7378a051bc6879b560c48103213646248 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2573/ and 
message: 'FAILURE
 2309 tests run, 1 skipped, 1 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2573/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
1 tests failed.
FAILED:  
org.apache.zookeeper.common.FileChangeWatcherTest.testCallbackErrorDoesNotCrashWatcherThread

Error Message:
expected:<2> but was:<3>

Stack Trace:
junit.framework.AssertionFailedError: expected:<2> but was:<3>
at 
org.apache.zookeeper.common.FileChangeWatcherTest.testCallbackErrorDoesNotCrashWatcherThread(FileChangeWatcherTest.java:241)
at 
org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2573/



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/681
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2574/



---

Failed: ZOOKEEPER- PreCommit Build #2574

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2574/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 83.68 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=0ED6EC08232B76D4F0A1F0543EE03F5C.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 15 minutes 1 second
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3176
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 70051f9b91c72434f3edb6581e091bdefd8bb01b to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2574/ and 
message: 'FAILURE
 2357 tests run, 1 skipped, 1 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2574/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
1 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[6]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2571/



---

Failed: ZOOKEEPER- PreCommit Build #2571

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2571/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 80.44 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=EE5E601F1555D6962BEE0586AB15DD15.
 [exec] mv: 
‘/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess’
 and 
‘/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess’
 are the same file
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 20 minutes 35 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of cdc84f29fe1337c74dcffd323d3ae652530aa39e to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2571/ and 
message: 'FAILURE
 2309 tests run, 1 skipped, 2 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2571/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
2 tests failed.
FAILED:  
org.apache.zookeeper.common.FileChangeWatcherTest.testCallbackErrorDoesNotCrashWatcherThread

Error Message:
expected:<2> but was:<3>

Stack Trace:
junit.framework.AssertionFailedError: expected:<2> but was:<3>
at 
org.apache.zookeeper.common.FileChangeWatcherTest.testCallbackErrorDoesNotCrashWatcherThread(FileChangeWatcherTest.java:241)
at 
org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)


FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[0]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/681
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2572/



---

Failed: ZOOKEEPER- PreCommit Build #2572

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2572/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 85.91 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 16 minutes 5 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3176
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of befadaf9dad2675351a5c91f4362a5e802968d70 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2572/ and 
message: 'FAILURE
 2357 tests run, 1 skipped, 1 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2572/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
1 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[0]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230523204
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
+synchronized (watcherThread) {
+while (!watcherThread.started) {
+watcherThread.wait();
+}
+}
+}
+
+/**
+ * Tells the background thread to stop. Does not wait for it to exit.
+ */
+public void stop() {
+watcherThread.shouldStop = true;
+watcherThread.interrupt();
+}
+
+/**
+ * Tells the background thread to stop and waits for it to exit. Only 
used by unit tests, which is why it is package
+ 

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230523105
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
--- End diff --

I'll see what I can do about this.


---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230522968
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
--- End diff --

I think we might still need locking because there is a background thread 
that's polling the WatchService for events. And starting/stopping the watcher 
requires starting/stopping the background thread, so some synchronization 
between threads is needed. Do you still think it makes sense to define life 
cycle stages if we still need locking?


---

[GitHub] zookeeper issue #685: [ZOOKEEPER-3104] Fix potential data inconsistency due ...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/685
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2570/



---

Success: ZOOKEEPER- PreCommit Build #2570

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2570/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 82.28 MB...]
 [exec] 
==
 [exec] Adding comment to Jira.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD SUCCESSFUL
Total time: 22 minutes 21 seconds
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3104
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 5703e334a1a775b9f8342409924fc3247475aa81 to SUCCESS with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2570/ and 
message: 'SUCCESS 
 1725 tests run, 3 skipped, 0 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2570/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Success
Sending email for trigger: Success
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
All tests passed

Re: ZooKeeper 3.5 blocker issues

[Fangmin Lv]

Andor,

Here is the PR to port ZK-3104 from master to 3.4:
https://github.com/apache/zookeeper/pull/685.

Fangmin

On Fri, Nov 2, 2018 at 11:46 AM Fangmin Lv  wrote:

> Hi Andor,
>
> Is anyone working on ZK-2778? I can pick it up if there is no one working
> on it yet.
>
> I'll open a 3.5 PR for ZK-3104 today.
>
> Fangmin
>
> On Fri, Oct 26, 2018 at 3:33 AM Andor Molnar  wrote:
>
>> Hi folks,
>>
>> You’ve probably realised lots of update emails coming from Jira. Please
>> be aware that we’ve updated a bunch of open blocker/critical 3.5 tickets to
>> reflect to what we discussed in this email.
>>
>> If you open up the following jira filter:
>>
>> project = ZooKeeper and resolution = Unresolved and fixVersion = 3.5.5
>> AND priority in (blocker, critical) ORDER BY priority DESC, key ASC
>>
>> You’ll see the most up-to-date list of tickets which need to be addressed
>> before the stable 3.5 release.
>>
>> Thank you for your efforts to get this done.
>>
>> Fangmin, ZK-3104 is waiting for backport, but ticket has already been
>> resolved. Have you created a separate ticket for the backport or shall I
>> just reopen it with the right fix versions?
>>
>> Thanks,
>> Andor
>>
>>
>>
>> > On 2018. Oct 8., at 12:34, Andor Molnar  wrote:
>> >
>> > Hi,
>> >
>> > Let me summarize and give a quick update on the outstanding issues for
>> 3.5 GA:
>> >
>> > - ZOOKEEPER-1818 (Fix don't care for trunk)
>> > - ZOOKEEPER-2778 (Potential server deadlock between follower sync with
>> leader and follower receiving external connection requests.)
>> > - ZOOKEEPER-3021 Migrate project structure to Maven (ongoing)
>> > - ZOOKEEPER-925 Docs generation to Maven
>> > - ZOOKEEPER-3104 (waiting for backport)
>> > - ZOOKEEPER-3125 (waiting for backport PR #647)
>> >
>> > The 2 Maven related tickets are no-brainers as well as the backports.
>> ZK-2778 has been picked up by Maoling (thanks!) as far as I can see,
>> ZK-1818 is the only one waiting for a volunteer.
>> >
>> > Please correct me if I’ve missed something.
>> >
>> > Regards,
>> > Andor
>> >
>> >
>> >
>> >
>> >> On 2018. Sep 28., at 18:32, Tamas Penzes 
>> wrote:
>> >>
>> >> Hi All,
>> >>
>> >> I would add ZOOKEEPER-3021
>> >>  Migrate project
>> >> structure to Maven build as a blocker too. Since the migration has
>> started
>> >> it would be good to finish before releasing ZK 3.5.x GA.
>> >>
>> >> ZOOKEEPER-925 
>> replace
>> >> our forrest site and documentation generation might also be a good
>> idea,
>> >> since then we could deliver the new MarkDown based documentation.
>> >>
>> >> Regards, Tamaas
>> >>
>> >> On Fri, Sep 14, 2018 at 10:09 AM Fangmin Lv 
>> wrote:
>> >>
>> >>> Oh, sorry for the confusion, I should provide more context.
>> >>>
>> >>> Leader will use on disk txn sync with followers to if the peer zxid
>> is not
>> >>> in it's in memory commit logs, the code is here: Leader on disk txn
>> sync
>> >>> <
>> >>>
>> https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java#L774
>>  .
>> >>> There is bug that potentially there will be gap in the txn files, like
>> >>> after snap sync, etc, so it's possible the peer will miss txns due to
>> this.
>> >>>
>> >>> The option to disable it is snapshotSizeFactor
>> >>> <
>> >>>
>> https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/ZKDatabase.java#L81
>>  ,
>> >>> set it to -1 will disable this feature. On 3.5, it's better to have a
>> PR to
>> >>> set this to -1 by default. It might have more SNAP sync, but from our
>> prod
>> >>> it doesn't seem to be a big problem to me.
>> >>>
>> >>> I can send out the diff to disable it by default on 3.5 if you guys
>> think
>> >>> this is the right way to do.
>> >>>
>> >>> Thanks,
>> >>> Fangmin
>> >>>
>> >>> On Thu, Sep 13, 2018 at 1:58 AM Andor Molnar 
>> wrote:
>> >>>
>>  What’s needed to turn it off?
>>  Do we need a PR or it’s just a config option?
>>  Shall we implement a feature switch for that and turn it off by
>> default?
>> 
>>  Sorry I don’t have too much insight on disk txn sync.
>> 
>>  Andor
>> 
>> 
>> 
>> > On 2018. Sep 13., at 9:16, Fangmin Lv  wrote:
>> >
>> > And to be clear, ZOOKEEPER-2418 is actually just one case of
>>  inconsistency
>> > which could caused by on disk txn sync, as I mentioned in a newer
>> JIRA
>> > ZOOKEEPER-2846 <
>> https://issues.apache.org/jira/browse/ZOOKEEPER-2846>,
>>  the
>> > snap sync or txn sync could also leave txns gap in the txn file,
>> which
>>  is a
>> > more common case could trigger this issue.
>> >
>> > I would suggest to turn off the on disk txn sync by default for now
>> to
>> > avoid this issue, after we finished ZOOKEEPER-3114, we can use that
>> to
>> > validate the on disk txns during syncing.
>> >
>> > Thanks,

[GitHub] zookeeper pull request #685: [ZOOKEEPER-3104] Fix potential data inconsisten...

[lvfangmin]

GitHub user lvfangmin opened a pull request:

https://github.com/apache/zookeeper/pull/685

[ZOOKEEPER-3104] Fix potential data inconsistency due to NEWLEADER packet 
being sent too early during SNAP sync

Port this fix from master to 3.5.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/lvfangmin/zookeeper ZOOKEEPER-3104-3.5

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/685.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #685


commit 5703e334a1a775b9f8342409924fc3247475aa81
Author: Fangmin Lyu 
Date:   2018-11-02T20:08:07Z

Fix potential data inconsistency due to NEWLEADER packet being sent too 
early during SNAP sync




---

Re: ZOOKEEPER-3183

[Fangmin Lv]

Great, thanks!

On Fri, Nov 2, 2018 at 12:01 PM Tumati Venky  wrote:

> Sure Fangmin.I will send PR in a day.
>
> Thanks,
> Tumati
>
> On Sat, Nov 3, 2018 at 12:08 AM Fangmin Lv  wrote:
>
>> Thanks Tumati for providing the improvement, please open a pull request
>> and I'll review it.
>>
>> Best,
>> Fangmin
>>
>> On Thu, Nov 1, 2018 at 3:22 PM Andor Molnár  wrote:
>>
>>> Hi Tumati!
>>>
>>>
>>> Of course. Your contribution is much appreciated.
>>>
>>> Would you please open a github pull request, when you're ready with code
>>> change to let the community review it?
>>>
>>>
>>> Thanks,
>>>
>>> Andor
>>>
>>>
>>> On 10/28/18 07:17, Tumati Venky wrote:
>>> > Hi Team,
>>> >
>>> > I see some improvement in WatcherCleaner worker thread.WatcherCleaner
>>> > thread is waiting to get certain dead watchers, the shut down can
>>> happen
>>> > when WatcherCleaner thread is waiting. It will be better if we
>>> Interrupt or
>>> > notify to avoid the long waiting as shut down is initiated. And also
>>> > complete the remaining dead watchers when interrupt happened.
>>> >
>>> > I have created ticket "ZOOKEEPER-3183" for this improvement. I  would
>>> > like to take up this improvement.
>>> >
>>> > Can I proceed to make this change?
>>> >
>>> > Thanks,
>>> > Tumati
>>> >
>>>
>>

Re: ZOOKEEPER-3183

[Tumati Venky]

Sure Fangmin.I will send PR in a day.

Thanks,
Tumati

On Sat, Nov 3, 2018 at 12:08 AM Fangmin Lv  wrote:

> Thanks Tumati for providing the improvement, please open a pull request
> and I'll review it.
>
> Best,
> Fangmin
>
> On Thu, Nov 1, 2018 at 3:22 PM Andor Molnár  wrote:
>
>> Hi Tumati!
>>
>>
>> Of course. Your contribution is much appreciated.
>>
>> Would you please open a github pull request, when you're ready with code
>> change to let the community review it?
>>
>>
>> Thanks,
>>
>> Andor
>>
>>
>> On 10/28/18 07:17, Tumati Venky wrote:
>> > Hi Team,
>> >
>> > I see some improvement in WatcherCleaner worker thread.WatcherCleaner
>> > thread is waiting to get certain dead watchers, the shut down can happen
>> > when WatcherCleaner thread is waiting. It will be better if we
>> Interrupt or
>> > notify to avoid the long waiting as shut down is initiated. And also
>> > complete the remaining dead watchers when interrupt happened.
>> >
>> > I have created ticket "ZOOKEEPER-3183" for this improvement. I  would
>> > like to take up this improvement.
>> >
>> > Can I proceed to make this change?
>> >
>> > Thanks,
>> > Tumati
>> >
>>
>

Re: ZooKeeper 3.5 blocker issues

[Fangmin Lv]

Hi Andor,

Is anyone working on ZK-2778? I can pick it up if there is no one working
on it yet.

I'll open a 3.5 PR for ZK-3104 today.

Fangmin

On Fri, Oct 26, 2018 at 3:33 AM Andor Molnar  wrote:

> Hi folks,
>
> You’ve probably realised lots of update emails coming from Jira. Please be
> aware that we’ve updated a bunch of open blocker/critical 3.5 tickets to
> reflect to what we discussed in this email.
>
> If you open up the following jira filter:
>
> project = ZooKeeper and resolution = Unresolved and fixVersion = 3.5.5 AND
> priority in (blocker, critical) ORDER BY priority DESC, key ASC
>
> You’ll see the most up-to-date list of tickets which need to be addressed
> before the stable 3.5 release.
>
> Thank you for your efforts to get this done.
>
> Fangmin, ZK-3104 is waiting for backport, but ticket has already been
> resolved. Have you created a separate ticket for the backport or shall I
> just reopen it with the right fix versions?
>
> Thanks,
> Andor
>
>
>
> > On 2018. Oct 8., at 12:34, Andor Molnar  wrote:
> >
> > Hi,
> >
> > Let me summarize and give a quick update on the outstanding issues for
> 3.5 GA:
> >
> > - ZOOKEEPER-1818 (Fix don't care for trunk)
> > - ZOOKEEPER-2778 (Potential server deadlock between follower sync with
> leader and follower receiving external connection requests.)
> > - ZOOKEEPER-3021 Migrate project structure to Maven (ongoing)
> > - ZOOKEEPER-925 Docs generation to Maven
> > - ZOOKEEPER-3104 (waiting for backport)
> > - ZOOKEEPER-3125 (waiting for backport PR #647)
> >
> > The 2 Maven related tickets are no-brainers as well as the backports.
> ZK-2778 has been picked up by Maoling (thanks!) as far as I can see,
> ZK-1818 is the only one waiting for a volunteer.
> >
> > Please correct me if I’ve missed something.
> >
> > Regards,
> > Andor
> >
> >
> >
> >
> >> On 2018. Sep 28., at 18:32, Tamas Penzes 
> wrote:
> >>
> >> Hi All,
> >>
> >> I would add ZOOKEEPER-3021
> >>  Migrate project
> >> structure to Maven build as a blocker too. Since the migration has
> started
> >> it would be good to finish before releasing ZK 3.5.x GA.
> >>
> >> ZOOKEEPER-925 
> replace
> >> our forrest site and documentation generation might also be a good idea,
> >> since then we could deliver the new MarkDown based documentation.
> >>
> >> Regards, Tamaas
> >>
> >> On Fri, Sep 14, 2018 at 10:09 AM Fangmin Lv 
> wrote:
> >>
> >>> Oh, sorry for the confusion, I should provide more context.
> >>>
> >>> Leader will use on disk txn sync with followers to if the peer zxid is
> not
> >>> in it's in memory commit logs, the code is here: Leader on disk txn
> sync
> >>> <
> >>>
> https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java#L774
>  .
> >>> There is bug that potentially there will be gap in the txn files, like
> >>> after snap sync, etc, so it's possible the peer will miss txns due to
> this.
> >>>
> >>> The option to disable it is snapshotSizeFactor
> >>> <
> >>>
> https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/ZKDatabase.java#L81
>  ,
> >>> set it to -1 will disable this feature. On 3.5, it's better to have a
> PR to
> >>> set this to -1 by default. It might have more SNAP sync, but from our
> prod
> >>> it doesn't seem to be a big problem to me.
> >>>
> >>> I can send out the diff to disable it by default on 3.5 if you guys
> think
> >>> this is the right way to do.
> >>>
> >>> Thanks,
> >>> Fangmin
> >>>
> >>> On Thu, Sep 13, 2018 at 1:58 AM Andor Molnar  wrote:
> >>>
>  What’s needed to turn it off?
>  Do we need a PR or it’s just a config option?
>  Shall we implement a feature switch for that and turn it off by
> default?
> 
>  Sorry I don’t have too much insight on disk txn sync.
> 
>  Andor
> 
> 
> 
> > On 2018. Sep 13., at 9:16, Fangmin Lv  wrote:
> >
> > And to be clear, ZOOKEEPER-2418 is actually just one case of
>  inconsistency
> > which could caused by on disk txn sync, as I mentioned in a newer
> JIRA
> > ZOOKEEPER-2846  >,
>  the
> > snap sync or txn sync could also leave txns gap in the txn file,
> which
>  is a
> > more common case could trigger this issue.
> >
> > I would suggest to turn off the on disk txn sync by default for now
> to
> > avoid this issue, after we finished ZOOKEEPER-3114, we can use that
> to
> > validate the on disk txns during syncing.
> >
> > Thanks,
> > Fangmin
> >
> > On Wed, Sep 12, 2018 at 9:55 AM Fangmin Lv 
> >>> wrote:
> >
> >> Andor,
> >>
> >> ZOOKEEPER-3114 is about adding real time digest checking to help
>  detecting
> >> inconsistency, it's a new feature with amounts of code change. I'll
>  start
> >> upstream it part 

Re: ZOOKEEPER-3183

[Fangmin Lv]

Thanks Tumati for providing the improvement, please open a pull request and
I'll review it.

Best,
Fangmin

On Thu, Nov 1, 2018 at 3:22 PM Andor Molnár  wrote:

> Hi Tumati!
>
>
> Of course. Your contribution is much appreciated.
>
> Would you please open a github pull request, when you're ready with code
> change to let the community review it?
>
>
> Thanks,
>
> Andor
>
>
> On 10/28/18 07:17, Tumati Venky wrote:
> > Hi Team,
> >
> > I see some improvement in WatcherCleaner worker thread.WatcherCleaner
> > thread is waiting to get certain dead watchers, the shut down can happen
> > when WatcherCleaner thread is waiting. It will be better if we Interrupt
> or
> > notify to avoid the long waiting as shut down is initiated. And also
> > complete the remaining dead watchers when interrupt happened.
> >
> > I have created ticket "ZOOKEEPER-3183" for this improvement. I  would
> > like to take up this improvement.
> >
> > Can I proceed to make this change?
> >
> > Thanks,
> > Tumati
> >
>

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230468188
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
--- End diff --

Yaa.Thanks for explaining


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230467749
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
+   String trustStorePath,
+   String keyStorePassword,
+   String trustStorePassword) {
+this.keyStorePath = keyStorePath;
+this.trustStorePath = trustStorePath;
+this.keyStorePassword = keyStorePassword;
+this.trustStorePassword = trustStorePassword;
+}
+
+/**
+ * Base class for builder pattern used by subclasses.
+ * @param  the subtype of FileKeyStoreLoader created by the Builder.
+ */
+static abstract class Builder {
+String keyStorePath;
+String trustStorePath;
+String keyStorePassword;
+String trustStorePassword;
+
+Builder() {}
+
+Builder setKeyStorePath(String keyStorePath) {
+this.keyStorePath = Objects.requireNonNull(keyStorePath);
+return this;
+}
+
+Builder setTrustStorePath(String trustStorePath) {
+this.trustStorePath = Objects.requireNonNull(trustStorePath);
+return this;
+}
+
+Builder setKeyStorePassword(String keyStorePassword) {
+this.keyStorePassword = 
Objects.requireNonNull(keyStorePassword);
+return this;
+}
+
+Builder setTrustStorePassword(String trustStorePassword) {
+this.trustStorePassword = 
Objects.requireNonNull(trustStorePassword);
+return this;
+}
+
+abstract T build();
+}
+
+/**
+ * Returns a {@link FileKeyStoreLoader.Builder} that can build a loader
+ * which loads keys and certs from files of the given
+ * {@link KeyStoreFileType}.
+ *
+ * @param type the file type to load keys/certs from.
+ * @return a new Builder.
+ */
+static Builder 
getBuilderForKeyStoreFileType(
--- End diff --

I see it. Thanks for making this change.


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230465652
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
--- End diff --

Also, loading private keys from arbitrary URLs goes against best security 
practices (unless that URL is for a local file, in which case there is no point 
in wrapping it in a URL). TLS private keys should never be transmitted across a 
network. The CA cert (i.e. Trust Store) could in theory be loaded from a remote 
URL though.


---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230463665
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
+synchronized (watcherThread) {
+while (!watcherThread.started) {
+watcherThread.wait();
+}
+}
+}
+
+/**
+ * Tells the background thread to stop. Does not wait for it to exit.
+ */
+public void stop() {
+watcherThread.shouldStop = true;
+watcherThread.interrupt();
+}
+
+/**
+ * Tells the background thread to stop and waits for it to exit. Only 
used by unit tests, which is why it is package
+ 

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230463114
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
--- End diff --

not able to mock or manage the file watcher generally smell some design 
improvement.The same logic implemented in test case if we expose the states of 
file watcher, locking also not required if we expose the state of  file 
watcher.It is good to avoid adding code for testing purpose


---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230461099
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
--- End diff --

How about defining the life cycle for file watcher like start, stop etc.   
and also define the state of the file watcher like starting, running, not 
started and stopping etc. The locking is not required if we define the states. 
The clients can leverage these states if there are any tasks depending on file 
watcher state


---

[GitHub] zookeeper pull request #680: ZOOKEEPER-3174: Quorum TLS - support reloading ...

[tumativ]

Github user tumativ commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/680#discussion_r230463216
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ---
@@ -0,0 +1,180 @@
+/**
+ * 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;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file 
changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the 
user will be called from a background thread.
+ * Some things to keep in mind:
+ * 
+ * The callback should be thread-safe.
+ * Changes that happen around the time the thread is started may be 
missed.
+ * There is a delay between a file changing and the callback 
firing.
+ * The watch is not recursive - changes to subdirectories will not 
trigger a callback.
+ * 
+ */
+public final class FileChangeWatcher {
+private static final Logger LOG = 
LoggerFactory.getLogger(FileChangeWatcher.class);
+
+private final WatcherThread watcherThread;
+
+/**
+ * Creates a watcher that watches dirPath and invokes 
callback on changes.
+ *
+ * @param dirPath the directory to watch.
+ * @param callback the callback to invoke with events. 
event.kind() will return the type of event,
+ * and event.context() will return the 
filename relative to dirPath.
+ * @throws IOException if there is an error creating the WatchService.
+ */
+public FileChangeWatcher(Path dirPath, Consumer> 
callback) throws IOException {
+FileSystem fs = dirPath.getFileSystem();
+WatchService watchService = fs.newWatchService();
+if (LOG.isDebugEnabled()) {
+LOG.debug("Registering with watch service: " + dirPath);
+}
+dirPath.register(
+watchService,
+new WatchEvent.Kind[]{
+StandardWatchEventKinds.ENTRY_CREATE,
+StandardWatchEventKinds.ENTRY_DELETE,
+StandardWatchEventKinds.ENTRY_MODIFY,
+StandardWatchEventKinds.OVERFLOW},
+SensitivityWatchEventModifier.HIGH);
+this.watcherThread = new WatcherThread(watchService, callback);
+this.watcherThread.setDaemon(true);
+this.watcherThread.start();
+}
+
+/**
+ * Waits for the background thread to enter the main loop before 
returning. This method exists mostly to make
+ * the unit tests simpler, which is why it is package private.
+ *
+ * @throws InterruptedException if this thread is interrupted while 
waiting for the background thread to start.
+ */
+void waitForBackgroundThreadToStart() throws InterruptedException {
+synchronized (watcherThread) {
+while (!watcherThread.started) {
+watcherThread.wait();
+}
+}
+}
+
+/**
+ * Tells the background thread to stop. Does not wait for it to exit.
+ */
+public void stop() {
+watcherThread.shouldStop = true;
+watcherThread.interrupt();
+}
+
+/**
+ * Tells the background thread to stop and waits for it to exit. Only 
used by unit tests, which is why it is package
+ 

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/681
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2569/



---

Failed: ZOOKEEPER- PreCommit Build #2568

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2568/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 37.92 KB...]
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=44E32A437E8D6DD743E2A1C3330C5CE4.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 3

Total time: 2 minutes 54 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
ERROR: Step ?Publish JUnit test result report? failed: No test report files 
were found. Configuration error?
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of e50010fc785e392c78835654ae39315e2d5f2a28 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2568/ and 
message: 'FAILURE
 No test results found.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2568/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
No tests ran.

Failed: ZOOKEEPER- PreCommit Build #2569

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2569/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 39.04 KB...]
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build@2/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build@2/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build@2/build.xml:1859:
 exec returned: 2

Total time: 2 minutes 51 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
ERROR: Step ?Publish JUnit test result report? failed: No test report files 
were found. Configuration error?
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3176
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 515c05986467c90217a6383439d330cda8b9031c to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2569/ and 
message: 'FAILURE
 No test results found.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2569/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
No tests ran.

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2568/



---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230455088
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
+   String trustStorePath,
+   String keyStorePassword,
+   String trustStorePassword) {
+this.keyStorePath = keyStorePath;
+this.trustStorePath = trustStorePath;
+this.keyStorePassword = keyStorePassword;
+this.trustStorePassword = trustStorePassword;
+}
+
+/**
+ * Base class for builder pattern used by subclasses.
+ * @param  the subtype of FileKeyStoreLoader created by the Builder.
+ */
+static abstract class Builder {
+String keyStorePath;
+String trustStorePath;
+String keyStorePassword;
+String trustStorePassword;
+
+Builder() {}
+
+Builder setKeyStorePath(String keyStorePath) {
+this.keyStorePath = Objects.requireNonNull(keyStorePath);
+return this;
+}
+
+Builder setTrustStorePath(String trustStorePath) {
+this.trustStorePath = Objects.requireNonNull(trustStorePath);
+return this;
+}
+
+Builder setKeyStorePassword(String keyStorePassword) {
+this.keyStorePassword = 
Objects.requireNonNull(keyStorePassword);
+return this;
+}
+
+Builder setTrustStorePassword(String trustStorePassword) {
+this.trustStorePassword = 
Objects.requireNonNull(trustStorePassword);
+return this;
+}
+
+abstract T build();
+}
+
+/**
+ * Returns a {@link FileKeyStoreLoader.Builder} that can build a loader
+ * which loads keys and certs from files of the given
+ * {@link KeyStoreFileType}.
+ *
+ * @param type the file type to load keys/certs from.
+ * @return a new Builder.
+ */
+static Builder 
getBuilderForKeyStoreFileType(
--- End diff --

@tumativ this is done, please see the latest version of the code


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230454818
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -221,15 +279,45 @@ public SSLContext createSSLContext(ZKConfig config) 
throws SSLContextException {
 }
 }
 
-public static X509KeyManager createKeyManager(String keyStoreLocation, 
String keyStorePassword)
+/**
+ * Creates a key manager by loading the key store from the given file 
of the given type, optionally decrypting it
+ * using the given password.
+ * @param keyStoreLocation the location of the key store file.
+ * @param keyStorePassword optional password to decrypt the key store. 
If empty, assumes the key store is not
+ * encrypted.
+ * @param keyStoreType must be JKS, PEM, or null. If null, attempts to 
autodetect the key store type from the file
+ * extension (.jks / .pem).
+ * @return the key manager.
+ * @throws KeyManagerException if something goes wrong.
+ */
+public static X509KeyManager createKeyManager(String keyStoreLocation, 
String keyStorePassword, StoreFileType keyStoreType)
 throws KeyManagerException {
 FileInputStream inputStream = null;
+if (keyStorePassword == null) {
+keyStorePassword = "";
+}
 try {
 char[] keyStorePasswordChars = keyStorePassword.toCharArray();
 File keyStoreFile = new File(keyStoreLocation);
-KeyStore ks = KeyStore.getInstance("JKS");
-inputStream = new FileInputStream(keyStoreFile);
-ks.load(inputStream, keyStorePasswordChars);
+if (keyStoreType == null) {
+keyStoreType = 
detectStoreFileTypeFromFileExtension(keyStoreFile);
+}
+KeyStore ks;
+switch (keyStoreType) {
--- End diff --

@anmolnar I believe this is done.


---

Failed: ZOOKEEPER- PreCommit Build #2567

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2567/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 86.11 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=DD00E03C0F8B346483DFEA1B71A3875C.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 2

Total time: 27 minutes 57 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3176
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of cf1a4e2ec09970efbed14e7acce9c3906064c0c5 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2567/ and 
message: 'FAILURE
 2357 tests run, 1 skipped, 0 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2567/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
All tests passed

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/681
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2567/



---

Failed: ZOOKEEPER- PreCommit Build #2566

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2566/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 85.92 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=1A6D4C44C83FC70B3A619182A273B39F.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 2

Total time: 13 minutes 41 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 76317e44bc116fdb41c8959a9a0e20c84405d7f4 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2566/ and 
message: 'FAILURE
 2309 tests run, 1 skipped, 1 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2566/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
1 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[0]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2566/



---

Failed: ZOOKEEPER- PreCommit Build #2563

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2563/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 85.92 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 23 minutes 4 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3173
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 9acfecae6dad59f40aa1607346ee2f1cecaac558 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2563/ and 
message: 'FAILURE
 2226 tests run, 2 skipped, 0 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2563/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
All tests passed

[GitHub] zookeeper issue #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/key sto...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/678
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2563/



---

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/680
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2565/



---

Failed: ZOOKEEPER- PreCommit Build #2565

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2565/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 86.80 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 2

Total time: 16 minutes 21 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3174
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of bb06dc8b5e3a40418909aca0a095e9e29e0b3872 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2565/ and 
message: 'FAILURE
 2309 tests run, 1 skipped, 3 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2565/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
3 tests failed.
FAILED:  
org.apache.zookeeper.server.quorum.QuorumPeerMainTest.testLeaderOutOfView

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.QuorumPeerMainTest.testLeaderOutOfView(QuorumPeerMainTest.java:973)
at 
org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)


FAILED:  
org.apache.zookeeper.server.quorum.StandaloneDisabledTest.startSingleServerTest

Error Message:
test timed out after 60 milliseconds

Stack Trace:
org.junit.runners.model.TestTimedOutException: test timed out after 60 
milliseconds
at java.lang.Thread.sleep(Native Method)
at 
org.apache.zookeeper.test.ReconfigTest.testNormalOperation(ReconfigTest.java:179)
at 
org.apache.zookeeper.server.quorum.StandaloneDisabledTest.testReconfig(StandaloneDisabledTest.java:243)
at 
org.apache.zookeeper.server.quorum.StandaloneDisabledTest.startSingleServerTest(StandaloneDisabledTest.java:113)
at 
org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:748)


FAILED:  
org.apache.zookeeper.test.WatchEventWhenAutoResetTest.testNodeDataChanged

Failed: ZOOKEEPER- PreCommit Build #2564

[Apache Jenkins Server]

Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2564/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 86.22 MB...]
 [exec] 
==
 [exec] 
 [exec] 
 [exec] 
 [exec] Error: No value specified for option "issue"
 [exec] Session logged out. Session was 
JSESSIONID=BCED1CB9F965310B07AE183FB29D16FE.
 [exec] 
 [exec] 
 [exec] 
==
 [exec] 
==
 [exec] Finished build.
 [exec] 
==
 [exec] 
==
 [exec] 
 [exec] 
 [exec] mv: 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 and 
'/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess'
 are the same file

BUILD FAILED
/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build.xml:1859:
 exec returned: 1

Total time: 14 minutes 22 seconds
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Recording test results
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
[description-setter] Description set: ZOOKEEPER-3172
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Adding one-line test results to commit status...
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting status of 65abe0d890e51ea36559a280a1b272261c5b50d7 to FAILURE with url 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2564/ and 
message: 'FAILURE
 2304 tests run, 1 skipped, 2 failed.'
Using context: Jenkins

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2564/

Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Email was triggered for: Failure - Any
Sending email for trigger: Failure - Any
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/
Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8
Setting MAVEN_3_LATEST__HOME=/home/jenkins/tools/maven/latest3/



###
## FAILED TESTS (if any) 
##
2 tests failed.
FAILED:  
org.apache.zookeeper.test.WatchEventWhenAutoResetTest.testNodeDataChanged

Error Message:
expected: but was:

Stack Trace:
junit.framework.AssertionFailedError: expected: but 
was:
at 
org.apache.zookeeper.test.WatchEventWhenAutoResetTest$EventsWatcher.assertEvent(WatchEventWhenAutoResetTest.java:67)
at 
org.apache.zookeeper.test.WatchEventWhenAutoResetTest.testNodeDataChanged(WatchEventWhenAutoResetTest.java:126)
at 
org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)


FAILED:  
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer[0]

Error Message:
null

Stack Trace:
junit.framework.AssertionFailedError
at 
org.apache.zookeeper.server.quorum.UnifiedServerSocketTest.testDenialOfServiceResistanceNonStrictServer(UnifiedServerSocketTest.java:426)

[GitHub] zookeeper issue #679: ZOOKEEPER-3172: Quorum TLS - fix port unification to a...

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/zookeeper/pull/679
  

Refer to this link for build results (access rights to CI server needed): 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2564/



---

[GitHub] zookeeper issue #680: ZOOKEEPER-3174: Quorum TLS - support reloading trust/k...

[ivmaykov]

Github user ivmaykov commented on the issue:

https://github.com/apache/zookeeper/pull/680
  
@eolivelli switched to lambdas, kept the finalizer in for now but added a 
TODO to remove it.


---

[GitHub] zookeeper pull request #:

[eolivelli]

Github user eolivelli commented on the pull request:


https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31151817
  
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java:
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java 
on line 544:
To me is okay a new JIRA.

When building on JDK10 you wlil have a "deprecation" warning, I think we 
should add a SuppressWarnings annotation.

ZK is building and running fine on JDK10/JDK11, please check


---

[GitHub] zookeeper pull request #:

[ivmaykov]

Github user ivmaykov commented on the pull request:


https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31151782
  
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java:
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java 
on line 544:
That seems a bit fragile and could potentially introduce bugs, since we 
will need an explicit shutdown method where none existed before. Since we are 
currently targeting Java 8, can I keep the finalizer in this PR and fix it in a 
later PR + JIRA?


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230423126
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -254,23 +282,54 @@ public static X509KeyManager createKeyManager(String 
keyStoreLocation, String ke
 }
 }
 
-public static X509TrustManager createTrustManager(String 
trustStoreLocation, String trustStorePassword,
-  boolean crlEnabled, 
boolean ocspEnabled,
-  final boolean 
serverHostnameVerificationEnabled,
-  final boolean 
clientHostnameVerificationEnabled)
+/**
+ * Creates a trust manager by loading the trust store from the given 
file
+ * of the given type, optionally decrypting it using the given 
password.
+ * @param trustStoreLocation the location of the trust store file.
+ * @param trustStorePassword optional password to decrypt the trust 
store
+ *   (only applies to JKS trust stores). If 
empty,
+ *   assumes the trust store is not encrypted.
+ * @param trustStoreTypeProp must be JKS, PEM, or null. If null, 
attempts
+ *   to autodetect the trust store type from 
the
+ *   file extension (.jks / .pem).
+ * @param crlEnabled enable CRL (certificate revocation list) checks.
+ * @param ocspEnabled enable OCSP (online certificate status protocol)
+ *checks.
+ * @param serverHostnameVerificationEnabled if true, verify hostnames 
of
+ *  remote servers that client
+ *  sockets created by this
+ *  X509Util connect to.
+ * @param clientHostnameVerificationEnabled if true, verify hostnames 
of
+ *  remote clients that server
+ *  sockets created by this
+ *  X509Util accept connections
+ *  from.
+ * @return the trust manager.
+ * @throws TrustManagerException if something goes wrong.
+ */
+public static X509TrustManager createTrustManager(
+String trustStoreLocation,
+String trustStorePassword,
+String trustStoreTypeProp,
+boolean crlEnabled,
+boolean ocspEnabled,
+final boolean serverHostnameVerificationEnabled,
+final boolean clientHostnameVerificationEnabled)
 throws TrustManagerException {
 FileInputStream inputStream = null;
+if (trustStorePassword == null) {
+trustStorePassword = "";
+}
 try {
-File trustStoreFile = new File(trustStoreLocation);
-KeyStore ts = KeyStore.getInstance("JKS");
-inputStream = new FileInputStream(trustStoreFile);
-if (trustStorePassword != null) {
-char[] trustStorePasswordChars = 
trustStorePassword.toCharArray();
-ts.load(inputStream, trustStorePasswordChars);
-} else {
-ts.load(inputStream, null);
-}
-
+KeyStoreFileType storeFileType =
--- End diff --

Will fix.


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230421926
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -221,27 +229,47 @@ public SSLContext createSSLContext(ZKConfig config) 
throws SSLContextException {
 }
 }
 
-public static X509KeyManager createKeyManager(String keyStoreLocation, 
String keyStorePassword)
+/**
+ * Creates a key manager by loading the key store from the given file 
of
+ * the given type, optionally decrypting it using the given password.
+ * @param keyStoreLocation the location of the key store file.
+ * @param keyStorePassword optional password to decrypt the key store. 
If
+ * empty, assumes the key store is not 
encrypted.
+ * @param keyStoreTypeProp must be JKS, PEM, or null. If null, 
attempts to
+ * autodetect the key store type from the file
+ * extension (.jks / .pem).
+ * @return the key manager.
+ * @throws KeyManagerException if something goes wrong.
+ */
+public static X509KeyManager createKeyManager(
+String keyStoreLocation,
+String keyStorePassword,
+String keyStoreTypeProp)
 throws KeyManagerException {
 FileInputStream inputStream = null;
+if (keyStorePassword == null) {
+keyStorePassword = "";
+}
 try {
-char[] keyStorePasswordChars = keyStorePassword.toCharArray();
-File keyStoreFile = new File(keyStoreLocation);
-KeyStore ks = KeyStore.getInstance("JKS");
-inputStream = new FileInputStream(keyStoreFile);
-ks.load(inputStream, keyStorePasswordChars);
+KeyStoreFileType storeFileType =
--- End diff --

Will fix. This also made me realize that `JKSFileLoader` and 
`PEMFileLoader` are leaking file input streams. Will fix that as well.


---

[GitHub] zookeeper pull request #:

[eolivelli]

Github user eolivelli commented on the pull request:


https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31151441
  
In 
zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java:
In 
zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
 on line 189:
on  branch 3.5 and on master we are already jdk8+ so we can use lambdas.
This stuff is not for 3.4 branch.
Btw it is not so important we are in tests


---

[GitHub] zookeeper pull request #:

[eolivelli]

Github user eolivelli commented on the pull request:


https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31151424
  
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java:
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java 
on line 544:
The best is to have complete control over this.
We should run disableCertFileReloading in an explicit  "shutdown"/"close" 
method.

We cannot count on Java finalization, I suggest to drop this 
"finalizerGuardian", we have full control on both server and client 
stopping/close procedures so I think we can safely rely on:
- server shutdown (the process will die)
- client calls ZooKeeper#close (it it does not so the application is 
already buggy)


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230421726
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
--- End diff --

Actually, the PemReader class doesn't currently support `URL`, only `File`. 
So I'd prefer to only support file paths here for now. We can modify it later 
to support loading from URLs, in a separate PR / JIRA.


---

[GitHub] zookeeper pull request #:

[eolivelli]

Github user eolivelli commented on the pull request:


https://github.com/apache/zookeeper/commit/232232e7f338a9eca440d89dcfc8ed22e0336e60#commitcomment-31151383
  
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java:
In zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java 
on line 489:
sorry.
I left this comment before starting the review of the next commit.




---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230419175
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
--- End diff --

Should I rename the class to URLKeyStoreLoader then?


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230418985
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java 
---
@@ -0,0 +1,67 @@
+/**
+ * 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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+
+/**
+ * Implementation of {@link FileKeyStoreLoader} that loads from JKS files.
+ */
+class JKSFileLoader extends FileKeyStoreLoader {
+private static final char[] EMPTY_CHAR_ARRAY = new char[0];
+
+private JKSFileLoader(String keyStorePath,
+  String trustStorePath,
+  String keyStorePassword,
+  String trustStorePassword) {
+super(keyStorePath, trustStorePath, keyStorePassword, 
trustStorePassword);
+}
+
+@Override
+public KeyStore loadKeyStore() throws IOException, 
GeneralSecurityException {
+KeyStore ks = KeyStore.getInstance("JKS");
--- End diff --

Will do


---

[GitHub] zookeeper pull request #678: ZOOKEEPER-3173: Quorum TLS - support PEM trust/...

[ivmaykov]

Github user ivmaykov commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/678#discussion_r230418497
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoader.java
 ---
@@ -0,0 +1,98 @@
+/**
+ * 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;
+
+import java.util.Objects;
+
+/**
+ * Base class for instances of {@link KeyStoreLoader} which load the 
key/trust
+ * stores from files on a filesystem.
+ */
+abstract class FileKeyStoreLoader implements KeyStoreLoader {
+final String keyStorePath;
+final String trustStorePath;
+final String keyStorePassword;
+final String trustStorePassword;
+
+FileKeyStoreLoader(String keyStorePath,
+   String trustStorePath,
+   String keyStorePassword,
+   String trustStorePassword) {
+this.keyStorePath = keyStorePath;
+this.trustStorePath = trustStorePath;
+this.keyStorePassword = keyStorePassword;
+this.trustStorePassword = trustStorePassword;
+}
+
+/**
+ * Base class for builder pattern used by subclasses.
+ * @param  the subtype of FileKeyStoreLoader created by the Builder.
+ */
+static abstract class Builder {
+String keyStorePath;
+String trustStorePath;
+String keyStorePassword;
+String trustStorePassword;
+
+Builder() {}
+
+Builder setKeyStorePath(String keyStorePath) {
+this.keyStorePath = Objects.requireNonNull(keyStorePath);
+return this;
+}
+
+Builder setTrustStorePath(String trustStorePath) {
+this.trustStorePath = Objects.requireNonNull(trustStorePath);
+return this;
+}
+
+Builder setKeyStorePassword(String keyStorePassword) {
+this.keyStorePassword = 
Objects.requireNonNull(keyStorePassword);
+return this;
+}
+
+Builder setTrustStorePassword(String trustStorePassword) {
+this.trustStorePassword = 
Objects.requireNonNull(trustStorePassword);
+return this;
+}
+
+abstract T build();
+}
+
+/**
+ * Returns a {@link FileKeyStoreLoader.Builder} that can build a loader
+ * which loads keys and certs from files of the given
+ * {@link KeyStoreFileType}.
+ *
+ * @param type the file type to load keys/certs from.
+ * @return a new Builder.
+ */
+static Builder 
getBuilderForKeyStoreFileType(
--- End diff --

Okay :( This is the part of java I dislike. The good old 
"AbstractBaseFooFactoryFactory" pattern :/

Relevant: 
https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition


---

[jira] [Commented] (ZOOKEEPER-3169) Reduce session revalidation time after zxid roll over

[JIRA]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16673049#comment-16673049
 ] 

田毅群 commented on ZOOKEEPER-3169:


Hi， could you please help to view the plan in file   "session 
revalidation.docx".  There is a detailed explanation of the plan. 

Besides, I  clone source code of the zookeeper and run test cases in local 
environment.  I change nothing but a few cases can not pass. So I don't commit 
my change now.

> Reduce session revalidation time after zxid roll over
> -
>
> Key: ZOOKEEPER-3169
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3169
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.4.5, 3.5.0
>Reporter: 田毅群
>Priority: Major
> Fix For: 3.4.5
>
> Attachments: session revalidation.docx
>
>
> 1. Sometimes Zookeeper cluster will receive a lot of connections from 
> clients, sometimes connection number even exceeds 1W.  When zxid rolls over, 
> the clients will reconnect and revalidate the session.
> 2. In Zookeeper design structure, when follower server receives the session 
> revalidation requests, it will send requests to leader server, which is 
> designed to be responsible for session revalidation. 
> 3.  In a short time, Leader will handle lots of requests.  I use a tool to 
> get the statistics, some clients need to wait over 20s. It is too long for 
> some special clients, like ResourceManager.
> 4.  I design a thought: when zxid rollover happens. Leader will record the 
> accurate time. When reelection finishs, all servers will get the rollover 
> time. When clients reconnect and revalidate session. All servers can judge 
> it. So it can reduce a lots of pressure of cluster,  all clients can will 
> wait for less time.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Updated] (ZOOKEEPER-3169) Reduce session revalidation time after zxid roll over

[JIRA]

 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

田毅群 updated ZOOKEEPER-3169:
---
Attachment: session revalidation.docx

> Reduce session revalidation time after zxid roll over
> -
>
> Key: ZOOKEEPER-3169
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3169
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.4.5, 3.5.0
>Reporter: 田毅群
>Priority: Major
> Fix For: 3.4.5
>
> Attachments: session revalidation.docx
>
>
> 1. Sometimes Zookeeper cluster will receive a lot of connections from 
> clients, sometimes connection number even exceeds 1W.  When zxid rolls over, 
> the clients will reconnect and revalidate the session.
> 2. In Zookeeper design structure, when follower server receives the session 
> revalidation requests, it will send requests to leader server, which is 
> designed to be responsible for session revalidation. 
> 3.  In a short time, Leader will handle lots of requests.  I use a tool to 
> get the statistics, some clients need to wait over 20s. It is too long for 
> some special clients, like ResourceManager.
> 4.  I design a thought: when zxid rollover happens. Leader will record the 
> accurate time. When reelection finishs, all servers will get the rollover 
> time. When clients reconnect and revalidate session. All servers can judge 
> it. So it can reduce a lots of pressure of cluster,  all clients can will 
> wait for less time.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

ZooKeeper_branch35_jdk8 - Build # 1177 - Still Failing

[Apache Jenkins Server]

See https://builds.apache.org/job/ZooKeeper_branch35_jdk8/1177/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 108.46 KB...]
[junit] Running org.apache.zookeeper.test.SaslClientTest in thread 3
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.647 sec, Thread: 3, Class: org.apache.zookeeper.test.SaslClientTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
4.51 sec, Thread: 4, Class: 
org.apache.zookeeper.test.SaslAuthMissingClientConfigTest
[junit] Running org.apache.zookeeper.test.SaslSuperUserTest in thread 1
[junit] Running org.apache.zookeeper.test.ServerCnxnTest in thread 3
[junit] Running org.apache.zookeeper.test.SessionInvalidationTest in thread 
4
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
4.62 sec, Thread: 1, Class: org.apache.zookeeper.test.SaslSuperUserTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
5.57 sec, Thread: 3, Class: org.apache.zookeeper.test.ServerCnxnTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.77 sec, Thread: 4, Class: org.apache.zookeeper.test.SessionInvalidationTest
[junit] Running org.apache.zookeeper.test.SessionTest in thread 1
[junit] Running org.apache.zookeeper.test.SessionTimeoutTest in thread 3
[junit] Running org.apache.zookeeper.test.SessionTrackerCheckTest in thread 
4
[junit] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.676 sec, Thread: 4, Class: org.apache.zookeeper.test.SessionTrackerCheckTest
[junit] Running org.apache.zookeeper.test.SessionUpgradeTest in thread 4
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
6.664 sec, Thread: 3, Class: org.apache.zookeeper.test.SessionTimeoutTest
[junit] Running org.apache.zookeeper.test.StandaloneTest in thread 3
[junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
16.899 sec, Thread: 1, Class: org.apache.zookeeper.test.SessionTest
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
4.861 sec, Thread: 3, Class: org.apache.zookeeper.test.StandaloneTest
[junit] Running org.apache.zookeeper.test.StatTest in thread 1
[junit] Running org.apache.zookeeper.test.StaticHostProviderTest in thread 3
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
6.889 sec, Thread: 1, Class: org.apache.zookeeper.test.StatTest
[junit] Tests run: 26, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
6.656 sec, Thread: 3, Class: org.apache.zookeeper.test.StaticHostProviderTest
[junit] Running org.apache.zookeeper.test.StringUtilTest in thread 1
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.43 sec, Thread: 1, Class: org.apache.zookeeper.test.StringUtilTest
[junit] Running org.apache.zookeeper.test.SyncCallTest in thread 3
[junit] Running org.apache.zookeeper.test.TruncateTest in thread 1
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
31.981 sec, Thread: 4, Class: org.apache.zookeeper.test.SessionUpgradeTest
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
4.576 sec, Thread: 3, Class: org.apache.zookeeper.test.SyncCallTest
[junit] Running org.apache.zookeeper.test.WatchedEventTest in thread 4
[junit] Running org.apache.zookeeper.test.WatchEventWhenAutoResetTest in 
thread 3
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.474 sec, Thread: 4, Class: org.apache.zookeeper.test.WatchedEventTest
[junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
10.526 sec, Thread: 1, Class: org.apache.zookeeper.test.TruncateTest
[junit] Running org.apache.zookeeper.test.WatcherFuncTest in thread 4
[junit] Running org.apache.zookeeper.test.WatcherTest in thread 1
[junit] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
9.888 sec, Thread: 4, Class: org.apache.zookeeper.test.WatcherFuncTest
[junit] Running org.apache.zookeeper.test.X509AuthTest in thread 4
[junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.541 sec, Thread: 4, Class: org.apache.zookeeper.test.X509AuthTest
[junit] Running org.apache.zookeeper.test.ZkDatabaseCorruptionTest in 
thread 4
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
34.464 sec, Thread: 3, Class: 
org.apache.zookeeper.test.WatchEventWhenAutoResetTest
[junit] Running org.apache.zookeeper.test.ZooKeeperQuotaTest in thread 3
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
6.352 sec, Thread: 3, Class: org.apache.zookeeper.test.ZooKeeperQuotaTest
[junit] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
40.834 sec, Thread: 1, Class: org.apache.zookeeper.test.WatcherTest

[GitHub] zookeeper pull request #673: [ZOOKEEPER-3177] Refactor request throttle logi...

[hanm]

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/673#discussion_r230276535
  
--- Diff: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java ---
@@ -68,8 +68,39 @@
 
 private volatile boolean stale = false;
 
+AtomicLong outstandingCount = new AtomicLong();
+
+/** The ZooKeeperServer for this connection. May be null if the server
+ * is not currently serving requests (for example if the server is not
+ * an active quorum participant.
+ */
+final ZooKeeperServer zkServer;
+
+public ServerCnxn(final ZooKeeperServer zkServer) {
+this.zkServer = zkServer;
+}
+
 abstract int getSessionTimeout();
 
+public void incrOutstandingAndCheckThrottle(RequestHeader h) {
+if (h.getXid() <= 0) {
+return;
+}
+if (zkServer.shouldThrottle(outstandingCount.incrementAndGet())) {
+disableRecv(false);
+}
+}
+
+// will be called from zkServer.processPacket
+public void decrOutstandingAndCheckThrottle(ReplyHeader h) {
--- End diff --

This is a good suggestion. This PR focus on consolidating and refactoring 
existing logic, and we usually prefer doing one thing at a time (so the patch 
is easier to review and land). Feel free to fire a PR for improvements after 
this patch is merged!


---