[GitHub] [zookeeper] anmolnar commented on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest
anmolnar commented on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest URL: https://github.com/apache/zookeeper/pull/949#issuecomment-493928910 Committed to master branch. Thanks @enixon ! Btw @enixon you don't assign Jiras to yourself. Is that because you don't have permission? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] asfgit closed pull request #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest
asfgit closed pull request #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest URL: https://github.com/apache/zookeeper/pull/949 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #952: ZOOKEEPER-3364: Compile with strict options in order to check code quality
eolivelli commented on issue #952: ZOOKEEPER-3364: Compile with strict options in order to check code quality URL: https://github.com/apache/zookeeper/pull/952#issuecomment-493840020 @anmolnar This is the port for 3.5 I want to do the same for 3.4. The goal is to drop ant based quality checks for 3.5 and master, but this part of automatic QA should be shared This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli opened a new pull request #952: ZOOKEEPER-3364: Compile with strict options in order to check code quality
eolivelli opened a new pull request #952: ZOOKEEPER-3364: Compile with strict options in order to check code quality URL: https://github.com/apache/zookeeper/pull/952 This is the port of ZOOKEEPER-3364 to branch-3.5 - Add extra compiler arguments in order to achieve better code quality. - Fix some minor issues reported by javac - Extra checks are not enabled in "contrib" module. Author: Enrico Olivelli Reviewers: an...@apache.org Closes #910 from eolivelli/fix/ZOOKEEPER-3364-javac-strict and squashes the following commits: 9660aff82 [Enrico Olivelli] Fix tests and fix warnings on JDK12 52895ec9d [Enrico Olivelli] ZOOKEEPER-3364 Compile with strict options in order to check code quality This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] maoling closed pull request #951: Zookeeper 3301
maoling closed pull request #951: Zookeeper 3301 URL: https://github.com/apache/zookeeper/pull/951 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] maoling opened a new pull request #951: Zookeeper 3301
maoling opened a new pull request #951: Zookeeper 3301 URL: https://github.com/apache/zookeeper/pull/951 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493597736 @anmolnar the result is the same, still the same warning. I feel we can live with it. This work is very useful This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli edited a comment on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest
eolivelli edited a comment on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest URL: https://github.com/apache/zookeeper/pull/949#issuecomment-493594696 @anmolnar can you please commit this one ? that test is very annoying on CI This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest
eolivelli commented on issue #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest URL: https://github.com/apache/zookeeper/pull/949#issuecomment-493594696 @anmolnar can you commit this one ? that test is very annoying on CI This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493474795 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493459800 Updated. Please check. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493453739 @anmolnar maybe it helps, I don't know. Please try This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
anmolnar commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493453441 @eolivelli shall we upgrade BouncyCastle? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil
eolivelli commented on issue #950: ZOOKEEPER-3263. JAVA9/11 Warnings: Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950#issuecomment-493429920 @anmolnar the issue you are trying to fix is fixed with this patch. I am seeing errors like the ones below due to BouncyCastle inside 019-05-17T13-50-00_871.dumpstream files: ``` WARNING: Illegal reflective access by org.bouncycastle.jcajce.provider.drbg.DRBG (file:/home/diennea.lan/enrico.olivelli/.m2/repository/org/bouncycastle/bcprov-jdk15on/1.60/bcprov-jdk15on-1.60.jar) to constructor sun.security.provider.Sun() # Created at 2019-05-17T13:50:22.497 WARNING: Please consider reporting this to the maintainers of org.bouncycastle.jcajce.provider.drbg.DRBG ``` This is fine because we are using bouncycastle only for tests and users won't see those errors in production. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on a change in pull request #923: ZOOKEEPER-1426: add version command to the zookeeper server
eolivelli commented on a change in pull request #923: ZOOKEEPER-1426: add version command to the zookeeper server URL: https://github.com/apache/zookeeper/pull/923#discussion_r285089642 ## File path: zookeeper-server/src/test/resources/test-scripts.sh ## @@ -217,6 +217,9 @@ stop $ZKSI --force --myid=1 --configfile "$ZOOCFGDIR/$ZOOCFG" || fail $LINENO +#test version script Review comment: is this file working on maven build ? It checks for "ls build/zookeeper*.jar" but the mavenized build does not create such file. If I am not wrong, do you mind @szepet fixing it up ? otherwise as soon as we drop the ant-based build this script will need a fix. Honestly I didn't known such file existed This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve efficiency of DeleteAllCommand
anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve
efficiency of DeleteAllCommand
URL: https://github.com/apache/zookeeper/pull/899#discussion_r284974888
##
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
##
@@ -45,20 +48,67 @@
*
* @throws IllegalArgumentException if an invalid path is specified
*/
-public static void deleteRecursive(ZooKeeper zk, final String pathRoot)
+public static boolean deleteRecursive(ZooKeeper zk, final String pathRoot)
throws InterruptedException, KeeperException
{
PathUtils.validatePath(pathRoot);
List tree = listSubTreeBFS(zk, pathRoot);
LOG.debug("Deleting " + tree);
LOG.debug("Deleting " + tree.size() + " subnodes ");
-for (int i = tree.size() - 1; i >= 0 ; --i) {
-//Delete the leaves first and eventually get rid of the root
-zk.delete(tree.get(i), -1); //Delete all versions of the node with
-1.
+
+int asyncReqRateLimit = 10;
+// Try deleting the tree nodes in batches of size 1000.
+// If some batch failed, try again with batches of size 1 to delete as
+// many nodes as possible.
Review comment:
Oh, this is not the cli, but `ZKUtil`, so essentially it's a public api.
Another reason not to be proactive.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve efficiency of DeleteAllCommand
anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve
efficiency of DeleteAllCommand
URL: https://github.com/apache/zookeeper/pull/899#discussion_r284974695
##
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
##
@@ -45,20 +48,67 @@
*
* @throws IllegalArgumentException if an invalid path is specified
*/
-public static void deleteRecursive(ZooKeeper zk, final String pathRoot)
+public static boolean deleteRecursive(ZooKeeper zk, final String pathRoot)
throws InterruptedException, KeeperException
{
PathUtils.validatePath(pathRoot);
List tree = listSubTreeBFS(zk, pathRoot);
LOG.debug("Deleting " + tree);
LOG.debug("Deleting " + tree.size() + " subnodes ");
-for (int i = tree.size() - 1; i >= 0 ; --i) {
-//Delete the leaves first and eventually get rid of the root
-zk.delete(tree.get(i), -1); //Delete all versions of the node with
-1.
+
+int asyncReqRateLimit = 10;
+// Try deleting the tree nodes in batches of size 1000.
+// If some batch failed, try again with batches of size 1 to delete as
+// many nodes as possible.
Review comment:
I agree with @szepet : I don't think we need this retry logic in the cli.
I would just let the user know about the command failed because of this and
that and let him decide whether he/she wants to retry with a different batch
size.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] elireisman commented on issue #923: ZOOKEEPER-1426: add version command to the zookeeper server
elireisman commented on issue #923: ZOOKEEPER-1426: add version command to the zookeeper server URL: https://github.com/apache/zookeeper/pull/923#issuecomment-493230356 Yay! Been a while (!!!) but happy to chime in if you need me, thanks for rebasing this This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #939: ZOOKEEPER-3385: Add admin command to display leader
enixon commented on issue #939: ZOOKEEPER-3385: Add admin command to display leader URL: https://github.com/apache/zookeeper/pull/939#issuecomment-493187070 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #947: ZOOKEEPER-3392: Add admin command to display last snapshot information
enixon commented on issue #947: ZOOKEEPER-3392: Add admin command to display last snapshot information URL: https://github.com/apache/zookeeper/pull/947#issuecomment-493186770 retest ant build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #851: ZOOKEEPER-3311: Allow a delay to the transaction log flush
enixon commented on issue #851: ZOOKEEPER-3311: Allow a delay to the transaction log flush URL: https://github.com/apache/zookeeper/pull/851#issuecomment-493186586 retest maven build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #918: ZOOKEEPER-3366: Pluggable metrics system for ZooKeeper - move remaining metrics to MetricsProvider
eolivelli commented on issue #918: ZOOKEEPER-3366: Pluggable metrics system for ZooKeeper - move remaining metrics to MetricsProvider URL: https://github.com/apache/zookeeper/pull/918#issuecomment-493170070 @lvfangmin do you have time to finish your review? Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #923: ZOOKEEPER-1426: add version command to the zookeeper server
anmolnar commented on issue #923: ZOOKEEPER-1426: add version command to the zookeeper server URL: https://github.com/apache/zookeeper/pull/923#issuecomment-493161575 @nkalmar I don't think those error messages are related to this patch. That's basically the expected behaviour of this script when no config file is available. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
anmolnar commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config URL: https://github.com/apache/zookeeper/pull/761#issuecomment-493090307 Committed to master branch. Thanks @enixon ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] asfgit closed pull request #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
asfgit closed pull request #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config URL: https://github.com/apache/zookeeper/pull/761 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #915: ZOOKEEPER-3370 Remove SVN specific revision generation
anmolnar commented on issue #915: ZOOKEEPER-3370 Remove SVN specific revision generation URL: https://github.com/apache/zookeeper/pull/915#issuecomment-493085224 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar closed pull request #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on conn…
anmolnar closed pull request #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on conn… URL: https://github.com/apache/zookeeper/pull/592 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on conn…
anmolnar commented on issue #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on conn… URL: https://github.com/apache/zookeeper/pull/592#issuecomment-493084748 Due to @PhantomThief 's comment, I'm closing this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493061694 @nkalmar Solution #2 seems to be feasible - I've just tried simple prototype in different branch: https://github.com/JiriOndrusek/zookeeper/commits/zookeeper-osgi-module This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar opened a new pull request #950: ZOOKEEPER-3263. Illegal reflective access in zookeer's kerberosUtil
anmolnar opened a new pull request #950: ZOOKEEPER-3263. Illegal reflective access in zookeer's kerberosUtil URL: https://github.com/apache/zookeeper/pull/950 Fixes warning messages of JDK 9/11 by upgrading libraries and refactoring `KerberosUtils` based on experiences of [HADOOP-10848](https://issues.apache.org/jira/browse/HADOOP-10848) Reviewers please run unit tests with various JDK versions including 9 and 11, because CI only runs on JDK 8. Thanks. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493025000 I've sent email to zookeeper-dev mailing list: ZooKeeper and OSGi (maven) Hi, I 've created issue [1] with missing exported packages in osgi for zookeeper 3.4.10. Then I started to prepare maven OSGi packaging [2] for the higher version of ZooKeeper (in the PR for issue). I've tried to implement OSGi packaging with the low impact. So I've tried to create OSGi bundles from Zookeeper-server and from zookeeper-jute modules. But there is a problem for this solution: zookeeper-jute has package 'org.apache.zookeeper.data', it has to be exported for zookeeper-server to use it zookeeper-server contains also package 'org.apache.zookeeper.data', which has to be exported, because it is used from packages like org.apache.zookeeper, which are exported -> bundles can not be deployed into osgi as two libraries are exporting the same packages Solution: best solution is to change name of one of these packages (probably in module zookeeper-jute - which us used only by zookeeper) - but question is, whether this change is feasible only other solution is to create only one bundle (e.g. zookeeper-osgi), which will contain both libraries together and will expose their both packages at the same time (similar approach is used in e.g. hibernate-osgi, httpcore-osgi, httpclient-osgi) Solution #1 is a better solution, I would like to ask for your opinion about feasibility of renaming zookeeper-jute generated packages to not collide with zookeeper-server. (As these packages are to be used only for zookeeper, it shouldn't cause any harm) If #1 is not acceptable, then we can go with #2. But I highly suggest to consider renaming of zookeeper-jute's packages in the nearest point in the future as possible and return to solution #1. Best regards, jiri [1] https://issues.apache.org/jira/browse/ZOOKEEPER-3389 [2] https://github.com/apache/zookeeper/pull/945 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493025000 I've sent email to zookeeper-dev mailing list: > ZooKeeper and OSGi (maven) Hi, I 've created issue [1] with missing exported packages in osgi for zookeeper 3.4.10. Then I started to prepare maven OSGi packaging [2] for the higher version of ZooKeeper (in the PR for issue). I've tried to implement OSGi packaging with the low impact. So I've tried to create OSGi bundles from Zookeeper-server and from zookeeper-jute modules. But there is a problem for this solution: zookeeper-jute has package 'org.apache.zookeeper.data', it has to be exported for zookeeper-server to use it zookeeper-server contains also package 'org.apache.zookeeper.data', which has to be exported, because it is used from packages like org.apache.zookeeper, which are exported -> bundles can not be deployed into osgi as two libraries are exporting the same packages Solution: best solution is to change name of one of these packages (probably in module zookeeper-jute - which us used only by zookeeper) - but question is, whether this change is feasible only other solution is to create only one bundle (e.g. zookeeper-osgi), which will contain both libraries together and will expose their both packages at the same time (similar approach is used in e.g. hibernate-osgi, httpcore-osgi, httpclient-osgi) Solution #1 is a better solution, I would like to ask for your opinion about feasibility of renaming zookeeper-jute generated packages to not collide with zookeeper-server. (As these packages are to be used only for zookeeper, it shouldn't cause any harm) If #1 is not acceptable, then we can go with #2. But I highly suggest to consider renaming of zookeeper-jute's packages in the nearest point in the future as possible and return to solution #1. Best regards, jiri [1] https://issues.apache.org/jira/browse/ZOOKEEPER-3389 [2] https://github.com/apache/zookeeper/pull/945 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493017349 @nkalmar I've just asked for subscription to dev mailing list, then I'd be able to send an email there. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493015133 We would still need to solve it for 3.5 branch. So, for me, creating a zookeeper-osgi seems the way to go, but this should be voted on the zookeeper-dev mail list. Can you write an email to the dev list @JiangJiafu with your findings? A short summary, and then you can link this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493015133 We would still need to solve it for 3.5 branch. So, for me, creating a zookeeper-osgi seems the way to go, but this should be voted on the zookeeper-dev mail list. Can you write an email to the dev list @JiangJiafu with your findings? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493012808 @eolivelli @nkalmar I've found problem with current approach (2 bundles, one is server and second is jute) Problem: - zookeeper-jute has package 'org.apache.zookeeper.data', it has to be exported for zookeeper-server to use it - zookeeper-server contais also package 'org.apache.zookeeper.data', which has to be exported, because it is use from packages like org.apache.zookeeper -> bundles can not be deployed into karaf as two libraries are exporting the same packages Solution: - best solution is to change name of one of these packages (probably in zookeeper-jute - which us used only by zookeeper) - but as @eolivelli wrote: "Yes changing the package name is feasible but only for 3.6+." - only other solution is to create only one bundle (e.g. zookeeper-osgi), which will contain both libraries together and will expose theit both packages at the same time (simillar approach is used in e.g. hibernate-osgi, httpcore-osgi, httpclient-osgi) But with the fact that in the 3.6+ version renaming is possible so we can use solution #1 from 3.6+ as it is a better solution, i would like to ask on your opinion about current version. Is renaming of jute's packages really unreal. Is creation of zookeeper-osgi library the only solution? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-493012808 @eolivelli @nkalmar I've found problem with current approach (2 bundles, one is server and second is jute) Problem: - zookeeper-jute has package 'org.apache.zookeeper.data', it has to be exported for zookeeper-server to use it - zookeeper-server contais also package 'org.apache.zookeeper.data', which has to be exported, because it is use from packages like org.apache.zookeeper -> bundles can not be deployed into karaf as two libraries are exporting the same packages Solution: - best solution is to change name of one of these packages (probably in zookeeper-jute - which us used only by zookeeper) - but as @eolivelli wrote: "Yes changing the package name is feasible but only for 3.6+." - only other solution is to create only one bundle (e.g. zookeeper-osgi), which will contain both libraries together and will expose theit both packages at the same time (simillar approach is used in e.g. hibernate-osgi, httpcore-osgi, httpclient-osgi) But with the fact that in the 3.6+ version renaming is possible so we can use solution #1 from 3.6+ as it is a better solution, i would like to ask on your opinion about current version. Is renaming of jute's packages really unreal. Is creation of zookeeper-osgi library the only solution? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve efficiency of DeleteAllCommand
anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve
efficiency of DeleteAllCommand
URL: https://github.com/apache/zookeeper/pull/899#discussion_r284615207
##
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
##
@@ -45,20 +48,71 @@
*
* @throws IllegalArgumentException if an invalid path is specified
*/
-public static void deleteRecursive(ZooKeeper zk, final String pathRoot)
+public static boolean deleteRecursive(ZooKeeper zk, final String pathRoot)
throws InterruptedException, KeeperException
{
PathUtils.validatePath(pathRoot);
List tree = listSubTreeBFS(zk, pathRoot);
LOG.debug("Deleting " + tree);
LOG.debug("Deleting " + tree.size() + " subnodes ");
-for (int i = tree.size() - 1; i >= 0 ; --i) {
-//Delete the leaves first and eventually get rid of the root
-zk.delete(tree.get(i), -1); //Delete all versions of the node with
-1.
+
+int asyncReqRateLimit = 10;
+// Try deleting the tree nodes in batches of size 1000.
Review comment:
I like (2)
Let it be 1000 by default and make overridable with an optional parameter.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492984271 I was able to install manually zookeeper with curator. But I had to change a little bit export-packages in zookeeper-server, which throws now warning during build about package "org.apache.zookeeper.data". Warning message looks like: zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.cli, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.client, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.server, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.server.auth, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.server.quorum, has 1, private references [org.apache.zookeeper.data] zookeeper:bundle:3.6.0-SNAPSHOT : Export org.apache.zookeeper.proto, has 1, private references [org.apache.zookeeper.data] Meaning of this warning is, that some classes from zookeeper (zookeeper.cli, ...) for example returns classes from package "org.apache.zookeeper.data" which is not exported from the bundle. I have to solve this minor thing. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492974467 @nkalmar I've added OSGi support to zookeeper-jute. I had to change pockaging to bundle as well, but in maven environment it behaves still as jar packaging. @eolivelli I think that it is possible to prepare integration test. I'll try to verify both bundles manually (that they could be deployed) I've talked to @grgrzybek and he could possibly help with integration test. I think that current state is, that both jars (z-server and z-jute) are also osgi bundles. Now I plan to fine tuning dependencies during manual installation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492971219 What I meant in "private package" is that we do not expose any of this to clients. Jute is only used to serialize messages between ZooKeeper nodes, client does not use Jute for serialization. In theory, anyone could use zookeeper-jute for serialization, in reality, no one will and no one should. But it was agreed upon jute should be a different module still. And on a side note, some want to replace jute with a more recent and actively developed serialization library like protobuf or avro. But, for now, jute is here to stay, and it might only be replaced in a new major version (like 4.0.0). Anyway, thanks again for looking into this, the merge stuff sounds good! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492933860 @eolivelli Ok, i'll continue with osgi packaging. I'll apply this also to zookeeper-jute and somehow solve "the same package name" problem in the way as described in my previous post. Integration test validating that OSGi is working, would be really nice. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492933860 @eolivelli Ok, i'll continue with osgi packaging. I'll apply this also to zookeeper-jute and somehow solve "the same package name" problem in the way as described in my previous post. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492824563 @enixon Thanks, it's great you already have a fix in mind. No worries and no rush, just take your time. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar edited a comment on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
anmolnar edited a comment on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492824563 @enixon Thanks, it's great you already have a fix. No worries and no rush, just take your time. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492824404 It would be great to have some integration test which loads zookeeper client with osgi and this way we will ensure that we are delivering something that really works. We can do this by adding another module to the build which uses the client inside an osgi container. It is not so simple (because the test will need to start at least one server and use the client and server and client are packaged inside of the same jars). We have to setup these kind of integration tests (maybe docker based) in the short term, not necessarily for thia case. I will be happy to work on this. Maybe you can continue with the osgi packaging stuff and I will then try to setup the integration test in a followup patch This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
enixon commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492812456 Created a new ticket (3396) and opened #949 to resolve (cc @anmolnar - sorry for the trouble!) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon opened a new pull request #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest
enixon opened a new pull request #949: ZOOKEEPER-3396: Flaky test in RestoreCommittedLogTest URL: https://github.com/apache/zookeeper/pull/949 Adjusting parameters of RestoreCommittedLogTest to avoid failures caused by threshold configured to be below the minimum allowed file size This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
enixon commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492804816 @anmolnar It looks like the issue is FileTxnLog::getCurrentLogSize returns a multiple of a high number that is dependent on the machine running the test (the current thresholds work for my machine but playing with them starts to generate predictable error). With the low size thresholds in use in this test (e.g. less than 100 Kb), this leads to false positives in the "should snapshot" logic from the point of view of the test and it fails. I can put up a patch that fixes this flakiness. What's the preferred procedure - open a new ticket or link to this ticket (ZK-3244)? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492796520 Hi @nkalmar @eolivelli, let me introduce "a better solution" for the problem with same package's names of zookeeper-jute and zookeeper-server. If I understand it correctly, maven way of solving this problem is acceptable even if it causes problems sometimes. From previous comment - "It's not ideal, and even causes problems like this one with OSGi." Maven solution is that in case that packages DOESN'T contain the same classes - it works as EXPECTED. If it contains the same classes, then classloader return class from jar based on ORDER of jars. (so even if I want clas from jute, I can get class from server) Which is typicalli zookeeper-server and then zookeeper-jute (as jute is dependency from server) I propose to use OSGi merge capability, which brings the same behavior. If someone wants class from that shared package, in case that classes are differen in both jars, this merge "feature" will behave as expected and correct class is returned. In case that there are overlapping classes, we define, which jar has advantage. (so I'd say that zookeeper-server has advantage). IMHO behavior will be a little bit better then in maven, because it will behave always in the same way. (In reality it won't robabnly happen, because the same classes in the shared packages will probably cause problems elsewhere, so everyone will try to avoid that) In other words - behavior of OSGI with merge will be the same as maven if packages contains different classes. If in the future there will be the overlapping classes, behavior in OSGi would be defined (in contrast with maven, where it would ddepend on order of jar loading - so almost "random") Possibly (I'm not sure about it in this time) I can also try to add some insurance/condition, that it will work for 3.5.x only. I mean that after upgrading to higher version of zookeeper, it will cause error during build (or at least warning), so someone will look into it and will find a comment explaining, that correct solution is to rename packages -> which is the correct solution. In case that packages won't be renamed at that time. This condition would allow us to solve problem in 3.5.x time, where packages couldn't be renamed. Just to avoid confusion, this merge capability only defines, how OSGi returns classes if the class is in package, which is shared in more jars. No actuall merging of compiled classes is not happening. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on a change in pull request #899: ZOOKEEPER-3354: Improve efficiency of DeleteAllCommand
enixon commented on a change in pull request #899: ZOOKEEPER-3354: Improve
efficiency of DeleteAllCommand
URL: https://github.com/apache/zookeeper/pull/899#discussion_r284359820
##
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
##
@@ -45,20 +48,71 @@
*
* @throws IllegalArgumentException if an invalid path is specified
*/
-public static void deleteRecursive(ZooKeeper zk, final String pathRoot)
+public static boolean deleteRecursive(ZooKeeper zk, final String pathRoot)
throws InterruptedException, KeeperException
{
PathUtils.validatePath(pathRoot);
List tree = listSubTreeBFS(zk, pathRoot);
LOG.debug("Deleting " + tree);
LOG.debug("Deleting " + tree.size() + " subnodes ");
-for (int i = tree.size() - 1; i >= 0 ; --i) {
-//Delete the leaves first and eventually get rid of the root
-zk.delete(tree.get(i), -1); //Delete all versions of the node with
-1.
+
+int asyncReqRateLimit = 10;
+// Try deleting the tree nodes in batches of size 1000.
Review comment:
1000 seemed like a reasonable number. I'm open to the idea of allowing the
user to do their own custom tuning, we didn't go with it because the available
schemes looked heavyweight and in practice, the users didn't care.
Two options: (1) take this property as a java/system arg or (2) modify the
cli for deleteall to allow batch size as a parameter in addition to the path.
Do you want to speak for one of these?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492714723 Maybe you can simply add raw entries to the manifest in the jar plugin configuration This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492704512 @nkalmar @eolivelli I'll continue with this tomorrow. I see in your comment: "Those should be all private to ZooKeeper, so we do not change public API" and the fact that jute is another artifact, I see "simple" solution: I'll try to dig up little bit if there is some way, when zookeeper-jute won't export these "private packages". But I have to try some possibilities, how to solve zookeeper-server's dependency on those libraries. I'm not sure whether it will be possible. Current status is: - build changes are only for zookeeper-server to be changed into OSGi bundle, - there are no warnings during build. - I have to validate whether there is some "real" difference between current manifest and manifest generated by ant. As there is a mentioning, that changing packaging from 'jar' to 'bundle' will cause some errors elsewhere. Unfortunately I'm not aware of other way of running OSGi build, but I'll try to look into it also tomorrow. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492683153 Yes changing the package name is feasible but only for 3.6+. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492680330 I see you changed zookeeper-server packaging from jar to bundle. Yeah... I will check up on this sometime this week. I'm afraid this also affects other things. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492677907 There will be 2 jars upon release, zookeeper-jute and zookeeper-server. zookeeper-jute is the serialization library (just like protobuf). zookeeper-server depends on zookeeper-jute. Since jute (originally) was an independent project, it was always kept seperate. But it evolved with zookeeper, hence the same package names. It's not ideal, and even causes problems like this one with OSGi. The jute classes are generated, just like in protobuf. I'm afraid merging the packages together is not.. uhm, preferable, to say. The most plausible solution out of these for me seems to be changing jute package names. Those should be all private to ZooKeeper, so we do not change public API. But I would be happy if we found another solution. Unfortunately I don't know much about OSGi, so I don't know what the best solution would be here. But zookeeper-server needs zookeeper-jute to serialize the messages between nodes. Maven automatically downloads zookeeper-jute when zookeeper-server is used, as it is a dependency for it. I'm not sure how OSGi solves this. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492677907 There will be 2 jars upon release, zookeeper-jute and zookeeper-server. zookeeper-jute is the serialization library (just like protobuf). zookeeper-server depends on zookeeper-jute. Since jute (originally) was an independent project, it was always kept seperate. But it evolved with zookeeper, hence the same package names. It's not ideal, and even causes problems like this one with OSGi. The jute classes are generated, just like in protobuf. I'm afraid merging the packages together is not.. uhm, preferable, to say. The most plausible solution out of these fro me seems to be changing jute package names. Those should be all private to ZooKeeper, so we do not change public API. But I would be happy if we found another solution. Unfortunately I don't know much about OSGi, so I don't know what the best solution would be here. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek edited a comment on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492672683 @nkalmar @eolivelli I'm getting close to the finishing. I just want to clarify some facts and also ask for the help. - just to be sure, only zookeeper-server jar will be compiled as OSGi bundle? (I see also possible modules jute and recipes, so I would like to be sure) - I'm currently facing a problem with OSGi build, there is a warning that zookeeper-server is exporting package, which is also present in different jar - zookeeper-jute. (Packages org/apache/zookeeper/server/quorum and org/apache/zookeeper/server/persistence) It would help me to know, how these packages should be handled. I looked into package 'quorum' and it seems, that it contains different classes. But even if they are different, it could change in the FUTURE. So I'd like to ask, how to face it - best solution from OSGi would be to change jute to use different package names - but it is probably not possible - best way (from my POV) is to merge packages together (with rule that classes from zookeeper-server has to be treated as primary) - but it can remove some classes which are needed to run and I have also problems to define it with maven-plugin (still not sure why it is not working, I think it should work) - may be it is possible also to ignore these packages from zookeeper-jute - but it doesn't seem much probable What do you think about it? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492672683 @nkalmar @eolivelli I'm getting close to the finishing. I just want to clarify some facts and also ask for the help. - just to be sure, only zookeeper-server jar will be compiled as OSGi bundle? (I see also possible modules jute and recipes, so I would like to be sure) - I'm currently facing a problem with OSGi build, there is a warning that zookeeper-server is exporting package, which is also present in different jar - zookeeper-jute. (Packages org/apache/zookeeper/server/quorum and org/apache/zookeeper/server/persistence) It would help me to know, how these packages should be handled. I looked into package 'quorum' and it seems, that it contains different classes. But even if they are different, it could change in the FUTURE. So I'd like to ask, how to face it - best solution from OSGi would be to change jute to use different package names - but it is probably not possible - best way (from my POV) is to merge packages together (with rule that classes from zookeeper-server has to be treated as primary) - but it can remove some classes which are needed to run and I have also problems to define it with maven-plugin (still not sure why it is not working, I think it should work) - may be it is possible also to ignore these packages from zookeeper-jute - but it doesn't seem much probable What do you think about it? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve efficiency of DeleteAllCommand
anmolnar commented on a change in pull request #899: ZOOKEEPER-3354: Improve
efficiency of DeleteAllCommand
URL: https://github.com/apache/zookeeper/pull/899#discussion_r284260180
##
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
##
@@ -45,20 +48,71 @@
*
* @throws IllegalArgumentException if an invalid path is specified
*/
-public static void deleteRecursive(ZooKeeper zk, final String pathRoot)
+public static boolean deleteRecursive(ZooKeeper zk, final String pathRoot)
throws InterruptedException, KeeperException
{
PathUtils.validatePath(pathRoot);
List tree = listSubTreeBFS(zk, pathRoot);
LOG.debug("Deleting " + tree);
LOG.debug("Deleting " + tree.size() + " subnodes ");
-for (int i = tree.size() - 1; i >= 0 ; --i) {
-//Delete the leaves first and eventually get rid of the root
-zk.delete(tree.get(i), -1); //Delete all versions of the node with
-1.
+
+int asyncReqRateLimit = 10;
+// Try deleting the tree nodes in batches of size 1000.
Review comment:
Why 1000?
What if we make it configurable somehow?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492604226 Committed to master branch. Thanks @enixon ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492603934 @enixon Please double check your test code, it looks like this patch has introduced a flaky test: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk-java10/563/ https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/521/ https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk-java11/422/ https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk-java11/423/ ... with the following error: ``` junit.framework.AssertionFailedError: too many snapshot files at org.apache.zookeeper.test.RestoreCommittedLogTest.testRestoreCommittedLog(RestoreCommittedLogTest.java:119) at org.apache.zookeeper.test.RestoreCommittedLogTest.testRestoreCommittedLogWithSnapSize(RestoreCommittedLogTest.java:67) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:80) ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] asfgit closed pull request #898: ZOOKEEPER-3353: Admin commands for showing initial settings
asfgit closed pull request #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492587354 I agree with Enrico, please just continue on this branch, as maven will be used to release future 3.5.x and 3.6.x versions. I don't have much to add, but I'll keep a lookout for this PR! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492569164 @JiriOndrusek I feel you can continue on this branch, we can merge all this OSGI stuff in a single commit. On branch 3.5 and master we are not using Ant anymore. Ant is living mostly only for 3.4 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492561609 Hi @nkalmar , I've started with osgification of maven build, so far in my branch https://github.com/JiriOndrusek/zookeeper/commits/osgi_in_maven If you've already started with this task also, we can calaborate. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492516231 retest maven build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
enixon commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config URL: https://github.com/apache/zookeeper/pull/761#issuecomment-492443350 retest maven build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492442399 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav…
enixon commented on issue #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav… URL: https://github.com/apache/zookeeper/pull/948#issuecomment-492394603 retest maven build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492394459 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
enixon commented on issue #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config URL: https://github.com/apache/zookeeper/pull/761#issuecomment-492394550 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on a change in pull request #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on a change in pull request #898: ZOOKEEPER-3353: Admin
commands for showing initial settings
URL: https://github.com/apache/zookeeper/pull/898#discussion_r283982376
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java
##
@@ -317,79 +322,81 @@ public CommandResponse run(ZooKeeperServer zkServer,
Map kwargs)
*/
public static class MonitorCommand extends CommandBase {
public MonitorCommand() {
-super(Arrays.asList("monitor", "mntr"));
+super(Arrays.asList("monitor", "mntr"), false);
}
@Override
public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) {
-ZKDatabase zkdb = zkServer.getZKDatabase();
-ServerStats stats = zkServer.serverStats();
-
CommandResponse response = initializeResponse();
response.put("version", Version.getFullVersion());
-response.put("avg_latency", stats.getAvgLatency());
-response.put("max_latency", stats.getMaxLatency());
-response.put("min_latency", stats.getMinLatency());
+OSMXBean osMbean = new OSMXBean();
+response.put("open_file_descriptor_count",
osMbean.getOpenFileDescriptorCount());
+response.put("max_file_descriptor_count",
osMbean.getMaxFileDescriptorCount());
-response.put("packets_received", stats.getPacketsReceived());
-response.put("packets_sent", stats.getPacketsSent());
-response.put("num_alive_connections",
stats.getNumAliveClientConnections());
+if (zkServer != null) {
+ZKDatabase zkdb = zkServer.getZKDatabase();
+ServerStats stats = zkServer.serverStats();
+response.put("avg_latency", stats.getAvgLatency());
+response.put("max_latency", stats.getMaxLatency());
+response.put("min_latency", stats.getMinLatency());
-response.put("outstanding_requests",
stats.getOutstandingRequests());
-response.put("uptime", stats.getUptime());
+response.put("packets_received", stats.getPacketsReceived());
+response.put("packets_sent", stats.getPacketsSent());
+response.put("num_alive_connections",
stats.getNumAliveClientConnections());
-response.put("server_state", stats.getServerState());
-response.put("znode_count", zkdb.getNodeCount());
+response.put("outstanding_requests",
stats.getOutstandingRequests());
+response.put("uptime", stats.getUptime());
-response.put("watch_count", zkdb.getDataTree().getWatchCount());
-response.put("ephemerals_count",
zkdb.getDataTree().getEphemeralsCount());
-response.put("approximate_data_size",
zkdb.getDataTree().cachedApproximateDataSize());
+response.put("server_state", stats.getServerState());
+response.put("znode_count", zkdb.getNodeCount());
-response.put("global_sessions", zkdb.getSessionCount());
-response.put("local_sessions",
-zkServer.getSessionTracker().getLocalSessionCount());
+response.put("watch_count",
zkdb.getDataTree().getWatchCount());
+response.put("ephemerals_count",
zkdb.getDataTree().getEphemeralsCount());
+response.put("approximate_data_size",
zkdb.getDataTree().cachedApproximateDataSize());
-OSMXBean osMbean = new OSMXBean();
-response.put("open_file_descriptor_count",
osMbean.getOpenFileDescriptorCount());
-response.put("max_file_descriptor_count",
osMbean.getMaxFileDescriptorCount());
-response.put("connection_drop_probability",
zkServer.getConnectionDropChance());
+response.put("global_sessions", zkdb.getSessionCount());
+response.put("local_sessions",
+zkServer.getSessionTracker().getLocalSessionCount());
-response.put("last_client_response_size",
stats.getClientResponseStats().getLastBufferSize());
-response.put("max_client_response_size",
stats.getClientResponseStats().getMaxBufferSize());
-response.put("min_client_response_size",
stats.getClientResponseStats().getMinBufferSize());
+response.put("connection_drop_probability",
zkServer.getConnectionDropChance());
-if (zkServer instanceof QuorumZooKeeperServer) {
-QuorumPeer peer = ((QuorumZooKeeperServer) zkServer).self;
-response.put("quorum_size", peer.getQuorumSize());
-}
+response.put("last_client_response_size",
stats.getClientResponseStats().getLastBufferSize());
+response.put("max_client_response_size",
stats.getClientResponseStats().getMaxBufferSize());
+
[GitHub] [zookeeper] eolivelli commented on a change in pull request #898: ZOOKEEPER-3353: Admin commands for showing initial settings
eolivelli commented on a change in pull request #898: ZOOKEEPER-3353: Admin
commands for showing initial settings
URL: https://github.com/apache/zookeeper/pull/898#discussion_r283979799
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java
##
@@ -317,79 +322,81 @@ public CommandResponse run(ZooKeeperServer zkServer,
Map kwargs)
*/
public static class MonitorCommand extends CommandBase {
public MonitorCommand() {
-super(Arrays.asList("monitor", "mntr"));
+super(Arrays.asList("monitor", "mntr"), false);
}
@Override
public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) {
-ZKDatabase zkdb = zkServer.getZKDatabase();
-ServerStats stats = zkServer.serverStats();
-
CommandResponse response = initializeResponse();
response.put("version", Version.getFullVersion());
-response.put("avg_latency", stats.getAvgLatency());
-response.put("max_latency", stats.getMaxLatency());
-response.put("min_latency", stats.getMinLatency());
+OSMXBean osMbean = new OSMXBean();
+response.put("open_file_descriptor_count",
osMbean.getOpenFileDescriptorCount());
+response.put("max_file_descriptor_count",
osMbean.getMaxFileDescriptorCount());
-response.put("packets_received", stats.getPacketsReceived());
-response.put("packets_sent", stats.getPacketsSent());
-response.put("num_alive_connections",
stats.getNumAliveClientConnections());
+if (zkServer != null) {
+ZKDatabase zkdb = zkServer.getZKDatabase();
+ServerStats stats = zkServer.serverStats();
+response.put("avg_latency", stats.getAvgLatency());
+response.put("max_latency", stats.getMaxLatency());
+response.put("min_latency", stats.getMinLatency());
-response.put("outstanding_requests",
stats.getOutstandingRequests());
-response.put("uptime", stats.getUptime());
+response.put("packets_received", stats.getPacketsReceived());
+response.put("packets_sent", stats.getPacketsSent());
+response.put("num_alive_connections",
stats.getNumAliveClientConnections());
-response.put("server_state", stats.getServerState());
-response.put("znode_count", zkdb.getNodeCount());
+response.put("outstanding_requests",
stats.getOutstandingRequests());
+response.put("uptime", stats.getUptime());
-response.put("watch_count", zkdb.getDataTree().getWatchCount());
-response.put("ephemerals_count",
zkdb.getDataTree().getEphemeralsCount());
-response.put("approximate_data_size",
zkdb.getDataTree().cachedApproximateDataSize());
+response.put("server_state", stats.getServerState());
+response.put("znode_count", zkdb.getNodeCount());
-response.put("global_sessions", zkdb.getSessionCount());
-response.put("local_sessions",
-zkServer.getSessionTracker().getLocalSessionCount());
+response.put("watch_count",
zkdb.getDataTree().getWatchCount());
+response.put("ephemerals_count",
zkdb.getDataTree().getEphemeralsCount());
+response.put("approximate_data_size",
zkdb.getDataTree().cachedApproximateDataSize());
-OSMXBean osMbean = new OSMXBean();
-response.put("open_file_descriptor_count",
osMbean.getOpenFileDescriptorCount());
-response.put("max_file_descriptor_count",
osMbean.getMaxFileDescriptorCount());
-response.put("connection_drop_probability",
zkServer.getConnectionDropChance());
+response.put("global_sessions", zkdb.getSessionCount());
+response.put("local_sessions",
+zkServer.getSessionTracker().getLocalSessionCount());
-response.put("last_client_response_size",
stats.getClientResponseStats().getLastBufferSize());
-response.put("max_client_response_size",
stats.getClientResponseStats().getMaxBufferSize());
-response.put("min_client_response_size",
stats.getClientResponseStats().getMinBufferSize());
+response.put("connection_drop_probability",
zkServer.getConnectionDropChance());
-if (zkServer instanceof QuorumZooKeeperServer) {
-QuorumPeer peer = ((QuorumZooKeeperServer) zkServer).self;
-response.put("quorum_size", peer.getQuorumSize());
-}
+response.put("last_client_response_size",
stats.getClientResponseStats().getLastBufferSize());
+response.put("max_client_response_size",
stats.getClientResponseStats().getMaxBufferSize());
+
[GitHub] [zookeeper] enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492366498 ZOOKEEPER-3395 for documentation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav…
enixon commented on issue #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav… URL: https://github.com/apache/zookeeper/pull/948#issuecomment-492361876 retest maven build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
enixon commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492361481 @anmolnar I've performed a basic refactor to capture my understanding of your nit, let me know if that's not what you were thinking :) The documentation of the admin server is a bit lacking at the moment. I'll open a ticket to improve it directly. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #850: ZOOKEEPER-3309: Add sync processor metrics
anmolnar commented on a change in pull request #850: ZOOKEEPER-3309: Add sync
processor metrics
URL: https://github.com/apache/zookeeper/pull/850#discussion_r283857589
##
File path:
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/SyncRequestProcessorMetricTest.java
##
@@ -0,0 +1,90 @@
+/**
+ * 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.quorum;
+
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.metrics.MetricsUtils;
+import org.apache.zookeeper.server.*;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+
+import static org.hamcrest.number.OrderingComparison.greaterThan;
+import static org.hamcrest.number.OrderingComparison.greaterThanOrEqualTo;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class SyncRequestProcessorMetricTest {
+ZooKeeperServer zks;
+
+@Before
+public void setup() throws Exception {
+ZKDatabase db = mock(ZKDatabase.class);
+when(db.append(any(Request.class))).thenReturn(true);
+doAnswer(invocation->{
+Thread.sleep(100);
+return null;
+}).when(db).commit();
+zks = mock(ZooKeeperServer.class);
+when(zks.getZKDatabase()).thenReturn(db);
+}
+
+private Request createRquest(long sessionId, int xid) {
+return new Request(null, sessionId, xid, ZooDefs.OpCode.setData,
+ByteBuffer.wrap(new byte[10]), null);
+}
+
+@Test
+public void testSyncProcessorMetrics() throws Exception{
+SyncRequestProcessor syncProcessor = new SyncRequestProcessor(zks,
null);
+for (int i=0; i<500; i++) {
+syncProcessor.processRequest(createRquest(1, i));
+}
+
+Map values = MetricsUtils.currentServerMetrics();
+Assert.assertEquals(500L, values.get("sync_processor_request_queued"));
+
+syncProcessor.start();
+
+
+Thread.sleep(500);
Review comment:
Sign of a flaky maybe?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #622: [ZOOKEEPER-3145] Fix potential watch missing issue due to stale pzxid when replaying CloseSession txn with fuzzy snapshot
anmolnar commented on issue #622: [ZOOKEEPER-3145] Fix potential watch missing issue due to stale pzxid when replaying CloseSession txn with fuzzy snapshot URL: https://github.com/apache/zookeeper/pull/622#issuecomment-492282970 @lvfangmin This patch broke `org.apache.zookeeper.server.quorum.FuzzySnapshotRelatedTest.testPZxidUpdatedWhenLoadingSnapshot`. Please fix. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size
anmolnar commented on issue #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770#issuecomment-492280588 Committed to master branch. Thanks @enixon ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] asfgit closed pull request #770: ZOOKEEPER-3244: Add option to snapshot based on log size
asfgit closed pull request #770: ZOOKEEPER-3244: Add option to snapshot based on log size URL: https://github.com/apache/zookeeper/pull/770 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #898: ZOOKEEPER-3353: Admin commands for showing initial settings
anmolnar commented on a change in pull request #898: ZOOKEEPER-3353: Admin
commands for showing initial settings
URL: https://github.com/apache/zookeeper/pull/898#discussion_r283848928
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java
##
@@ -62,13 +64,17 @@
/** Maps command names to Command instances */
private static Map commands = new HashMap();
private static Set primaryNames = new HashSet();
+private static Set commandsAvailableBeforePeerSync = new
HashSet();
/**
* Registers the given command. Registered commands can be run by passing
* any of their names to runCommand.
*/
-public static void registerCommand(Command command) {
+public static void registerCommand(Command command, boolean
enabledBeforePeerSync) {
Review comment:
nit: You can overload the constructor to avoid modifying the signature at so
many places.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492276446 Setting up the admin server is already documented. We should only have a detailed list of available admin commands to be on the sunny side. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings
anmolnar commented on issue #898: ZOOKEEPER-3353: Admin commands for showing initial settings URL: https://github.com/apache/zookeeper/pull/898#issuecomment-492275063 @enixon Sorry for the delay. Unfortunately there's no plan to deprecate 4LW commands, but we already have the consensus in the community (as far as I'm concerned). Therefore I don't think it's worth the effort to add new functionality. We should push users towards using Admin commands instead. Documentation-wise I think this is an area that we should improve. It would be great if you could pick this up and put together some initial documentation on ZooKeeper admin commands along with your new patches. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
anmolnar commented on a change in pull request #761: ZOOKEEPER-3237: Allow IPv6
wildcard address in peer config
URL: https://github.com/apache/zookeeper/pull/761#discussion_r283836766
##
File path:
zookeeper-server/src/test/java/org/apache/zookeeper/test/ServerDuplicateTest.java
##
@@ -0,0 +1,76 @@
+/**
+ * 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 org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.server.quorum.QuorumPeer;
+import org.junit.Test;
+
+import java.net.InetSocketAddress;
+
+public class ServerDuplicateTest extends ZKTestCase {
Review comment:
These are unit tests of `QuorumServer` class. Please move them to
`QuorumServerTest.java` file.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on a change in pull request #761: ZOOKEEPER-3237: Allow IPv6 wildcard address in peer config
anmolnar commented on a change in pull request #761: ZOOKEEPER-3237: Allow IPv6
wildcard address in peer config
URL: https://github.com/apache/zookeeper/pull/761#discussion_r283835507
##
File path:
zookeeper-server/src/test/java/org/apache/zookeeper/test/ServerDuplicateTest.java
##
@@ -0,0 +1,76 @@
+/**
+ * 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 org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.server.quorum.QuorumPeer;
+import org.junit.Test;
+
+import java.net.InetSocketAddress;
+
+public class ServerDuplicateTest extends ZKTestCase {
+/**
+ * You don't necessarily write an integration test like this via Reconfig
functionality.
Review comment:
I think you added my comment by mistake. :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar closed pull request #943: Merge pull request #1 from apache/master
anmolnar closed pull request #943: Merge pull request #1 from apache/master URL: https://github.com/apache/zookeeper/pull/943 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #943: Merge pull request #1 from apache/master
anmolnar commented on issue #943: Merge pull request #1 from apache/master URL: https://github.com/apache/zookeeper/pull/943#issuecomment-492265897 I think this has been opened by mistake. Closing. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
nkalmar commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492236889 Hi @JiriOndrusek , Thanks for this patch and looking into this. Unfortunately there is no maven OSGi whatsoever, so it needs to be added. As this is not a regression, I believe rc6 will be rolled out, still short on binding +1 though. So at my current knowledge, target version for this is 3.5.6. But it needs a maven fix for sure. I'll also try to look into it, I'll let you know if I find anything. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
JiriOndrusek commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492213488 @eolivelli Hi, I've checked history of build.xml and it is not a regression. Just an issue, which could be seen only after installing Curator 4.1.1 (may be older) in OSGi. For maven: I haven't found any occurrence of OSGi headers in main pom.xml. I'm not sure whether I just misses them -> so only a little fix would be needed or whether it has to be added -> slightly bigger change. I can look into it later this week, is this time horizon acceptable? (I see in comment above "this will be a -1 on the 3.5.5rc6") This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #622: [ZOOKEEPER-3145] Fix potential watch missing issue due to stale pzxid when replaying CloseSession txn with fuzzy snapshot
anmolnar commented on issue #622: [ZOOKEEPER-3145] Fix potential watch missing issue due to stale pzxid when replaying CloseSession txn with fuzzy snapshot URL: https://github.com/apache/zookeeper/pull/622#issuecomment-492211073 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar edited a comment on issue #946: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (for Curator)
nkalmar edited a comment on issue #946: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (for Curator) URL: https://github.com/apache/zookeeper/pull/946#issuecomment-492154707 Less of an importance on branch 3.4 to fix it on maven (we do not plan to release with maven on this branch), but I agree, we should definitely fix it on master and 3.5. I'm also looking into a maven fix. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #946: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (for Curator)
nkalmar commented on issue #946: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (for Curator) URL: https://github.com/apache/zookeeper/pull/946#issuecomment-492154707 Less of an importance on branch 3.4 to fix it on maven (we do not plan to release with maven on this branch), but I agree, we should definitely fix it on master and 3.5. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492130327 Is this @JiriOndrusek a "regression" ? As you are adding the manifest lines on ANT build I guess this is not a problem introduced by the recent Maven Migration cc @nkalmar @anmolnar This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator)
eolivelli commented on issue #945: [ZOOKEEPER-3389] Zookeeper does not export all required packages in OSGi (needed for curator) URL: https://github.com/apache/zookeeper/pull/945#issuecomment-492130477 Can you please try to add a fix even for the maven build ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] eolivelli commented on a change in pull request #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav…
eolivelli commented on a change in pull request #948: ZOOKEEPER-3394: Delay
observer reconnect when all learner masters hav…
URL: https://github.com/apache/zookeeper/pull/948#discussion_r283656398
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
##
@@ -214,8 +226,16 @@ public void shutdown() {
}
static void waitForReconnectDelay() {
-if (reconnectDelayMs > 0) {
-long randomDelay = (long) (reconnectDelayMs * Math.random());
+Observer.waitForReconnectDelayHelper(reconnectDelayMs);
Review comment:
Nit: no need to call Observer explicitly
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] enixon opened a new pull request #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav…
enixon opened a new pull request #948: ZOOKEEPER-3394: Delay observer reconnect when all learner masters hav… URL: https://github.com/apache/zookeeper/pull/948 …e been tried This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon opened a new pull request #947: ZOOKEEPER-3392: Add admin command to display last snapshot information
enixon opened a new pull request #947: ZOOKEEPER-3392: Add admin command to display last snapshot information URL: https://github.com/apache/zookeeper/pull/947 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #944: ZOOKEEPER-3388: Allow client port to support plaintext and encrypted …
enixon commented on issue #944: ZOOKEEPER-3388: Allow client port to support plaintext and encrypted … URL: https://github.com/apache/zookeeper/pull/944#issuecomment-492012054 I know that @ivmaykov has been experimenting with openssl so you may see him put up a pr in the near future for that. I'm going to remove the boringssl change for now and leave that change to a later commit since it's not strictly necessary for this pr. It is a good idea and I'd like to see the change happen eventually. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #917: ZOOKEEPER-3318:Add a complete backup mechanism for zookeeper internal
enixon commented on issue #917: ZOOKEEPER-3318:Add a complete backup mechanism for zookeeper internal URL: https://github.com/apache/zookeeper/pull/917#issuecomment-491919486 Directed snapshots seem useful to me; I have reservations about snapshot taking being triggered by the ZooKeeper client. If it's just a way to freeze the FinalRequestProcessor and take a synchronous snapshot then is there a way to achieve that while using the admin server or jmx as the entry point? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
