[GitHub] [zookeeper] phunt commented on a change in pull request #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4
phunt commented on a change in pull request #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4 URL: https://github.com/apache/zookeeper/pull/847#discussion_r265428602 ## File path: zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml ## @@ -33,6 +33,9 @@ LogGraph is an application for viewing and filtering zookeeper logs. It can handle transaction logs and message logs. + +9.4.14.v20181114 Review comment: Should we update both of them? :-) 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 commented on a change in pull request #822: ZOOKEEPER-3281:Add a new CLI:watch
maoling commented on a change in pull request #822: ZOOKEEPER-3281:Add a new
CLI:watch
URL: https://github.com/apache/zookeeper/pull/822#discussion_r265391612
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/cli/WatchCommand.java
##
@@ -0,0 +1,129 @@
+/**
+ * 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.cli;
+
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
+import org.apache.commons.cli.Parser;
+import org.apache.commons.cli.PosixParser;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+
+/**
+ * watch command for CLI
+ */
+public class WatchCommand extends CliCommand {
+private static Options options = new Options();
+private String[] args;
+private CommandLine cl;
+
+public WatchCommand() {
+super("watch", "[-d|-c|-e] path");
+
+options.addOption(new Option("d", false, "data changed watch"));
+options.addOption(new Option("c", false, "child changed watch"));
+options.addOption(new Option("e", false, "exist watch"));
+}
+
+@Override
+public CliCommand parse(String[] cmdArgs) throws CliParseException {
+Parser parser = new PosixParser();
+try {
+cl = parser.parse(options, cmdArgs);
+} catch (ParseException ex) {
+throw new CliParseException(ex);
+}
+args = cl.getArgs();
+if (args.length < 2) {
+throw new CliParseException(getUsageStr());
+}
+
+return this;
+}
+
+@Override
+public boolean exec() throws CliException {
+try {
+String path = args[1];
+CountDownLatch latch = new CountDownLatch(1);
+SimpleCLIWatcher cliWatcher = new SimpleCLIWatcher(latch);
+if (cl.hasOption("d") || cl.getOptions().length == 0) {
+zk.getData(path, cliWatcher, null);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+if
(watchedEvent.getType().equals(Watcher.Event.EventType.NodeDataChanged)) {
+byte[] newData = zk.getData(path, false, null);
+out.println("new data:" + new String(newData));
+}
+} else if (cl.hasOption("c")) {
+zk.getChildren(path, cliWatcher);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+if
((watchedEvent.getType().equals(Watcher.Event.EventType.NodeChildrenChanged))) {
+List newChildList = zk.getChildren(path, false);
+out.println("new child list:" + newChildList.toString());
+}
+} else if (cl.hasOption("e")) {
+zk.exists(path, cliWatcher);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+}
+} catch (KeeperException | InterruptedException ex) {
+throw new CliWrapperException(ex);
+}
+
+return false;
+}
+
+private void printWatchedEvent(WatchedEvent watchedEvent) {
+out.println("WatchedEvent state:" + watchedEvent.getState());
+out.println("type:" + watchedEvent.getType());
+out.println("path:" + watchedEvent.getPath());
+}
+
+private static class SimpleCLIWatcher implements Watcher {
+private CountDownLatch latch;
+private WatchedEvent event;
+
+public SimpleCLIWatcher(CountDownLatch latch) {
+this.latch = latch;
+}
+
+public WatchedEvent getEvent() {
+return event;
+}
+
+public void process(WatchedEvent e) {
+if (e.getType() == Event.EventType.None) {
+return;
+}
+if
[jira] [Updated] (ZOOKEEPER-3311) Allow a delay to the transaction log flush
[ https://issues.apache.org/jira/browse/ZOOKEEPER-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated ZOOKEEPER-3311: -- Labels: pull-request-available (was: ) > Allow a delay to the transaction log flush > --- > > Key: ZOOKEEPER-3311 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3311 > Project: ZooKeeper > Issue Type: New Feature > Components: server >Affects Versions: 3.6.0 >Reporter: Brian Nixon >Priority: Minor > Labels: pull-request-available > > The SyncRequestProcessor flushes writes to disk either when 1000 writes are > pending to be flushed or when the processor fails to retrieve another write > from its incoming queue. The "flush when queue empty" condition operates > poorly under many workloads as it can quickly degrade into flushing after > every write -- losing all benefits of batching and leading to a continuous > stream of flushes + fsyncs which overwhelm the underlying disk. > > A configurable flush delay would ensure flushes do not happen more frequently > than once every X milliseconds. This can be used in-place of or jointly with > batch size triggered flushes. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [zookeeper] enixon opened a new pull request #851: ZOOKEEPER-3311: Allow a delay to the transaction log flush
enixon opened a new pull request #851: ZOOKEEPER-3311: Allow a delay to the transaction log flush URL: https://github.com/apache/zookeeper/pull/851 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
[jira] [Created] (ZOOKEEPER-3311) Allow a delay to the transaction log flush
Brian Nixon created ZOOKEEPER-3311: -- Summary: Allow a delay to the transaction log flush Key: ZOOKEEPER-3311 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3311 Project: ZooKeeper Issue Type: New Feature Components: server Affects Versions: 3.6.0 Reporter: Brian Nixon The SyncRequestProcessor flushes writes to disk either when 1000 writes are pending to be flushed or when the processor fails to retrieve another write from its incoming queue. The "flush when queue empty" condition operates poorly under many workloads as it can quickly degrade into flushing after every write -- losing all benefits of batching and leading to a continuous stream of flushes + fsyncs which overwhelm the underlying disk. A configurable flush delay would ensure flushes do not happen more frequently than once every X milliseconds. This can be used in-place of or jointly with batch size triggered flushes. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [zookeeper] enixon commented on a change in pull request #770: ZOOKEEPER-3244: Add option to snapshot based on log size
enixon commented on a change in pull request #770: ZOOKEEPER-3244: Add option
to snapshot based on log size
URL: https://github.com/apache/zookeeper/pull/770#discussion_r265370057
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
##
@@ -873,6 +873,10 @@ public int getGlobalOutstandingLimit() {
return limit;
}
+public static long getSnapSize() {
+return Long.getLong("zookeeper.snapSize", 4294967296L); // 4GB by
default
Review comment:
Excellent point! I've extended the documentation to describe the new
property. Let's leave it on by default and let the community decide if they
want to modify that value.
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 #846: ZOOKEEPER-3302 ZooKeeper C client does not compile on Fedora 29
eolivelli commented on a change in pull request #846: ZOOKEEPER-3302 ZooKeeper
C client does not compile on Fedora 29
URL: https://github.com/apache/zookeeper/pull/846#discussion_r265340602
##
File path: zookeeper-client/zookeeper-client-c/src/cli.c
##
@@ -678,15 +678,15 @@ int main(int argc, char **argv) {
}
if (argc > 2) {
if(strncmp("cmd:",argv[2],4)==0){
Review comment:
Sure.
Honestly I don't know how tests are implement in C projects.
I will take a look at existing tests and try to come back with a test.
Is there any way to run a single C test?
Btw I will check existing docs
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 #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4
eolivelli commented on a change in pull request #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4 URL: https://github.com/apache/zookeeper/pull/847#discussion_r265339393 ## File path: zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml ## @@ -33,6 +33,9 @@ LogGraph is an application for viewing and filtering zookeeper logs. It can handle transaction logs and message logs. + +9.4.14.v20181114 Review comment: It is the same version we have in master @phunt 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 #822: ZOOKEEPER-3281:Add a new CLI:watch
enixon commented on a change in pull request #822: ZOOKEEPER-3281:Add a new
CLI:watch
URL: https://github.com/apache/zookeeper/pull/822#discussion_r265336756
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/cli/WatchCommand.java
##
@@ -0,0 +1,129 @@
+/**
+ * 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.cli;
+
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
+import org.apache.commons.cli.Parser;
+import org.apache.commons.cli.PosixParser;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+
+/**
+ * watch command for CLI
+ */
+public class WatchCommand extends CliCommand {
+private static Options options = new Options();
+private String[] args;
+private CommandLine cl;
+
+public WatchCommand() {
+super("watch", "[-d|-c|-e] path");
+
+options.addOption(new Option("d", false, "data changed watch"));
+options.addOption(new Option("c", false, "child changed watch"));
+options.addOption(new Option("e", false, "exist watch"));
+}
+
+@Override
+public CliCommand parse(String[] cmdArgs) throws CliParseException {
+Parser parser = new PosixParser();
+try {
+cl = parser.parse(options, cmdArgs);
+} catch (ParseException ex) {
+throw new CliParseException(ex);
+}
+args = cl.getArgs();
+if (args.length < 2) {
+throw new CliParseException(getUsageStr());
+}
+
+return this;
+}
+
+@Override
+public boolean exec() throws CliException {
+try {
+String path = args[1];
+CountDownLatch latch = new CountDownLatch(1);
+SimpleCLIWatcher cliWatcher = new SimpleCLIWatcher(latch);
+if (cl.hasOption("d") || cl.getOptions().length == 0) {
+zk.getData(path, cliWatcher, null);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+if
(watchedEvent.getType().equals(Watcher.Event.EventType.NodeDataChanged)) {
+byte[] newData = zk.getData(path, false, null);
+out.println("new data:" + new String(newData));
+}
+} else if (cl.hasOption("c")) {
+zk.getChildren(path, cliWatcher);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+if
((watchedEvent.getType().equals(Watcher.Event.EventType.NodeChildrenChanged))) {
+List newChildList = zk.getChildren(path, false);
+out.println("new child list:" + newChildList.toString());
+}
+} else if (cl.hasOption("e")) {
+zk.exists(path, cliWatcher);
+latch.await();
+WatchedEvent watchedEvent = cliWatcher.getEvent();
+printWatchedEvent(watchedEvent);
+}
+} catch (KeeperException | InterruptedException ex) {
+throw new CliWrapperException(ex);
+}
+
+return false;
+}
+
+private void printWatchedEvent(WatchedEvent watchedEvent) {
+out.println("WatchedEvent state:" + watchedEvent.getState());
+out.println("type:" + watchedEvent.getType());
+out.println("path:" + watchedEvent.getPath());
+}
+
+private static class SimpleCLIWatcher implements Watcher {
+private CountDownLatch latch;
+private WatchedEvent event;
+
+public SimpleCLIWatcher(CountDownLatch latch) {
+this.latch = latch;
+}
+
+public WatchedEvent getEvent() {
+return event;
+}
+
+public void process(WatchedEvent e) {
+if (e.getType() == Event.EventType.None) {
+return;
+}
+if
[GitHub] [zookeeper] phunt commented on a change in pull request #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4
phunt commented on a change in pull request #847: ZOOKEEPER-3304 Maven build of "loggraph" is broken on branch-3.4 URL: https://github.com/apache/zookeeper/pull/847#discussion_r265272360 ## File path: zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml ## @@ -33,6 +33,9 @@ LogGraph is an application for viewing and filtering zookeeper logs. It can handle transaction logs and message logs. + +9.4.14.v20181114 Review comment: Latest jetty is 9.4.15.v20190215 - any reason we wouldn't use that? 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] phunt commented on a change in pull request #846: ZOOKEEPER-3302 ZooKeeper C client does not compile on Fedora 29
phunt commented on a change in pull request #846: ZOOKEEPER-3302 ZooKeeper C
client does not compile on Fedora 29
URL: https://github.com/apache/zookeeper/pull/846#discussion_r265271358
##
File path: zookeeper-client/zookeeper-client-c/src/cli.c
##
@@ -678,15 +678,15 @@ int main(int argc, char **argv) {
}
if (argc > 2) {
if(strncmp("cmd:",argv[2],4)==0){
Review comment:
Possible to extract this out into a method and add a test for 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] phunt commented on a change in pull request #832: ZOOKEEPER-2503:do a hard constraints on the number of myid which must be between 1 and 255
phunt commented on a change in pull request #832: ZOOKEEPER-2503:do a hard
constraints on the number of myid which must be between 1 and 255
URL: https://github.com/apache/zookeeper/pull/832#discussion_r265270476
##
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
##
@@ -721,6 +721,10 @@ private void setupMyId() throws IOException {
throw new IllegalArgumentException("serverid " + myIdString
+ " is not a number");
}
+//myid must be in [0, 255]
+if (serverId < 0 || serverId > 255) {
Review comment:
I believe the recent changes for TTL limit this to 254?
@Randgalt can you weigh in 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] phunt commented on a change in pull request #832: ZOOKEEPER-2503:do a hard constraints on the number of myid which must be between 1 and 255
phunt commented on a change in pull request #832: ZOOKEEPER-2503:do a hard constraints on the number of myid which must be between 1 and 255 URL: https://github.com/apache/zookeeper/pull/832#discussion_r265270041 ## File path: zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md ## @@ -212,7 +212,8 @@ ensemble: consists of a single line containing only the text of that machine's id. So *myid* of server 1 would contain the text "1" and nothing else. The id must be unique within the - ensemble and should have a value between 1 and 255. + ensemble and must have a value between 0 and 255. Review comment: Is there a reason why we are changing the official spec here? (as official as we get at least). Perhaps we should continue to enforce the "don't use zero" rule? Not sure I have a strong feeling either way but this has been on the books since ZOOKEEPER-260 Reserving 0 may be a good idea for something in the future...? 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
Re: [VOTE] Apache ZooKeeper release 3.4.14 candidate 5
+1 - sig/xsum verified, rat ran clean, was able to run various ensemble sizes successfully. Patrick On Wed, Mar 6, 2019 at 10:56 AM Andor Molnar wrote: > This is a bugfix release candidate for 3.4.14. It fixes 8 issues, mostly > build / unit tests issues, > dependency updates flagged by OWASP, NPE and a name resolution problem. > Among these it also supports > experimental Maven build and Markdown based documentation generation. > > The full release notes is available at: > > > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310801=12343587 > > *** Please download, test and vote by March 10th 2019, 23:59 UTC+0. *** > > Source files: > https://dist.apache.org/repos/dist/dev/zookeeper/zookeeper-3.4.14-rc5/ > > Maven staging repo: > > https://repository.apache.org/content/groups/staging/org/apache/zookeeper/zookeeper/3.4.14/ > > The release candidate tag in git to be voted upon: release-3.4.14-rc5 > > ZooKeeper's KEYS file containing PGP keys we use to sign the release: > http://www.apache.org/dist/zookeeper/KEYS > > Should we release this candidate? > > Regards, > Andor > > >
[GitHub] [zookeeper] jhuan31 closed pull request #850: ZOOKEEPER-3309: Add sync processor metrics
jhuan31 closed pull request #850: ZOOKEEPER-3309: Add sync processor metrics URL: https://github.com/apache/zookeeper/pull/850 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] jhuan31 opened a new pull request #850: ZOOKEEPER-3309: Add sync processor metrics
jhuan31 opened a new pull request #850: ZOOKEEPER-3309: Add sync processor metrics URL: https://github.com/apache/zookeeper/pull/850 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
