Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-3570:make the special client xid constant #1136

wants to merge 1 commit into from


Copy link

maoling commented Nov 7, 2019

Copy link

lvfangmin left a comment


Thanks for cleaning this up.

@maoling maoling force-pushed the maoling:ZOOKEEPER-3570 branch from 7211c92 to 708fe76 Nov 15, 2019
enixon approved these changes Nov 18, 2019
Copy link

enixon left a comment


@asfgit asfgit closed this in ae68c7d Nov 21, 2019
junyoungKimGit added a commit to junyoungKimGit/zookeeper that referenced this pull request Dec 9, 2019
* ZOOKEEPER-3530: add new artifact for compiled c-client code

During the old ZooKeeper 3.4 ant builds (ant package-native), there was an artifact (zookeeper-<version>-lib.tar.gz) created just for the C-client, with the following content:

  |--- bin
         |--- cli_mt
         |--- cli_st
         |--- load_gen
  |--- include
         |--- zookeeper
                |--- proto.h
                |--- recordio.h
                |--- zookeeper.h
                |--- zookeeper.jute.h
                |--- zookeeper_log.h
                |--- zookeeper_version.h
  |--- lib
         |--- libzookeeper_mt.a
         |--- libzookeeper_st.a

Currently with maven, when we are generating a tarball during full-build then the C-client is not getting archived. In [PR-1078](apache#1078) we discussed that we should re-introduce the apache-zookeeper-<version>-lib.tar.gz artifact.

The goals of this PR are:
- re-introduce the 'lib' artifact, with the same structure we had for the older zookeeper 3.4.x ant generated tar file
- we should also add the LICENSE.txt file to the archive (it was missing from the 3.4.x version tar.gz file)
- the new artifact should be generated only when the full-build profile is set for maven
- we should also update the file

Author: Mate Szalay-Beko <>


Closes apache#1113 from symat/ZOOKEEPER-3530-PR

* ZOOKEEPER-3593: fix the default value of jute.maxbuffer in client side and an optimization for the documentation

- The default value of `jute.maxbuffer` in client side said it's 4MB, but actually, it was never working, because when the client reads the deserialized znode data, it also calls `checkLength(` where  `jute.maxbuffer` default value is 1MB. It's easy to reproduce, just read a znode more than 1MB with any special configure client. Look at the stack trace I attached in the JIRA
- Users also confused about that the doc said `jute.maxbuffer` must be set on all servers and clients, but their default value is not same in the [ZOOKEEPER-1295](
- more details in the [ZOOKEEPER-3593](

Author: maoling <>


Closes apache#1129 from maoling/ZOOKEEPER-3593

* ZOOKEEPER-3605: connThrottle needs to be assigned in alternate consructor

`connThrottle` needs to be assigned in alternate consructor to avoid NPEs

Author: randgalt <>

Reviewers: Enrico Olivelli, Andor Molnár

Closes apache#1132 from Randgalt/ZOOKEEPER-3605

* ZOOKEEPER-1416 - Persistent, recursive watchers

### Background

Note: this is a port of apache#136

Implementation for a persistent, recursive watch addition for ZK. These watches are set via a new method, addPersistentWatch() and are removed via the existing watcher removal methods. Persistent, recursive watches have these characteristics: a) Once set, they do not auto-remove when triggered; b) they trigger for all event types (child, data, etc.) on the node they are registered for and any child znode recursively; c) they are efficiently implemented by using the existing watch internals. A new class PathIterator walks up the path parent-by-parent when checking if a watcher applies.

### Implementation Details

- A new enum manages the different "modes" for watchers: `WatcherMode`.
- For traditional, "standard" watchers, the code path is almost exactly the same. There is very little overhead other than a bit of extra checks in `WatchManager`
- Given how this is implemented it was difficult to add support when `WatchManagerOptimized` is used. I'm open to adding it for that version but it will take work. We should consider not supporting persistent/recursive watchers when WatchManagerOptimized is used. I notice that `WatchManagerOptimized` is not even mentioned in the docs.
- The mode for a given watcher/path pair is held in a map inside of `WatcherModeManager`. The absence of an entry means Standard. This way, there's no overhead for old, standard watchers.
- `PathParentIterator` is the "meat" of the implementation. Rather than set watchers on every ZNode implied by a recursive watcher. WatchManager passes any paths it processes through PathParentIterator which iterates up each parent znode looking for watchers.
- The remainder of the changes are scaffolding to match how other watchers are used as well as Jute/API changes to set persistent/recursive watchers

### Testing

The tests were written years ago. I think they're comprehensive but reviewers should pay attention to anything that was missed. There is much ZooKeeper knowledge that's only in the heads of ZK committers.

- `PersistentWatcherTest` - tests persistent, non-recursive watchers
- `PersistentRecursiveWatcherTest` - tests persistent, recursive watchers
- `PathParentIteratorTest`- exercises edges of PathParentIterator

Author: randgalt <>

Reviewers: Enrico Olivelli <eolivelli@apache,org>, Norbert Kalmar <>, Andor Molnár <>, Justin Mao Ling <>

Closes apache#1106 from Randgalt/ZOOKEEPER-1416

* ZOOKEEPER-3598: Fix potential data inconsistency issue due to CommitProcessor not gracefully shutdown

Note: use exit code 16 for SHUTDOWN_UNGRACEFULLY, since internally we've already using 15 for other exit code, which will be upstreamed later.

Author: Fangmin Lyu <>

Reviewers: Enrico Olivelli <>, Michael Han <>

Closes apache#1130 from lvfangmin/ZOOKEEPER-3598

* ZOOKEEPER-1260: Audit logging in ZooKeeper servers.

Author: Mohammad Arshad <>

Reviewers: Enrico Olivelli <>, Andor Molnar <>

Closes apache#1133 from arshadmohammad/ZOOKEEPER-1260-AuditLog-master

* ZOOKEEPER-3340: Introduce CircularBlockingQueue in

There are many caveats and comments regarding Exception thrown 'which is safe to ignore'.  Added a new data structure that removes all of these comments and will perform without generating exceptions while more clearly implementing the desired behavior without the caveats.

Author: Beluga Behr <>
Author: David Mollitor <>
Author: David Mollitor <>


Closes apache#880 from belugabehr/ZOOKEEPER-3340 and squashes the following commits:

7b74dac [David Mollitor] Changed comment from conrete ArrayBlockingQueue to BlockingQueue
38d7e3f [David Mollitor] Shutdown ExecutorService in test
f5ea2b6 [David Mollitor] Updated to fix checkstyle ImportOrder: Extra separation
b3ac3cc [David Mollitor] Rebased to master
c63ab85 [Beluga Behr] Added updates from code review
d887780 [Beluga Behr] Introduce new class CircularBlockingQueue
2793db2 [Beluga Behr] Added additional logging
ce96b70 [Beluga Behr] ZOOKEEPER-3340: Improve Queue Usage in

* ZOOKEEPER-2238: Support limiting the maximum number of connections/clients to a zookeeper server

Support limiting the maximum number of connections/clients to a zookeeper server

Author: sujithsimon22 <>

Reviewers: Justin Mao Ling <>, Brian Nixon <>, Edward Ribeiro <>, Enrico Olivelli <>, Mohammad Arshad <>

Closes apache#1108 from sujithsimon22/ZOOKEEPER-2238

* ZOOKEEPER-3571: Ensure test base directory before tests

Ensure `build.test.dir` is present as a directory. Otherwise many times it suffers from

``` No such file or directory

	at Method)
	at org.apache.zookeeper.test.ClientBase.createTmpDir(
	at org.apache.zookeeper.test.ClientBase.setUpWithServerId(
	at org.apache.zookeeper.test.ClientBase.setUp(

Author: tison <>


Closes apache#1112 from TisonKun/ZOOKEEPER-3571

* Revert "ZOOKEEPER-3598: Fix potential data inconsistency issue due to CommitProcessor not gracefully shutdown"

This reverts commit 79f99af.

* ZOOKEEPER-3363: Drop ANT build.xml

- drop Ant and Ivy files
- drop old Cobertura README file
- drop old jdiff file

Author: Enrico Olivelli <>
Author: Enrico Olivelli <>

Reviewers: Norbert Kalmar <>

Closes apache#1139 from eolivelli/fix/drop-ant

* ZOOKEEPER-3560: Add response cache to serve get children (2) requests.

ZOOKEEPER-3180 introduces response cache but it only covers getData requests. This commit is to extend the response cache based on the infrastructure set up by ZOOKEEPER-3180 to so the response of get children requests can also be served out of cache. Some design decisions:

* Only OpCode.getChildren2 is supported, as OpCode.getChildren does not have associated stats and current cache infra relies on stats to invalidate cache.

* The children list is stored in a separate response cache object so it does not pollute the existing data cache that's serving getData requests, and this separation also allows potential separate tuning of each cache based on workload characteristics.

* As a result of cache object separation, new server metrics is added to measure cache hit / miss for get children requests, that's separated from get data requests.

Similar as ZOOKEEPER-3180, the get children response cache is enabled by default, with a default cache size of 400, and can be disabled (together with get data response cache.).

Author: Michael Han <>
Author: Michael Han <>


Closes apache#1098 from hanm/cache

* ZOOKEEPER-3502: improve the server command: zabstate to have a better observation on the process of leader election

- Just as we knew, the rule of LE is that who has the biggest last zxid who will win.when the zxid is the same, chose the myid number bigger one.
- Do the following cmd will have a bird's-eye view on LE
 ➜  bin curl http://localhost:8081/commands/zabstate  http://localhost:8082/commands/zabstate http://localhost:8083/commands/zabstate
  "myid" : 1,
  "is_leader" : true,
  "endpoint" : "/",
  "voting" : true,
  "last_zxid" : "0xfa2c00000000",
  "zab_epoch" : 64044,
  "zab_counter" : 0,
  "zabstate" : "broadcast",
  "command" : "zabstate",
  "error" : null
  "myid" : 2,
  "is_leader" : false,
  "endpoint" : "/",
  "voting" : true,
  "last_zxid" : "0xfa2b00000004",
  "zab_epoch" : 64043,
  "zab_counter" : 4,
  "zabstate" : "broadcast",
  "command" : "zabstate",
  "error" : null
  "myid" : 3,
  "is_leader" : false,
  "endpoint" : "/",
  "voting" : true,
  "last_zxid" : "0xfa2b00000004",
  "zab_epoch" : 64043,
  "zab_counter" : 4,
  "zabstate" : "broadcast",
  "command" : "zabstate",
  "error" : null
- more details in the [ZOOKEEPER-3502](

Author: maoling <>

Reviewers: Brian Nixon <>, Andor Molnar <>, Enrico Olivelli <>

Closes apache#1052 from maoling/ZOOKEEPER-3502

* ZOOKEEPER-3570: make the special client xid constant

- more details in the [ZOOKEEPER-3570](

Author: maoling <>

Reviewers: Fangmin Lyu <>, Brian Nixon <>

Closes apache#1136 from maoling/ZOOKEEPER-3570

* ZOOKEEPER-2122: add SSL support for C-client

This PR is based on the works of Asnish Amarnath and Suhas Dantkale. Most of the kudos should go to them and those who were reviewing all the previous PRs.

**The PR includes the following changes from PR#639:**
- OPENSSL 1.1.1 version support in C-client

**The PR includes the following changes from PR#990:**
- also supporting OPENSSL 1.0.2
- SSL connection on non-blocking socket is handled correctly
- Support of Certificate Chains
- Fix Memory leaks
- Dynamically generated test certificates

**The following new changes were added into the PR:**
- fix CMake + VisualStudio2019 build with windows
- fix C CLI to compile / work both with windows and linux (I tested them manually)
- fix (and simplify) the way how the server is started with C unit tests, so it is compatible with maven build now
- the test case `testReadOnly` was failing with the previous PR because there was a bug in the C-client code, I fixed that
- I also added new test case: `testReadOnlyWithSSL`

**Testing this PR on linux:**
git clean -xdf

# compile ZooKeeper server plus the C-client code
mvn clean install -DskipTests -Pfull-build

# compile and execute C-client unit tests
cd zookeeper-client/
mvn clean install -Pfull-build

**Compile the code on windows (only cmake is supported):**
- download C-Make:
- Install community edition of Visual Studio:
- Download OpenSSL (e.g. 1.0.2): (e.g. install it to `c:\OpenSSL-Win64`)
- compile the java code using: `mvn clean install -DskipTests`
- go to the Client folder: `cd zookeeper-client\zookeeper-client-c`
- configure the project:  `cmake . -D WITH_OPENSSL=c:\OpenSSL-Win64`
- build the project: `cmake --build .`

**Testing the C-client with SSL manually:**
- run the `zookeeper-client/zookeeper-client-c/ssl/` to generate certificate files (e.g. copy it to an empty folder like `/tmp/ssl/` and start is)
- start a ZooKeeper server, using some config file like this one:

- start the command line client (cli.exe on windows, cli_mt or cli_st on linux): `./cli_mt --host localhost:22281 --ssl /tmp/ssl/server.crt,/tmp/ssl/client.crt,/tmp/ssl/clientkey.pem,password`

Author: Mate Szalay-Beko <>
Author: Mate Szalay-Beko <>


Closes apache#1107 from symat/ZOOKEEPER-2122 and squashes the following commits:

08294ce [Mate Szalay-Beko] ZOOKEEPER-2122: update readme + use FQDN in SSL certs during testing
17e504a [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-2122
317241d [Mate Szalay-Beko] ZOOKEEPER-2122: minor fix in SSL certificates used for testing
6f37b66 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into HEAD
9809143 [Mate Szalay-Beko] ZOOKEEPER-2122: add SSL support for C-client

* ZOOKEEPER-3590: Zookeeper is unable to set the zookeeper.sasl.client.canonicalize.hostname using system property


This is a very small patch that gives sets the option after reading it from the system.getProperty().

There should not be any side effects since this is mostly copy paste.

Author: aristotelhs <>


Closes apache#1124 from aristotelhs/ZOOKEEPER-3590

* ZOOKEEPER-3627: Update Jackson to

Author: Colm O hEigeartaigh <>


Closes apache#1150 from coheigea/ZOOKEEPER-3627

* ZOOKEEPER-3473: Improving successful TLS handshake throughput with concurrent control

When there are lots of clients trying to re-establish sessions, there might be lots of half finished handshake timed out, and those failed ones keep reconnecting to another server and restarting the handshake from beginning again, which caused herd effect.

And the number of total ZK sessions could be supported within session timeout are impacted a lot after enabling TLS.

To improve the throughput, we added the TLS concurrent control to reduce the herd effect, and from out benchmark this doubled the sessions we could support within session timeout.

E2E test result:

Tested performance and correctness from E2E. For correctness, tested both secure and insecure
connections, the outstandingHandshakeNum will go to 0 eventually.

For performance, tested with 110k sessions with 10s session timeout, there is no session expire when leader election triggered, while before it can only support 50k sessions.

Author: Fangmin Lyu <>

Reviewers: Enrico Olivelli <>, Andor Molnar <>

Closes apache#1027 from lvfangmin/ZOOKEEPER-3473

* ZOOKEEPER-3546 - Allow optional deletion of never used Container Nodes

Edge cases can cause Container Nodes to never be deleted. i.e. if the container node is created and then the client that create the node crashes the container will not get deleted unless another client creates a node inside of it. This is because the initial implementation does not delete container nodes with a cversion of 0. This PR adds a new system property, "znode.container.maxNeverUsedIntervalMs", that can be set to delete containers with a cversion of 0 that have been retained for a period of time. This is a backward compatible change as the default value for this is Long.MAX_VALUE - i.e. never.

Author: randgalt <>

Reviewers: Enrico Olivelli <>, Fangmin Lyu <>

Closes apache#1138 from Randgalt/ZOOKEEPER-3546-allow-delete-of-never-used-containers

* ZOOKEEPER-3620: Allow to override calls to System.exit in server side code

- Introduce a way to override calls to System.exit
- enable DM_EXIT spotbugs rule

see for more context.

Author: Enrico Olivelli <>
Author: Enrico Olivelli <>


Closes apache#1147 from eolivelli/fix/ZOOKEEPER-3620-no-systemexit and squashes the following commits:

a234f85 [Enrico Olivelli] Fix checkstyle
4c4fec4 [Enrico Olivelli] Fix spotbugs warning
ae339b7 [Enrico Olivelli] Revert changes to
0e5ee07 [Enrico Olivelli] Enable DM_EXIT spotbugs rule for the full code base
b05a4bf [Enrico Olivelli] ZOOKEEPER-3620 Allow to override calls to System.exit in server side code: - Use a common utility to call System.exit - Override calls to System.exit to a NO-OP function in tests

* ZOOKEEPER-3188: Improve resilience to network

This PR is the rebase of the [previous pull request](apache#730), so all the kudos should go to the original authors...

In [ZOOKEEPER-3188]( we add ability to specify several addresses for quorum operations. Also added reconnection attempts if connection to leader lost.

In this PR I rebased the changes on the current master, resolving some minor conflicts with:
- [ZOOKEEPER-3296]( Explicitly closing the sslsocket when it failed handshake to prevent issue where peers cannot join quorum
- [ZOOKEEPER-3320]( Leader election port stop listen when hostname unresolvable for some time
- [ZOOKEEPER-3385]( Add admin command to display leader
- [ZOOKEEPER-3386]( Add admin command to display voting view
- [ZOOKEEPER-3398]( Learner.connectToLeader() may take too long to time-out

I still want to test the feature manually (e.g. using docker containers with multiple virtual networks / interfaces). The steps to the manual test could be recorded in the [google docs]( as well.

Also I think we could add a few more unit tests where we are using multiple addresses. The current tests are using a single address only.

Also the Zookeeper documentation needs to be changed (e.g. by a follow-up Jira?) to promote the new feature and the new config format (possibly including also the admin command documentation in relation with [ZOOKEEPER-3386]( and [ZOOKEEPER-3461](

Author: Mate Szalay-Beko <>
Author: Mate Szalay-Beko <>


Closes apache#1048 from symat/ZOOKEEPER-3188 and squashes the following commits:

3c6fc52 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
356882d [Mate Szalay-Beko] ZOOKEEPER-3188: document new configuration format for using multiple addresses
45b6c0f [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
4b6bcea [Mate Szalay-Beko] ZOOKEEPER-3188: MultiAddress unit tests for Quorum TLS and Kerberos/Digest authentication
40bc44c [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
f875f5c [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
31805e7 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
0f95678 [Mate Szalay-Beko] ZOOKEEPER-3188: skip unreachable addresses when Learner connects to Leader
e232c55 [Mate Szalay-Beko] ZOOKEEPER-3188: fix flaky unit MultiAddress unit test
e892d8d [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
6f2ab75 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
2eedf26 [Mate Szalay-Beko] ZOOKEEPER-3188: fix PR commits; handle case when Leader can not bind to port on startup
483d2fc [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
a5d6bcb [Mate Szalay-Beko] ZOOKEEPER-3188: support for dynamic reconfig + add more unit tests
ed31d2c [Mate Szalay-Beko] ZOOKEEPER-3188: better shutdown for executors (following PR comments)
8713a5b [Mate Szalay-Beko] ZOOKEEPER-3188: add fixes for PR comments
05eae83 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into ZOOKEEPER-3188
e823af4 [Mate Szalay-Beko] Merge remote-tracking branch 'origin/master' into ZOOKEEPER-3188
de7bad2 [Mate Szalay-Beko] Merge remote-tracking branch 'origin/master' into ZOOKEEPER-3188
da98a8d [Mate Szalay-Beko] ZOOKEEPER-3188: fix JDK-13 warning
5bd1f4e [Mate Szalay-Beko] ZOOKEEPER-3188: supress spotbugs warning
42a52a6 [Mate Szalay-Beko] ZOOKEEPER-3188: improve based on code review comments
6c4220a [Mate Szalay-Beko] ZOOKEEPER-3188: fix SendWorker.asyncValidateIfSocketIsStillReachable
5b22432 [Mate Szalay-Beko] ZOOKEEPER-3188: fix LeaderElection to work with multiple election addresses
7bfbe7e [Mate Szalay-Beko] ZOOKEEPER-3188: Improve resilience to network

* ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used

When only secureClientPort is defined in the config and there is no regular clientPort,
then both the stat and the conf commands on the AdminServer result in 500 Server Error caused by
NullPointerExceptions. The problem is that no serverCnxFactory is defined in the
ZooKeeperServer in this case, we have only secureServerCnxnFactory.

In the fix we return info about both the secure and unsecure connections.
Example of the stat command output for secure-only configuration:
  "version" : "3.6.0-SNAPSHOT-8e8905069f4bff670c0492fe9e28ced0f86bca00, built on 11/29/2019 08:04 GMT",
  "read_only" : false,
  "server_stats" : {
    "packets_sent" : 1,
    "packets_received" : 1,
    "fsync_threshold_exceed_count" : 0,
    "client_response_stats" : {
      "last_buffer_size" : -1,
      "min_buffer_size" : -1,
      "max_buffer_size" : -1
    "data_dir_size" : 671094270,
    "log_dir_size" : 671094270,
    "last_processed_zxid" : 20,
    "outstanding_requests" : 0,
    "server_state" : "standalone",
    "avg_latency" : 5.0,
    "max_latency" : 5,
    "min_latency" : 5,
    "num_alive_client_connections" : 1,
    "provider_null" : false,
    "uptime" : 15020
  "client_response" : {
    "last_buffer_size" : -1,
    "min_buffer_size" : -1,
    "max_buffer_size" : -1
  "node_count" : 6,
  "connections" : [ ],
  "secure_connections" : [ {
    "remote_socket_address" : "",
    "interest_ops" : 1,
    "outstanding_requests" : 0,
    "packets_received" : 1,
    "packets_sent" : 1
  } ],
  "command" : "stats",
  "error" : null

Author: Mate Szalay-Beko <>

Reviewers: Andor Molnar <>, Enrico Olivelli <>, Norbert Kalmar <>

Closes apache#1161 from symat/ZOOKEEPER-3633

* ZOOKEEPER-3595: restore the handling of the fsync parameter

Author: Jingguo Yao <>


Closes apache#1146 from yaojingguo/ZOOKEEPER-3595

* ZOOKEEPER-3546: fix missed change, default should be 0 not Long.MAX_VALUE

fix missed change, default should be 0 not Long.MAX_VALUE

Author: randgalt <>


Closes apache#1158 from Randgalt/ZOOKEEPER-3546-allow-delete-of-never-used-containers-fixit

* ZOOKEEPER-3635: Use Docker and Maven Release Plugin to prepare ZooKeeper releases

- configure maven-release-plugin
- configure maven-scm-plugin
- use maven-antrun-plugin in order to alter C client files during release procedure
- update Apache Parent pom to 21
- use openjdk8 for building releases

Author: Enrico Olivelli <>
Author: Enrico Olivelli <>
Author: Enrico Olivelli <>

Reviewers: Norbert Kalmar <>

Closes apache#1162 from eolivelli/fix/ZOOKEEPER-3635-new-release-proc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
3 participants
You can’t perform that action at this time.