Skip to content

ZOOKEEPER-233: Split zookeeper module to provide separated zookeeper-client, zookeeper-server #2307

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

ZOOKEEPER-233: Split zookeeper module to provide separated zookeeper-client, zookeeper-server #2307

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c247522
ZOOKEEPER-4966: Decouple `ZKConfig` from `QuorumPeerConfig.ConfigExce…
kezhuw Aug 20, 2025
cc4c376
ZOOKEEPER-233: Split `zookeeper-client` for `zookeeper`
kezhuw Aug 20, 2025
8d011b0
ZOOKEEPER-233: Split `zookeeper-common`, `zookeeper-server` from `zoo…
kezhuw Aug 21, 2025
da36e01
ZOOKEEPER-233: Split `zookeeper-cli` from `zookeeper`
kezhuw Aug 21, 2025
8e7eb53
zookeeper-cient-c belongs to full-build profile
kezhuw Sep 10, 2025
e690829
Include `zookeeper-client`, `zookeeper-server` and `zookeeper-cli` ex…
kezhuw Sep 10, 2025
0c53d33
Introduce `package-info.java` in module `zookeeper` for `maven-bundle…
kezhuw Sep 10, 2025
6120e83
Include `zookeeper-common` explictly also in `zookeeper-assembly`
kezhuw Sep 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 33 additions & 29 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,35 +69,39 @@ tags
obj
zookeeper-server/src/main/resources/lib/ant-eclipse-*
zookeeper-server/src/main/resources/lib/ivy-*
zookeeper-client/zookeeper-client-c/Makefile.in
zookeeper-client/zookeeper-client-c/aclocal.m4
zookeeper-client/zookeeper-client-c/autom4te.cache/
zookeeper-client/zookeeper-client-c/compile
zookeeper-client/zookeeper-client-c/config.guess
zookeeper-client/zookeeper-client-c/config.h.in
zookeeper-client/zookeeper-client-c/config.sub
zookeeper-client/zookeeper-client-c/configure
zookeeper-client/zookeeper-client-c/depcomp
zookeeper-client/zookeeper-client-c/install-sh
zookeeper-client/zookeeper-client-c/ltmain.sh
zookeeper-client/zookeeper-client-c/missing
zookeeper-client/zookeeper-client-c/.deps/
zookeeper-client/zookeeper-client-c/.libs/
zookeeper-client/zookeeper-client-c/Makefile
zookeeper-client/zookeeper-client-c/cli_mt
zookeeper-client/zookeeper-client-c/cli_st
zookeeper-client/zookeeper-client-c/config.h
zookeeper-client/zookeeper-client-c/config.status
zookeeper-client/zookeeper-client-c/libtool
zookeeper-client/zookeeper-client-c/load_gen
zookeeper-client/zookeeper-client-c/stamp-h1
zookeeper-client/zookeeper-client-c/build
zookeeper-client/zookeeper-client-c/core.*
zookeeper-client/zookeeper-client-c/TEST-*.txt
zookeeper-client/zookeeper-client-c/*.la
zookeeper-client/zookeeper-client-c/*.lo
zookeeper-client/zookeeper-client-c/*.o
zookeeper-client/zookeeper-client-c/generated/

zookeeper-client-c/Makefile.in
zookeeper-client-c/aclocal.m4
zookeeper-client-c/autom4te.cache/
zookeeper-client-c/compile
zookeeper-client-c/config.guess
zookeeper-client-c/config.h.in
zookeeper-client-c/config.sub
zookeeper-client-c/configure
zookeeper-client-c/depcomp
zookeeper-client-c/install-sh
zookeeper-client-c/ltmain.sh
zookeeper-client-c/missing
zookeeper-client-c/.deps/
zookeeper-client-c/.libs/
zookeeper-client-c/Makefile
zookeeper-client-c/cli_mt
zookeeper-client-c/cli_st
zookeeper-client-c/config.h
zookeeper-client-c/config.status
zookeeper-client-c/libtool
zookeeper-client-c/load_gen
zookeeper-client-c/stamp-h1
zookeeper-client-c/build
zookeeper-client-c/core.*
zookeeper-client-c/TEST-*.txt
zookeeper-client-c/*.la
zookeeper-client-c/*.lo
zookeeper-client-c/*.o
zookeeper-client-c/generated/

# Old c client files
zookeeper-client/zookeeper-client-c

# Python
*.py[cod]
2 changes: 1 addition & 1 deletion .typos.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,5 @@

[files]
extend-exclude = [
"zookeeper-server/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java",
"zookeeper-common/src/test/java/org/apache/zookeeper/common/CertificatesToPlayWith.java",
]
4 changes: 3 additions & 1 deletion bin/zkEnv.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ for d in "$ZOOBINDIR"/../build/lib/*.jar; do
done

#make it work for developers
for d in "$ZOOBINDIR"/../zookeeper-server/target/lib/*.jar; do
for d in "$ZOOBINDIR"/../zookeeper/target/lib/*.jar; do
CLASSPATH="$d:$CLASSPATH"
done

Expand All @@ -117,6 +117,8 @@ done
CLASSPATH="$ZOOBINDIR/../build/classes:$CLASSPATH"

#make it work for developers
CLASSPATH="$ZOOBINDIR/../zookeeper-client/target/classes:$CLASSPATH"
CLASSPATH="$ZOOBINDIR/../zookeeper-common/target/classes:$CLASSPATH"
CLASSPATH="$ZOOBINDIR/../zookeeper-server/target/classes:$CLASSPATH"

#make it work for developers
Expand Down
9 changes: 9 additions & 0 deletions checkstyleSuppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,20 @@
<suppress checks="LineLength" files=".+[\\/]jute[\\/].+\.java"/>

<!-- TODO ZOOKEEPER-3508 -->
<suppress checks="LineLength" files=".+[\\/]zookeeper[\\/].+\.java"/>
<suppress checks="LineLength" files=".+[\\/]zookeeper-common[\\/].+\.java"/>
<suppress checks="LineLength" files=".+[\\/]zookeeper-client[\\/].+\.java"/>
<suppress checks="LineLength" files=".+[\\/]zookeeper-server[\\/].+\.java"/>

<!-- TODO ZOOKEEPER-3507 -->
<suppress checks=".*Name.*" files=".+[\\/]zookeeper[\\/].+\.java"/>
<suppress checks=".*Name.*" files=".+[\\/]zookeeper-common[\\/].+\.java"/>
<suppress checks=".*Name.*" files=".+[\\/]zookeeper-client[\\/].+\.java"/>
<suppress checks=".*Name.*" files=".+[\\/]zookeeper-server[\\/].+\.java"/>

<!-- TODO ZOOKEEPER-3469 -->
<suppress checks="Javadoc.+" files=".+[\\/]zookeeper[\\/].+\.java"/>
<suppress checks="Javadoc.+" files=".+[\\/]zookeeper-common[\\/].+\.java"/>
<suppress checks="Javadoc.+" files=".+[\\/]zookeeper-client[\\/].+\.java"/>
<suppress checks="Javadoc.+" files=".+[\\/]zookeeper-server[\\/].+\.java"/>
</suppressions>
5 changes: 5 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@
</licenses>

<modules>
<module>zookeeper</module>
<module>zookeeper-cli</module>
<module>zookeeper-docs</module>
<module>zookeeper-jute</module>
<module>zookeeper-server</module>
<module>zookeeper-metrics-providers</module>
<module>zookeeper-common</module>
<module>zookeeper-client</module>
<module>zookeeper-client-c</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
<module>zookeeper-compatibility-tests</module>
Expand Down Expand Up @@ -292,6 +296,7 @@
<module>zookeeper-contrib</module>
<!-- zookeeper-it needed by fatjar contrib -->
<module>zookeeper-it</module>
<module>zookeeper-client-c</module>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is redundant with line 68 above, where this module has already been added to the normal build.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it in commit 8e7eb53.

</modules>
</profile>
<profile>
Expand Down
6 changes: 0 additions & 6 deletions zookeeper-assembly/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@
<artifactId>zookeeper</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper-client</artifactId>
<version>${project.version}</version>
<type>pom</type>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client should be included in the assembly, and should be declared explicitly. This old entry brought in the (useless) intermediate pom.xml file, but dropping the <type>pom</type> line should be sufficient to explicitly declare the new zookeeper-client jar as a dependency of the assembly module.

Currently, it seems like the client is being included in the assembly indirectly, via a transitive dependency through the new OSGi library named simply zookeeper (immediately prior dependency in this file). The same seems true of the other new jars. However, they shouldn't be brought in transitively... they should be declared explicitly, so it's easier to see what is intended to be included in the assembly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List zookeeper-client, zookeeper-server and zookeeper-cli explicitly now in e690829. I think it is ok for zookeeper-common to be included transitively ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, I prefer to have tighter control over what gets included in the assembly, so for Accumulo, we do not include any dependencies in our assembly descriptor transitively, and explicitly add them to the POM (see https://github.com/apache/accumulo/tree/2.1/assemble), using a somewhat convoluted process so we just declare what we want in the POM, and only those things get included.

ZooKeeper isn't doing any of that, so it's probably okay for zookeeper-common to be included transitively, but I would probably include it to try to achieve some consistency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZooKeeper isn't doing any of that, so it's probably okay for zookeeper-common to be included transitively, but I would probably include it to try to achieve some consistency.

Done in 6120e83.

using a somewhat convoluted process so we just declare what we want in the POM, and only those things get included.

Out of curiosity, why do you route to this path ? It seems more complicated than transitive approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's mainly because we specifically want to exclude some parts of our dependency tree in Accumulo, because part of our installation instructions assumes you've already installed some of those. For example, we depend on Hadoop and ZooKeeper. We don't ship those dependencies in our build, because we expect the user to add those installations to your Accumulo classpath instead. Hadoop, in particular, has a bunch of changes between different versions with widely different dependencies for itself across different versions. You can run Accumulo on a bunch of different versions of Hadoop, but the dependencies you need is affected by which Hadoop version you use. So, we don't ship them, and require you to link them at runtime instead.

<dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper-prometheus-metrics</artifactId>
Expand Down
17 changes: 15 additions & 2 deletions zookeeper-assembly/src/main/assembly/bin-package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
</includes>
<excludes>
<exclude>org.apache.zookeeper:zookeeper-recipes</exclude>
<exclude>org.apache.zookeeper:zookeeper-client</exclude>
<exclude>org.apache.zookeeper:zookeeper-docs</exclude>
</excludes>
<useProjectArtifact>false</useProjectArtifact>
Expand All @@ -63,16 +62,30 @@
<fileMode>${rw.file.permission}</fileMode>
<directoryMode>${rwx.file.permission}</directoryMode>
</fileSet>
<fileSet>
<!-- ZooKeeper client generated api document -->
<directory>${project.basedir}/../zookeeper-client/target/apidocs</directory>
<outputDirectory>docs/apidocs/zookeeper-client</outputDirectory>
<fileMode>${rw.file.permission}</fileMode>
<directoryMode>${rwx.file.permission}</directoryMode>
</fileSet>
Comment on lines +65 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never noticed before that the apidocs were being included in the ZooKeeper -bin tarball. I don't think it needs to do that. ZK already publishes the javadoc jars, which is a better way to consume the apidocs for IDEs, and for manual viewing, the published apidocs on the zookeeper.apache.org site is sufficient. I think they make the tarball too big, and the build too complicated, and they just aren't needed in the tarball. This is really just an aside comment, though. It's not related to this PR specifically.

<fileSet>
<!-- ZooKeeper server generated api document -->
<directory>${project.basedir}/../zookeeper-server/target/apidocs</directory>
<outputDirectory>docs/apidocs/zookeeper-server</outputDirectory>
<fileMode>${rw.file.permission}</fileMode>
<directoryMode>${rwx.file.permission}</directoryMode>
</fileSet>
<fileSet>
<!-- ZooKeeper cli generated api document -->
<directory>${project.basedir}/../zookeeper-cli/target/apidocs</directory>
<outputDirectory>docs/apidocs/zookeeper-cli</outputDirectory>
<fileMode>${rw.file.permission}</fileMode>
<directoryMode>${rwx.file.permission}</directoryMode>
</fileSet>
<fileSet>
<!-- License files for 3rd party libs -->
<directory>${project.basedir}/../zookeeper-server/src/main/resources/lib</directory>
<directory>${project.basedir}/../zookeeper/src/main/resources/lib</directory>
<includes>
<include>*.txt</include>
</includes>
Expand Down
6 changes: 3 additions & 3 deletions zookeeper-assembly/src/main/assembly/lib-package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<fileSets>
<fileSet>
<!-- ZooKeeper C client lib and include files -->
<directory>${project.basedir}/../zookeeper-client/zookeeper-client-c/target/c</directory>
<directory>${project.basedir}/../zookeeper-client-c/target/c</directory>
<outputDirectory>usr</outputDirectory>
<includes>
<include>include/**/*</include>
Expand All @@ -40,7 +40,7 @@
</fileSet>
<fileSet>
<!-- ZooKeeper C client binaries-->
<directory>${project.basedir}/../zookeeper-client/zookeeper-client-c/target/c</directory>
<directory>${project.basedir}/../zookeeper-client-c/target/c</directory>
<outputDirectory>usr</outputDirectory>
<includes>
<include>bin/*</include>
Expand All @@ -50,7 +50,7 @@
</fileSet>
<fileSet>
<!-- ZooKeeper license -->
<directory>${project.basedir}/../zookeeper-client/zookeeper-client-c</directory>
<directory>${project.basedir}/../zookeeper-client-c</directory>
<includes>
<include>LICENSE</include>
</includes>
Expand Down
Loading