[GitHub] zookeeper issue #708: ZOOKEEPER-3029 - add pom.xml for jute, client and serv...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 retest this please ---
[GitHub] zookeeper issue #708: ZOOKEEPER-3029 - add pom.xml for jute, client and serv...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 retest this please ---
[GitHub] zookeeper issue #708: ZOOKEEPER-3029 - add pom.xml for jute, client and serv...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 retest this please ---
[GitHub] zookeeper issue #708: ZOOKEEPER-3029 - add pom.xml for jute, client and serv...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 retest this please ---
[GitHub] zookeeper issue #708: ZOOKEEPER-3029 - add pom.xml for jute, client and serv...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 Everything looks good. Ant works as before, maven builds jute, server and client (c). Tests run - unfortunately only on 1 thread - I get address already in use a lot of time, way more often than with ant. ---
[GitHub] zookeeper issue #728: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/728 Thanks @arankin-irl , I will review it (I'm guessing it's the same as for 3.5, so it will be quick), but I am not a committer, I won't be able to merge your PR :( ---
[GitHub] zookeeper issue #715: Rollup of blocker/critical fixes for 3.5 (to trigger C...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/715 Oh, so it's basically an aggregation of the flaky test fixes, I already seen some of your PR on them. This is awesome, thanks for this! ---
[GitHub] zookeeper issue #715: Rollup of blocker/critical fixes for 3.5 (to trigger C...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/715 I'm not sure what this is. Do you have a corresponding jira? ---
[GitHub] zookeeper pull request #724: ZOOKEEPER-2488: Synchronized access to shutting...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/724#discussion_r238194209 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java --- @@ -1162,10 +1162,12 @@ public void run() { }; try { roZkMgr.start(); -reconfigFlagClear(); -if (shuttingDownLE) { -shuttingDownLE = false; -startLeaderElection(); +synchronized (this) { +reconfigFlagClear(); +if (shuttingDownLE) { --- End diff -- Wouldn't it make more sense to make shuttingDownLE AtomicBoolean and use compareAndSet() here? The two methods (reconfigFlagClear and startLeaderElection) is already synchronized anyway, and only synchronized blocks access shuttingDownLE. What do you think? ---
[GitHub] zookeeper issue #654: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/654 Yes please. But it should be fairly easy, most of the time 3.5 and master are compatible. So just cherry-picking your commits on master should work, hopefully without merge conflicts. If you need any help, let me know! ---
[GitHub] zookeeper issue #654: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/654 I just realized this is for 3.5. Did you create a PR for the master branch? 3.5 is pretty much closed for new features, in order to stabilize it for the stable release. ---
[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/720#discussion_r236578709 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java --- @@ -50,6 +50,8 @@ public final int getChildren = 8; +public final int getAllChildrenNumber = 20; --- End diff -- 20 is already taken for deleteContainer, please change it to something else (22 seems to be the first free number until 100) ---
[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/720#discussion_r236036464 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java --- @@ -2495,6 +2495,30 @@ public void setACL(final String path, List acl, int version, return getChildren(path, watch ? watchManager.defaultWatcher : null); } +/* +* Get all children number of one node +* */ +public int getAllChildrenNumber(final String path) +throws KeeperException, InterruptedException { +int totalNumber = 0; +final String clientPath = path; +PathUtils.validatePath(clientPath); +// the watch contains the un-chroot path +WatchRegistration wcb = null; +final String serverPath = prependChroot(clientPath); +RequestHeader h = new RequestHeader(); +h.setType(ZooDefs.OpCode.getAllChildrenNumber); +GetAllChildrenNumberRequest request = new GetAllChildrenNumberRequest(); --- End diff -- You need to import the class GetAllChildrenNumberRequest which jute generates. ---
[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/720#discussion_r236036475 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java --- @@ -616,6 +616,14 @@ private void processEvent(Object event) { } else { cb.processResult(rc, clientPath, p.ctx, null); } + } else if (p.response instanceof GetAllChildrenNumberResponse) { --- End diff -- Also need to import GetAllChildrenNumberResponse ---
[GitHub] zookeeper issue #708: WIP - ZOOKEEPER-3029 - add pom.xml for jute, client an...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/708 This PR is not yet complete, please wait with the merge! ---
[GitHub] zookeeper pull request #708: WIP - ZOOKEEPER-3029 - add pom.xml for jute, cl...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/708 WIP - ZOOKEEPER-3029 - add pom.xml for jute, client and server Work In Progress Creating the maven build for zookeeper-server, zookeeper-jute and zookeeper-client (which only contains C client right now, and it hasn't been implemented yet - maven is not calling make or autoconf) You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3029r Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/708.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 #708 commit c09f4be3b4bb1009101dccece139796ae34bbb4c Author: Norbert Kalmar Date: 2018-10-25T19:49:43Z ZOOKEEPER-3029 - add pom.xml for jute, client and server ---
[GitHub] zookeeper issue #692: ZOOKEEPER-3184: Use the same method to generate websit...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/692 I also like the design, compared to HBase, I can definitely see the inspiration :) Good job, +1 non-binding ---
[GitHub] zookeeper pull request #654: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/654#discussion_r233916605 --- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java --- @@ -0,0 +1,12 @@ +package org.apache.zookeeper.common; --- End diff -- Add apache header ---
[GitHub] zookeeper pull request #654: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/654#discussion_r233916877 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java --- @@ -0,0 +1,18 @@ +package org.apache.zookeeper.common; --- End diff -- add Apache header ---
[GitHub] zookeeper issue #654: ZOOKEEPER-3160: Custom User SSLContext
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/654 Unfortunately the link at the Jenkins build are wrong in some cases. Just navigate manually from build artifacts in the jenkins build. findbugs for example: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2645/artifact/build/test/findbugs/newPatchFindbugsWarnings.html ---
[GitHub] zookeeper pull request #690: ZOOKEEPER-3179: Add snapshot compression to red...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/690#discussion_r232325878 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/SnapStream.java --- @@ -0,0 +1,336 @@ +/** + * 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.server.persistence; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.zip.Adler32; +import java.util.zip.CheckedInputStream; +import java.util.zip.CheckedOutputStream; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; + +import org.apache.jute.InputArchive; +import org.apache.jute.OutputArchive; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.xerial.snappy.SnappyCodec; +import org.xerial.snappy.SnappyInputStream; +import org.xerial.snappy.SnappyOutputStream; + +/** + * Represent the Stream used in serialize and deserialize the Snapshot. + */ +public class SnapStream { + +private static final Logger LOG = LoggerFactory.getLogger(SnapStream.class); + +public static final String ZOOKEEPER_SHAPSHOT_STREAM_MODE = +"zookeeper.snapshot.compression.method"; + +private static StreamMode streamMode = +StreamMode.fromString( +System.getProperty(ZOOKEEPER_SHAPSHOT_STREAM_MODE, + StreamMode.DEFAULT_MODE.getName())); + +static { +LOG.info(ZOOKEEPER_SHAPSHOT_STREAM_MODE + "=" + streamMode); +} + +public static enum StreamMode { +GZIP("gz"), +SNAPPY("snappy"), +CHECKED(""); + +public static final StreamMode DEFAULT_MODE = CHECKED; + +private String name; + +StreamMode(String name) { + this.name = name; +} + +public String getName() { +return name; +} + +public String getFileExtension() { +return name.isEmpty() ? "" : "." + name; +} + +public static StreamMode fromString(String name) { +for (StreamMode c : values()) { +if (c.getName().compareToIgnoreCase(name) == 0) { +return c; +} +} +return DEFAULT_MODE; +} +} + +/** + * Return the CheckedInputStream based on the extension of the fileName. + * + * @param fileName the file the InputStream read from + * @return the specific InputStream + * @throws IOException + */ +public static CheckedInputStream getInputStream(File file) throws IOException { +FileInputStream fis = new FileInputStream(file); +InputStream is; +switch (getStreamMode(file.getName())) { +case GZIP: +is = new GZIPInputStream(fis); +break; +case SNAPPY: +is = new SnappyInputStream(fis); +break; +case CHECKED: +default: +is = new BufferedInputStream(fis); +} +return new CheckedInputStream(is, new Adler32()); +} + +/** + * Return the OutputStream based on predefined stream mode. + * + * @param fileName the file the OutputStream writes to + * @return the specific OutputStream + * @throws IOException
[GitHub] zookeeper pull request #690: ZOOKEEPER-3179: Add snapshot compression to red...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/690#discussion_r232325666 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/SnapStream.java --- @@ -0,0 +1,336 @@ +/** + * 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.server.persistence; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.zip.Adler32; +import java.util.zip.CheckedInputStream; +import java.util.zip.CheckedOutputStream; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; + +import org.apache.jute.InputArchive; +import org.apache.jute.OutputArchive; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.xerial.snappy.SnappyCodec; +import org.xerial.snappy.SnappyInputStream; +import org.xerial.snappy.SnappyOutputStream; + +/** + * Represent the Stream used in serialize and deserialize the Snapshot. + */ +public class SnapStream { + +private static final Logger LOG = LoggerFactory.getLogger(SnapStream.class); + +public static final String ZOOKEEPER_SHAPSHOT_STREAM_MODE = +"zookeeper.snapshot.compression.method"; + +private static StreamMode streamMode = +StreamMode.fromString( +System.getProperty(ZOOKEEPER_SHAPSHOT_STREAM_MODE, + StreamMode.DEFAULT_MODE.getName())); + +static { +LOG.info(ZOOKEEPER_SHAPSHOT_STREAM_MODE + "=" + streamMode); +} + +public static enum StreamMode { +GZIP("gz"), +SNAPPY("snappy"), +CHECKED(""); + +public static final StreamMode DEFAULT_MODE = CHECKED; + +private String name; + +StreamMode(String name) { + this.name = name; +} + +public String getName() { +return name; +} + +public String getFileExtension() { +return name.isEmpty() ? "" : "." + name; +} + +public static StreamMode fromString(String name) { +for (StreamMode c : values()) { +if (c.getName().compareToIgnoreCase(name) == 0) { +return c; +} +} +return DEFAULT_MODE; +} +} + +/** + * Return the CheckedInputStream based on the extension of the fileName. + * + * @param fileName the file the InputStream read from + * @return the specific InputStream + * @throws IOException + */ +public static CheckedInputStream getInputStream(File file) throws IOException { +FileInputStream fis = new FileInputStream(file); +InputStream is; +switch (getStreamMode(file.getName())) { +case GZIP: +is = new GZIPInputStream(fis); +break; +case SNAPPY: +is = new SnappyInputStream(fis); +break; +case CHECKED: +default: +is = new BufferedInputStream(fis); +} +return new CheckedInputStream(is, new Adler32()); +} + +/** + * Return the OutputStream based on predefined stream mode. + * + * @param fileName the file the OutputStream writes to + * @return the specific OutputStream + * @throws IOException
[GitHub] zookeeper pull request #690: ZOOKEEPER-3179: Add snapshot compression to red...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/690#discussion_r232326140 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/SnapStream.java --- @@ -0,0 +1,336 @@ +/** + * 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.server.persistence; + +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.zip.Adler32; +import java.util.zip.CheckedInputStream; +import java.util.zip.CheckedOutputStream; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; + +import org.apache.jute.InputArchive; +import org.apache.jute.OutputArchive; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.xerial.snappy.SnappyCodec; +import org.xerial.snappy.SnappyInputStream; +import org.xerial.snappy.SnappyOutputStream; + +/** + * Represent the Stream used in serialize and deserialize the Snapshot. + */ +public class SnapStream { + +private static final Logger LOG = LoggerFactory.getLogger(SnapStream.class); + +public static final String ZOOKEEPER_SHAPSHOT_STREAM_MODE = +"zookeeper.snapshot.compression.method"; + +private static StreamMode streamMode = +StreamMode.fromString( +System.getProperty(ZOOKEEPER_SHAPSHOT_STREAM_MODE, + StreamMode.DEFAULT_MODE.getName())); + +static { +LOG.info(ZOOKEEPER_SHAPSHOT_STREAM_MODE + "=" + streamMode); +} + +public static enum StreamMode { +GZIP("gz"), +SNAPPY("snappy"), +CHECKED(""); + +public static final StreamMode DEFAULT_MODE = CHECKED; + +private String name; + +StreamMode(String name) { + this.name = name; +} + +public String getName() { +return name; +} + +public String getFileExtension() { +return name.isEmpty() ? "" : "." + name; +} + +public static StreamMode fromString(String name) { +for (StreamMode c : values()) { +if (c.getName().compareToIgnoreCase(name) == 0) { +return c; +} +} +return DEFAULT_MODE; +} +} + +/** + * Return the CheckedInputStream based on the extension of the fileName. + * + * @param fileName the file the InputStream read from + * @return the specific InputStream + * @throws IOException + */ +public static CheckedInputStream getInputStream(File file) throws IOException { +FileInputStream fis = new FileInputStream(file); +InputStream is; +switch (getStreamMode(file.getName())) { +case GZIP: +is = new GZIPInputStream(fis); +break; +case SNAPPY: +is = new SnappyInputStream(fis); +break; +case CHECKED: +default: +is = new BufferedInputStream(fis); +} +return new CheckedInputStream(is, new Adler32()); +} + +/** + * Return the OutputStream based on predefined stream mode. + * + * @param fileName the file the OutputStream writes to + * @return the specific OutputStream + * @throws IOException
[GitHub] zookeeper issue #688: reduce session revalidation time after zxid roll over
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/688 LGTM. Please create a jira for this PR and/or mention the jira number in the commit message and PR name. Thanks! ---
[GitHub] zookeeper issue #687: Zookeeper 3169
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/687 I think you need to rebase before commiting. But theres too much commit, are you sure you branched from master, and not from i.e. 3.4? ---
[GitHub] zookeeper pull request #675: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.4 - ...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/675 ---
[GitHub] zookeeper pull request #674: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - ...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/674 ---
[GitHub] zookeeper pull request #675: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.4 - ...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/675 ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.4 - zookeeper-server You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032r-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/675.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 #675 commit 98a30ac3f284cc82d88759e59a46ad0d7b27b229 Author: Norbert Kalmar Date: 2018-10-19T19:04:22Z ZOOKEEPER-3032: MAVEN MIGRATION - zookeeper-server ---
[GitHub] zookeeper pull request #674: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - ...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/674 ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - zookeeper-server Separating the java code is not feasible. Moving common and client back to server. Author: Norbert Kalmar Reviewers: an...@apache.org Closes #672 from nkalmar/ZOOKEEPER-3032r You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032r-3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/674.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 #674 commit 70e5c926ea524a2e8e486cee116398fe12a19be1 Author: Norbert Kalmar Date: 2018-10-19T12:39:50Z ZOOKEEPER-3032: MAVEN MIGRATION - zookeeper-server Separating the java code is not feasible. Moving common and client back to server. Author: Norbert Kalmar Reviewers: an...@apache.org Closes #672 from nkalmar/ZOOKEEPER-3032r ---
[GitHub] zookeeper pull request #670: DO_NOT_MERGE - MAVEN MIGRATION - Separation of ...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/670 ---
[GitHub] zookeeper issue #672: ZOOKEEPER-3032 - MAVEN MIGRATION - zookeeper-server
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/672 retest this please ---
[GitHub] zookeeper issue #672: ZOOKEEPER-3032 - MAVEN MIGRATION - zookeeper-server
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/672 retest this please ---
[GitHub] zookeeper pull request #672: ZOOKEEPER-3032 - MAVEN MIGRATION - zookeeper-se...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/672 ZOOKEEPER-3032 - MAVEN MIGRATION - zookeeper-server Separating the java code is not feasible. Moving common and client back to server. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032r Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/672.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 #672 commit d3f351e8e657b504ec296ba3f847ae4f3d435004 Author: Norbert Kalmar Date: 2018-10-16T12:28:17Z ZOOKEEPER-3032 Move zookeeper-common and zookeeper-client-java back to zookeeper-server ---
[GitHub] zookeeper pull request #670: DO_NOT_MERGE - MAVEN MIGRATION - Separation of ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/670#discussion_r225422359 --- Diff: zookeeper-client/zookeeper-client-java/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java --- @@ -23,7 +23,7 @@ import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.ZooKeeper; import org.apache.zookeeper.common.ZKConfig; -import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException; +import org.apache.zookeeper.server.quorum.ConfigException; --- End diff -- Yes, one of the main reasons (not just this particular instance) that it cannot be separated "nicely". zookeeper-common and zookeeper-client-java will probably merge back to zookeeper-server. ---
[GitHub] zookeeper pull request #670: DO_NOT_MERGE - MAVEN MIGRATION - Separation of ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/670#discussion_r225420100 --- Diff: zookeeper-common/pom.xml --- @@ -0,0 +1,123 @@ + +http://maven.apache.org/POM/4.0.0; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd;> + + 4.0.0 + +org.apache.zookeeper +zookeeper +2.6.0-SNAPSHOT + + + zookeeper-common + Apache ZooKeeper - Common + ZooKeeper common + + + + org.apache.zookeeper + zookeeper-jute + 2.6.0-SNAPSHOT --- End diff -- This is a strongly work in progress, and possibly it will never make it into the codebase. I created this PR to show that it is possibly not a good idea to seperate server and client java code :( ---
[GitHub] zookeeper pull request #670: DO_NOT_MERGE - MAVEN MIGRATION - Separation of ...
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/670#discussion_r225420204 --- Diff: zookeeper-client/zookeeper-client-java/pom.xml --- @@ -0,0 +1,59 @@ + +http://maven.apache.org/POM/4.0.0; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd;> + + 4.0.0 + +org.apache.zookeeper +zookeeper +2.6.0-SNAPSHOT --- End diff -- Yes, it was like this on the master pom... I will definitely fix it in a "real" PR. ---
[GitHub] zookeeper pull request #670: DO_NOT_MERGE - MAVEN MIGRATION - Separation of ...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/670 DO_NOT_MERGE - MAVEN MIGRATION - Separation of server and client This is to show that unlike I thought at the start of maven migration, separating java client and server code is too much of a refactor, and even introduces package change (like ConfigException needs to be removed from QuorumPeerConfig.ConfigException as both client and server related classes throws it. Moving QuorumPeerConfig would bring too much file in common.) Also, zookeeper-common, zookeeper-client and zookeeper-server would have similar packages, sometimes tripling the package names. Dependency hierarchy wise server would depend on client, there's no way to make the two depend only on common. And I didn't even start looking at the tests. Most of them depend on ZooKeeper.java and other "main" classes. My suggestion: just leave zookeeper-server, or just zookeeper. An email has been sent to the dev list to start the discussion on this. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3029 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/670.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 #670 commit dae0114a829d3f33198c76bf732b35961d5c26b7 Author: Norbert Kalmar Date: 2018-10-15T13:51:52Z ZOOKEEPER-3029 - MAVEN MIGRATION - create pom for core modules commit 9c0e82fe37f27910daaf20248b3253e2aacef129 Author: Norbert Kalmar Date: 2018-10-15T19:34:50Z common-build-works commit 0ba918a53d8204bdb5bdf7d5967d3f3382ca4a70 Author: Norbert Kalmar Date: 2018-10-15T21:12:25Z client in progress ---
[GitHub] zookeeper issue #662: ZOOKEEPER-3162. Broken lock semantics in C client lock...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/662 retest this please ---
[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/665 retest this please ---
[GitHub] zookeeper pull request #658: ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 m...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/658 ---
[GitHub] zookeeper issue #662: ZOOKEEPER-3162. Broken lock semantics in C client lock...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/662 Looks good, thanks @andreareale The C test fail looks like it's the flaky one: *** Error in `./zktest-mt': free(): invalid pointer: 0x2b913dcd5000 *** I think there's already a fix underway. This should also go into 3.5 as well (possibly to 3.4 especially the include path fix). ---
[GitHub] zookeeper issue #665: [ZOOKEEPER-3163] Use session map in the Netty to impro...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/665 retest this please ---
[GitHub] zookeeper issue #658: ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 move jav...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/658 retest this please ---
[GitHub] zookeeper issue #658: ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 move jav...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/658 Sorry, I know this has some issue, but I escaped the digital world for the past 3 days. I will get back to this PR now... ---
[GitHub] zookeeper pull request #656: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - ...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/656 ---
[GitHub] zookeeper pull request #658: ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 m...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/658 ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 move java server, client You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/658.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 #658 commit 34692d3814540406ac5a7e83e4e025eff520dbe1 Author: Norbert Kalmar Date: 2018-10-05T12:25:43Z ZOOKEEPER-3032 - MAVEN MIGRATION - branch 3.4 move java server, client ---
[GitHub] zookeeper pull request #656: ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - ...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/656 ZOOKEEPER-3032: MAVEN MIGRATION - branch-3.5 - move java server, client Author: Norbert Kalmar Reviewers: h...@apache.org, an...@apache.org Closes #633 from nkalmar/ZOOKEEPER-3032 and squashes the following commits: 3f9a0eca [Norbert Kalmar] ZOOKEEPER-3032 fix flaky QuorumPeerMainTest.testLeaderElectionWithDisloyalVoter_stillHasMajority 27295ed6 [Norbert Kalmar] ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, client You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032-3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/656.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 #656 commit 3a5b8b2fbe990e9825d10d72da46a067211d6ed4 Author: Norbert Kalmar Date: 2018-10-05T12:25:43Z ZOOKEEPER-3032: MAVEN MIGRATION - move java server, client Author: Norbert Kalmar Reviewers: h...@apache.org, an...@apache.org Closes #633 from nkalmar/ZOOKEEPER-3032 and squashes the following commits: 3f9a0eca [Norbert Kalmar] ZOOKEEPER-3032 fix flaky QuorumPeerMainTest.testLeaderElectionWithDisloyalVoter_stillHasMajority 27295ed6 [Norbert Kalmar] ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, client ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 retest this please ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 retest this please ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 retest this please ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 retest this please ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 I'm starting to think Jenkins hates me. I did multiple tests on a Linux server. Although it did fail sometime, after adding more memory, as much as the Jenkins job does, it passes all the time. I run it with: ant test -Dtest.junit.threads=8 -Dtest.junit.maxmem=2g (Oh, on 1 thread it never fails). First I thought there is a bug in PortAssignment, but I debugged that, looks fine. I'm still investigating, and also re-running Jenkins job maybe I get a green build... but this is extremely flaky, so I hope I find something... ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 org.apache.zookeeper.server.quorum.QuorumPeerMainTest.testLeaderElectionWithDisloyalVoter_stillHasMajority seems pretty flaky, bu it runs without a problem on my local machine. Anyway, one time it was like 10 failure, the next run had only the mentioned test fail. Hopefully I will get a green build, looks OK on local build... ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 A green build would be nice. I rebased, as it seems not perfectly, as there are build errors. Fixing them right now. ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 It is preserved, I double checked that. I also read multiple articles on the topic, there was a good one, which I can't find anymore... The points was the ones I wrote - mainly, github web client can't be set to use --follow which is very unfortunate. And this is how git works. It doesn't really care about files, rather than content. But for example, as I mentioned, in my Idea, it does the --follow automatically, at least I can see the logs just like before. So it depends on the Git client too. There are some workarounds, but those actually rewrite the history, which is a huge NO in my opinion. ---
[GitHub] zookeeper issue #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, c...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/633 How can I do that? I tried a few things like git mv, but didn't work either. As far as I know, this is how git works unfortunately. But it did not loose the history, it's just that git is tracking content, not files. It is a matter of git client how it displays. git log will make it look like the history is lost, but using git log --follow displays the history fine. Unfortunately, github web interface does not have this feature. Do you know a workaround @hanm ? ---
[GitHub] zookeeper pull request #633: ZOOKEEPER-3032 - MAVEN MIGRATION - move java se...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/633 ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, client You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3032 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/633.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 #633 commit 828950b458eb6d9fa1edf1df6c819a971f9d4312 Author: Norbert Kalmar Date: 2018-09-19T14:16:36Z ZOOKEEPER-3032 - MAVEN MIGRATION - move java server, client ---
[GitHub] zookeeper issue #590: [ZOOKEEPER-1177] Add the memory optimized watch manage...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/590 systest will go to src/test/java , from my side, you can put the bench in org.apache.zookeeper.test.system . Thinking about it, that's a pretty good place. No need to create main directory or anything, I will move all the files anyway. Just move the files amongst the others in systest. (Hopefully no package level dependency, I didn't check) But there's a chance you will have to rebase if this this PR cannot be merged before the movement of all the remaining files as the directory refactor's last step. Sorry about that in advance... I'm not going to be the most popular with that PR :( ---
[GitHub] zookeeper pull request #616: ZOOKEEPER-3080_3.4: MAVEN MIGRATION - Step 1.5 ...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/616 ---
[GitHub] zookeeper pull request #617: ZOOKEEPER-1990 - fix Random instances
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/617 ZOOKEEPER-1990 - fix Random instances See the jira for more info. Basically we have multiple ways of creating Random instances in ZooKeeper. Since java 1.7, the default constructor is good enough even in multi-threaded environment, we get a good seed. But in some places, we just create a random instance, where System.nanotime is the seed, which is not a good practice in multi-threaded environments. I only replaced those, and I also left the tests as is, because in some cases it is intentional in them. I created the PR to bring more attention to the ticket, please feel free to share your ideas on the topic! You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-1990 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/617.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 #617 commit 4bbb8470ecd1445761c1091c49b9188e15e8c06c Author: Norbert Kalmar Date: 2018-09-03T13:49:26Z ZOOKEEPER-1990 - fix Random instances ---
[GitHub] zookeeper pull request #616: ZOOKEEPER-3080_3.4: MAVEN MIGRATION - Step 1.5 ...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/616 ZOOKEEPER-3080_3.4: MAVEN MIGRATION - Step 1.5 - move jute dir You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3080_3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/616.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 #616 commit fbcb63995769c9407a3744e145968032344a089d Author: Norbert Kalmar Date: 2018-09-03T13:24:15Z ZOOKEEPER-3080_3.4: MAVEN MIGRATION - Step 1.5 - move jute dir ---
[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/610 Looks like the C test failed, can you please re-run if Jenkins throws the same error? (just add an empty --amend commit). ---
[GitHub] zookeeper issue #610: [ZOOKEEPER-3124] Remove special logic to handle cversi...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/610 Oh, nice, thanks @lvfangmin ! I will take a look on both then. ---
[GitHub] zookeeper pull request #609: ZOOKEEPER-3080: MAVEN MIGRATION - Step 1.5 - mo...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/609 ZOOKEEPER-3080: MAVEN MIGRATION - Step 1.5 - move jute dir You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3080 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/609.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 #609 commit 56f5bd3b88a562ce5f8103e0ef92855bd0177664 Author: Norbert Kalmar Date: 2018-08-27T10:27:40Z ZOOKEEPER-3080: MAVEN MIGRATION - Step 1.5 - move jute dir ---
[GitHub] zookeeper pull request #603: ZOOKEEPER-3031 3.4: MAVEN MIGRATION - move clie...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/603 ---
[GitHub] zookeeper issue #603: ZOOKEEPER-3031 3.4: MAVEN MIGRATION - move client dir
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/603 I had the build re-run multiple times, still says core tests failed. But the test report says all test passed... ---
[GitHub] zookeeper pull request #602: ZOOKEEPER-3031 3.5: MAVEN MIGRATION - move clie...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/602 ---
[GitHub] zookeeper pull request #603: ZOOKEEPER-3031 3.4: MAVEN MIGRATION - move clie...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/603 ZOOKEEPER-3031 3.4: MAVEN MIGRATION - move client dir Separate PR for branch 3.5, code change based on 176bd68222f3a3bddac9deb412eee7f538cb01ca You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3031_3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/603.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 #603 commit de634cc71d26a1d1391caa5e0547209c4997451d Author: Norbert Kalmar Date: 2018-08-21T09:32:45Z ZOOKEEPER-3031 3.4: MAVEN MIGRATION - move client dir ---
[GitHub] zookeeper pull request #602: ZOOKEEPER-3031 3.5: MAVEN MIGRATION - move clie...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/602 ZOOKEEPER-3031 3.5: MAVEN MIGRATION - move client dir Separate PR for branch 3.5, cherry picked from 176bd68222f3a3bddac9deb412eee7f538cb01ca You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3031_3.5 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/602.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 #602 commit b4e0a7f4bff220b22ce75eed4311a6f1ad464e04 Author: Norbert Kalmar Date: 2018-08-21T05:30:53Z ZOOKEEPER-3031 3.5: MAVEN MIGRATION - move client dir ---
[GitHub] zookeeper pull request #600: ZOOKEEPER-3033 create maven structure for recip...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/600 ---
[GitHub] zookeeper pull request #600: ZOOKEEPER-3033 create maven structure for recip...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/600 ZOOKEEPER-3033 create maven structure for recipes Step 1.2 fix for 3.4 (create maven structure for recipes) You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3033_3.4_fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/600.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 #600 commit dbc030b609c193952bca5311571853f0f129f2b8 Author: Norbert Kalmar Date: 2018-08-17T10:15:03Z ZOOKEEPER-3033 create maven structure for recipes ---
[GitHub] zookeeper pull request #599: ZOOKEEPER-3033 - MAVEN MIGRATION - fix director...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/599 ZOOKEEPER-3033 - MAVEN MIGRATION - fix directory structure I reopened step 1.2 to fix the directory structure of the recipes, as it is better to do every directory change at once. (As it has been done in step 1.3 already - first step, docs is irrelevant in this manner). You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3033_fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/599.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 #599 ---
[GitHub] zookeeper pull request #597: ZOOKEEPER-3031: MAVEN MIGRATION - Step 1.4 - mo...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/597 ZOOKEEPER-3031: MAVEN MIGRATION - Step 1.4 - move client dir You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3031 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/597.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 #597 commit 828ce683ff03712f9d7a9589b198fbfb56d39b50 Author: Norbert Kalmar Date: 2018-08-14T08:17:00Z ZOOKEEPER-3031 - Maven Migration - move client dir ---
[GitHub] zookeeper pull request #589: ZOOKEEPER-3030 - MAVEN MIGRATION 3.4 - Step 1.3...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/589 ---
[GitHub] zookeeper pull request #589: ZOOKEEPER-3030 - MAVEN MIGRATION 3.4 - Step 1.3...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/589 ZOOKEEPER-3030 - MAVEN MIGRATION 3.4 - Step 1.3 - move contrib directories â¦ories You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3030_3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/589.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 #589 commit c8d742752153c8d9202deae24b817dcc657039dd Author: Norbert Kalmar Date: 2018-08-06T13:36:37Z ZOOKEEPER-3030 - MAVEN MIGRATION 3.4 - Step 1.3 - move contrib directories ---
[GitHub] zookeeper pull request #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - m...
GitHub user nkalmar reopened a pull request: https://github.com/apache/zookeeper/pull/574 ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - move contrib directories Move the contrib projects according to new directory structure. In the PR src/contrib/zkperl/build/check_zk_version.c and .h looks like I removed the file. but everything looks good on my local (I just moved them, did not modify anything). Will investigate further. DO-NOT-MERGE-YET: Checking if creating the maven structure of directories now will break anything other then the already different directory structure in build/zookeeper-[version]/src/ (I will write an e-mail about this on dev list) You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3030 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/574.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 #574 commit 165d4dc5d02ad37f2cb20c13681fd22efaf42fbc Author: Norbert Kalmar Date: 2018-07-17T10:08:16Z ZOOKEEPER-3030 - MAVEN MIGRATION - move contrib directories commit 59d9d17d9ddadd5bfe6e37b8c4458b6920aa32eb Author: Norbert Kalmar Date: 2018-07-20T14:28:24Z ZOOKEEPER-3030 add missing files in dist.dir commit 9f19ab06a73a3c75bacbe141f7fa469c4f9e4e60 Author: Norbert Kalmar Date: 2018-07-20T15:38:40Z ZOOKEEPER-3030 fix releaseaudit license missing exclusions commit 49b7519bed90a3bc2f87cfe39f77f9e6bcb13f98 Author: Norbert Kalmar Date: 2018-07-23T13:36:17Z ZOOKEEPER-3030 create maven dir structure ---
[GitHub] zookeeper pull request #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - m...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/574 ---
[GitHub] zookeeper issue #572: ZOOKEEPER-3085 define exit codes in enum
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/572 The build failed due to NetUtilsTest not having apache license. I did not create or made modification to that file. I checked master, it is currently in the same way, so master should fail with this warning as well. [rat:report] !? /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build/zookeeper-3.6.0-SNAPSHOT/src/java/test/org/apache/zookeeper/common/NetUtilsTest.java Lines that start with ? in the release audit report indicate files that do not have an Apache license header. ---
[GitHub] zookeeper pull request #:
Github user nkalmar commented on the pull request: https://github.com/apache/zookeeper/commit/a2623a625a4778720f7d5482d0a66e9b37ae556f#commitcomment-29934308 Both JMX and Jetty can be secured. The problem here is, as of my understanding, is that 4ltw command uses the client port. You can secure JMX port, introduce authentication, SSL etc. But you cannot secure the client port like that. So leaving the port open, and the ability to call functions without any authentication or authorization via telnet is not the best practice. By the way, JMX port should only be open on the local machine, as it is the default setting on ZooKeeper. But if you wan't to open it, it should be secured with firewall/gateway settings, IP restrictions, SASL or whatever. Jetty can be also configured for SSL. ---
[GitHub] zookeeper pull request #:
Github user nkalmar commented on the pull request: https://github.com/apache/zookeeper/commit/a2623a625a4778720f7d5482d0a66e9b37ae556f#commitcomment-29917129 @maoling , the problem is, there is no security implemented. Anyone user who can access ZooKeeper, can send commands to the ensemble. While all 4ltw commands is read-only, some takes quite some time, so a DOS attack is actually possible. So it has been deemed unsecure and deprecated, as far as I know. Originally I implemented the 4ltw command for this PR but the I was suggested to remove from 3.6 and 3.5 ---
[GitHub] zookeeper issue #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - move con...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/574 @anmolnar I did an amend commit, it doesn't seem to trigger the build. I also did a normal commit yesterday on my other PR, which also didn't trigger the build. Strange. Maybe I'll closing and re-opening the PR ---
[GitHub] zookeeper issue #572: ZOOKEEPER-3085 define exit codes in enum
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/572 @pravsingh , I changed the comments to javadoc comments. But after creating the tests, it was kind of strange. I looked at other enum in ZK, we don't check the values. I think we will see in a PR if someone wants to change the enum's value. (And if someone wants to change it, he might as easily change the tests too). Bottom line, I don't think it is necessary to have a unit test to check ExitCode values. What do others think? ---
[GitHub] zookeeper pull request #572: ZOOKEEPER-3085 define exit codes in enum
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/572#discussion_r206045238 --- Diff: src/java/main/org/apache/zookeeper/server/ExitCode.java --- @@ -20,8 +20,35 @@ /** * Exit code used to exit server */ -public class ExitCode { +public enum ExitCode { + +/* Execution finished normally */ +EXECUTION_FINISHED(0), +/* Unexpected errors like IO Exceptions */ +UNEXPECTED_ERROR(1), +/* Invalid arguments during invocations */ +INVALID_INVOCATION(2), +/* Cannot access datadir when trying to replicate server */ +UNABLE_TO_ACCESS_DATADIR(3), +/* Unable to start admin server at ZooKeeper startup */ +ERROR_STARTING_ADMIN_SERVER(4), +/* Severe error during snapshot IO */ +TXNLOG_ERROR_TAKING_SNAPSHOT(10), +/* zxid from COMMIT does not match the one from pendingTxns queue */ +UNMATCHED_TXN_COMMIT(12), +/* Unexpected packet from leader, or unable to truncate log on Leader.TRUNC */ +QUORUM_PACKET_ERROR(13), +/* Unable to bind to the quorum (election) port after multiple retry */ +UNABLE_TO_BIND_QUORUM_PORT(14); + +private final int value; + +ExitCode(final int newValue) { +value = newValue; +} + +public int getValue() { --- End diff -- No, I will write them. Thanks! ---
[GitHub] zookeeper pull request #:
Github user nkalmar commented on the pull request: https://github.com/apache/zookeeper/commit/a2623a625a4778720f7d5482d0a66e9b37ae556f#commitcomment-29880817 @maoling It has been intentionally removed from 3.6 and 3.5, as I was told the 4 letter words is deprecated, and it will be removed. No new metrics should be introduced. ---
[GitHub] zookeeper issue #583: [ZOOKEEPER-3104] Fix data inconsistency due to NEWLEAD...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/583 It's not a blocker for me, I agree it would be nice to have a unified format, but that's pretty hard to achieve on an Apache I think :( Anyway, that's why I wrote a "comment" review, not a request for change. Thanks for the fix @lvfangmin ! :) ---
[GitHub] zookeeper issue #581: ZOOKEEPER-3101: add reminder to ZOO_ERRORS
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/581 Yes, all PR fails due to the same reason. I think there is already an INFRA ticket but I'll double check. ---
[GitHub] zookeeper issue #581: ZOOKEEPER-3101: add reminder to ZOO_ERRORS
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/581 Looks to me jenkins was simply overloaded. You can re-run the jenkins job by doing an empty --amend commit and pushing it to your PR branch. ---
[GitHub] zookeeper issue #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - move con...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/574 So... The problem was that by moving directories from src, ant did not copy them to build/zookeeper-[version]/src/ - now I don't know why it is required here, as it won't be in the jars. I made a script to dif all the filenames in build directory and to diff all the jars (with jar tf output), and the jars do not differ, but the files do. For example, used to be: build/zookeeper-3.6.0-SNAPSHOT/src/recipes/lock/build.xml now: build/zookeeper-3.6.0-SNAPSHOT/src/zookeeper-recipes/zookeeper-recipes-lock/build.xml Again the jar's did not change as packages remained the same! ---
[GitHub] zookeeper issue #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - move con...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/574 The problem with that is that it requires a lot of ant change and the final artifact could change. I agree @tamaashu , that was the plan. I will try to do this with the contrib directory (this PR), and carefully check the end artifacts. ---
[GitHub] zookeeper pull request #572: ZOOKEEPER-3085 define exit codes in enum
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/572#discussion_r203643727 --- Diff: src/java/main/org/apache/zookeeper/server/ExitCode.java --- @@ -20,8 +20,35 @@ /** * Exit code used to exit server */ -public class ExitCode { +public enum ExitCode { + +/* Execution finished normally */ +EXECUTION_FINISHED(0), +/* Unexpected errors like IO Exceptions */ +UNEXPECTED_ERROR(1), +/* Invalid arguments during invocations */ +INVALID_INVOCATION(2), +/* Cannot access datadir when trying to replicate server */ +UNABLE_TO_ACCESS_DATADIR(3), +/* Unable to start admin server at ZooKeeper startup */ +ERROR_STARTING_ADMIN_SERVER(4), +/* Severe error during snapshot IO */ +TXNLOG_ERROR_TAKING_SNAPSHOT(10), +/* zxid from COMMIT does not match the one from pendingTxns queue */ +INVALID_TXN_COMMIT(12), --- End diff -- Thanks for the suggestions @lvfangmin , I will commit them soon. ---
[GitHub] zookeeper issue #572: ZOOKEEPER-3085 define exit codes in enum
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/572 Yes, I missed those, thanks @maoling ! (By the way, isn't there a reply option for the comments? Or I'm just blind? :) ) ---
[GitHub] zookeeper issue #574: ZOOKEEPER-3030 - MAVEN MIGRATION - Step 1.3 - move con...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/574 Found the problem, fixed it with a quick amend commit. The problem was in .gitignore we have build/ and that matched the new directory, and I had to explicitly add them. ---
[GitHub] zookeeper pull request #574: ZOOKEEPER-3030 - MAVEN MIGRATION - move contrib...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/574 ZOOKEEPER-3030 - MAVEN MIGRATION - move contrib directories Move the contrib projects according to new directory structure. In the PR src/contrib/zkperl/build/check_zk_version.c and .h looks like I removed the file. but everything looks good on my local (I just moved them, did not modify anything). Will investigate further. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3030 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/574.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 #574 commit b46f7bfa910c6d018e09ec2122ec8712571b2e67 Author: Norbert Kalmar Date: 2018-07-17T10:08:16Z ZOOKEEPER-3030 - MAVEN MIGRATION - move contrib directories ---
[GitHub] zookeeper pull request #569: ZOOKEEPER-3033 branch-3.4 - MAVEN MIGRATION - S...
Github user nkalmar closed the pull request at: https://github.com/apache/zookeeper/pull/569 ---
[GitHub] zookeeper pull request #572: ZOOKEEPER-3085 define exit codes in enum
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/572 ZOOKEEPER-3085 define exit codes in enum Define the used exit codes in one enum. contrib projects are excluded, as they can have seperate exit codes (and they do use it differently then in core ZK). Some exit codes overlap, but refactoring the exit code used is not backward compatible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3085 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/572.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 #572 commit d321f44f82c637681bfff28c2de4b6720acd5015 Author: Norbert Kalmar Date: 2018-07-15T18:13:01Z ZOOKEEPER-3085 define exit codes in enum ---
[GitHub] zookeeper pull request #569: ZOOKEEPER-3033 branch-3.4 - MAVEN MIGRATION - S...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/569 ZOOKEEPER-3033 branch-3.4 - MAVEN MIGRATION - Step 1.2 - zk-recipes This is the structure for zk-recipes. (Already in 3.5 and 3.6) You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3033-3.4 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/569.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 #569 commit c399e14a04fe4422927003c787edd32c3dbe4df6 Author: Norbert Kalmar Date: 2018-07-14T17:42:30Z ZOOKEEPER-3033 3.4 - MAVEN MIGRATION - Step 1.2 - create zk-recipes maven structure ---
[GitHub] zookeeper pull request #563: Fix for ZOOKEEPER-3072
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/563#discussion_r201258688 --- Diff: src/java/test/org/apache/zookeeper/test/ThrottleRaceTest.java --- @@ -0,0 +1,156 @@ +/** + * 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.test; + +import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT; +import static org.apache.zookeeper.test.ClientBase.verifyThreadTerminated; + +import org.apache.zookeeper.AsyncCallback.StatCallback; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZKTestCase; +import org.apache.zookeeper.TestableZooKeeper; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.data.Stat; +import org.apache.zookeeper.test.ClientBase.CountdownWatcher; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ThrottleRaceTest extends ZKTestCase { +private static final Logger LOG = LoggerFactory.getLogger(ThrottleRaceTest.class); + +private QuorumBase qb = new QuorumBase(); + +private volatile boolean bang; + +public void setUp() throws Exception { +qb.setUp(); +} + +public void tearDown() throws Exception { +LOG.info("Test clients shutting down"); +qb.tearDown(); +} + +/** + * Send exists /zookeeper requests asynchronously, max 30 outstanding + */ +class HammerThreadExists extends Thread implements StatCallback { +private static final int MAX_OUTSTANDING = 30; + +private TestableZooKeeper zk; +private int outstanding; + +private volatile boolean failed = false; + +public HammerThreadExists(String name) { +super(name); +} + +public void run() { +try { +CountdownWatcher watcher = new CountdownWatcher(); +zk = new TestableZooKeeper(qb.hostPort, CONNECTION_TIMEOUT, +watcher); +watcher.waitForConnected(CONNECTION_TIMEOUT); +while(bang) { +incOutstanding(); // before create otw race +zk.exists("/zookeeper", false, this, null); +} +} catch (InterruptedException e) { +if (bang) { +LOG.error("sanity check Assert.failed!!!"); // sanity check +return; +} +} catch (Exception e) { +LOG.error("Client create operation Assert.failed", e); +return; +} finally { +if (zk != null) { +try { +if (!zk.close(CONNECTION_TIMEOUT)) { +failed = true; +LOG.error("Client did not shutdown"); +} +} catch (InterruptedException e) { +LOG.info("Interrupted", e); +} +} +} +} + +private synchronized void incOutstanding() throws InterruptedException { +outstanding++; +while(outstanding > MAX_OUTSTANDING) { +wait(); +} +} + +private synchronized void decOutstanding() { +outstanding--; +Assert.assertTrue("outstanding >= 0", outstanding >= 0); +notifyAll(); +} + +public void process(WatchedEvent event) { +// ignore for purposes of this test +} + +
[GitHub] zookeeper issue #564: ZOOKEEPER-3033 - MAVEN MIGRATION - Step 1.2 - create z...
Github user nkalmar commented on the issue: https://github.com/apache/zookeeper/pull/564 I did, thanks @tamaashu ! I will make the changes. ---
[GitHub] zookeeper pull request #564: ZOOKEEPER-3033 create zk-recipes maven structur...
GitHub user nkalmar opened a pull request: https://github.com/apache/zookeeper/pull/564 ZOOKEEPER-3033 create zk-recipes maven structure Create the zk-recipes maven structure for the maven migration. Tested the recipes build with the new directory structure. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nkalmar/zookeeper ZOOKEEPER-3033 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/564.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 #564 commit aad023111b36c1c2628f8043de513871efa12665 Author: Norbert Kalmar Date: 2018-07-09T13:01:18Z ZOOKEEPER-3033 create zk-recipes maven structure ---
[GitHub] zookeeper pull request #559: ZOOKEEPER-3079: avoid unsafe use of sprintf(3)
Github user nkalmar commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/559#discussion_r200190816 --- Diff: src/c/src/zookeeper.c --- @@ -4357,7 +4357,7 @@ int zoo_add_auth(zhandle_t *zh,const char* scheme,const char* cert, static const char* format_endpoint_info(const struct sockaddr_storage* ep) { static char buf[128] = { 0 }; -char addrstr[128] = { 0 }; +char addrstr[INET6_ADDRSTRLEN] = { 0 }; --- End diff -- OK, thanks for clearing that up. Well, you were clear now that I understand the issue - probably the problem was that I'm not much of a C developer :) ---