[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689657#comment-15689657
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89285632
  
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -576,174 +580,174 @@ public void genCsharpCode(File outputDirectory) 
throws IOException {
 } else if (!outputDirectory.isDirectory()) {
 throw new IOException(outputDirectory + " is not a 
directory.");
 }
-File csharpFile = new File(outputDirectory, getName()+".cs");
-FileWriter cs = new FileWriter(csharpFile);
-cs.write("// File generated by hadoop record compiler. Do not 
edit.\n");
-cs.write("/**\n");
-cs.write("* Licensed to the Apache Software Foundation (ASF) under 
one\n");
-cs.write("* or more contributor license agreements.  See the 
NOTICE file\n");
-cs.write("* distributed with this work for additional 
information\n");
-cs.write("* regarding copyright ownership.  The ASF licenses this 
file\n");
-cs.write("* to you under the Apache License, Version 2.0 (the\n");
-cs.write("* \"License\"); you may not use this file except in 
compliance\n");
-cs.write("* with the License.  You may obtain a copy of the 
License at\n");
-cs.write("*\n");
-cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
-cs.write("*\n");
-cs.write("* Unless required by applicable law or agreed to in 
writing, software\n");
-cs.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
-cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either 
express or implied.\n");
-cs.write("* See the License for the specific language governing 
permissions and\n");
-cs.write("* limitations under the License.\n");
-cs.write("*/\n");
-cs.write("\n");
-cs.write("using System;\n");
-cs.write("using Org.Apache.Jute;\n");
-cs.write("\n");
-cs.write("namespace "+getCsharpNameSpace()+"\n");
-cs.write("{\n");
-
-String className = getCsharpName();
-cs.write("public class "+className+" : IRecord, IComparable \n");
-cs.write("{\n");
-cs.write("  public "+ className +"() {\n");
-cs.write("  }\n");
-
-cs.write("  public "+className+"(\n");
-int fIdx = 0;
-int fLen = mFields.size();
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
-cs.write((fLen-1 == fIdx)?"":",\n");
-}
-cs.write(") {\n");
-fIdx = 0;
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
-}
-cs.write("  }\n");
-fIdx = 0;
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpGetSet(fIdx));
+
+try (FileWriter cs = new FileWriter(new File(outputDirectory, 
getName() + ".cs"));) {
+cs.write("// File generated by hadoop record compiler. Do not 
edit.\n");
+cs.write("/**\n");
+cs.write("* Licensed to the Apache Software Foundation (ASF) 
under one\n");
+cs.write("* or more contributor license agreements.  See the 
NOTICE file\n");
+cs.write("* distributed with this work for additional 
information\n");
+cs.write("* regarding copyright ownership.  The ASF licenses 
this file\n");
+cs.write("* to you under the Apache License, Version 2.0 
(the\n");
+cs.write("* \"License\"); you may not use this file except in 
compliance\n");
+cs.write("* with the License.  You may obtain a copy of the 
License at\n");
+cs.write("*\n");
+cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
+cs.write("*\n");
+cs.write("* Unless required by applicable law or agreed to in 
writing, software\n");
+cs.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
+cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, 
either express or implied.\n");
+cs.write("* See the License for the specific language 

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689659#comment-15689659
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89264584
  
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) 
throws IOException {
 } else if (!pkgdir.isDirectory()) {
 throw new IOException(pkgpath + " is not a directory.");
 }
-File jfile = new File(pkgdir, getName()+".java");
-FileWriter jj = new FileWriter(jfile);
-jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
-jj.write("/**\n");
-jj.write("* Licensed to the Apache Software Foundation (ASF) under 
one\n");
-jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
-jj.write("* distributed with this work for additional 
information\n");
-jj.write("* regarding copyright ownership.  The ASF licenses this 
file\n");
-jj.write("* to you under the Apache License, Version 2.0 (the\n");
-jj.write("* \"License\"); you may not use this file except in 
compliance\n");
-jj.write("* with the License.  You may obtain a copy of the 
License at\n");
-jj.write("*\n");
-jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
-jj.write("*\n");
-jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
-jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
-jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either 
express or implied.\n");
-jj.write("* See the License for the specific language governing 
permissions and\n");
-jj.write("* limitations under the License.\n");
-jj.write("*/\n");
-jj.write("\n");
-jj.write("package "+getJavaPackage()+";\n\n");
-jj.write("import org.apache.jute.*;\n");
-jj.write("public class "+getName()+" implements Record {\n");
-for (Iterator i = mFields.iterator(); i.hasNext();) {
-JField jf = i.next();
-jj.write(jf.genJavaDecl());
-}
-jj.write("  public "+getName()+"() {\n");
-jj.write("  }\n");
+try (FileWriter jj = new FileWriter(new File(pkgdir, 
getName()+".java"))) {
+jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
+jj.write("/**\n");
+jj.write("* Licensed to the Apache Software Foundation (ASF) 
under one\n");
+jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
+jj.write("* distributed with this work for additional 
information\n");
+jj.write("* regarding copyright ownership.  The ASF licenses 
this file\n");
+jj.write("* to you under the Apache License, Version 2.0 
(the\n");
+jj.write("* \"License\"); you may not use this file except in 
compliance\n");
+jj.write("* with the License.  You may obtain a copy of the 
License at\n");
+jj.write("*\n");
+jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
+jj.write("*\n");
+jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
+jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
+jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, 
either express or implied.\n");
+jj.write("* See the License for the specific language 
governing permissions and\n");
+jj.write("* limitations under the License.\n");
+jj.write("*/\n");
+jj.write("\n");
+jj.write("package " + getJavaPackage() + ";\n\n");
+jj.write("import org.apache.jute.*;\n");
+jj.write("public class " + getName() + " implements Record 
{\n");
+for (Iterator i = mFields.iterator(); i.hasNext(); ) {
+JField jf = i.next();
+jj.write(jf.genJavaDecl());
+}
+jj.write("  public " + getName() + "() {\n");
+jj.write("  }\n");
 
-jj.write("  public "+getName()+"(\n");
-int fIdx = 0;
-int fLen = mFields.size();
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689660#comment-15689660
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89283922
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java ---
@@ -151,9 +153,13 @@
  */
 public final static int telnetCloseCmd = 0xfff4fffd;
 
-public final static HashMap cmd2String =
+final static HashMap cmd2String =
--- End diff --

If we can remove this `public`, then I think we should. Also agree with the 
consistent `Map` declaration comment.


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689676#comment-15689676
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89287398
  
--- Diff: src/java/test/config/findbugsExcludeFile.xml ---
@@ -144,4 +144,10 @@
 
   
 
+  
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689658#comment-15689658
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89264869
  
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) 
throws IOException {
 } else if (!pkgdir.isDirectory()) {
 throw new IOException(pkgpath + " is not a directory.");
 }
-File jfile = new File(pkgdir, getName()+".java");
-FileWriter jj = new FileWriter(jfile);
-jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
-jj.write("/**\n");
-jj.write("* Licensed to the Apache Software Foundation (ASF) under 
one\n");
-jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
-jj.write("* distributed with this work for additional 
information\n");
-jj.write("* regarding copyright ownership.  The ASF licenses this 
file\n");
-jj.write("* to you under the Apache License, Version 2.0 (the\n");
-jj.write("* \"License\"); you may not use this file except in 
compliance\n");
-jj.write("* with the License.  You may obtain a copy of the 
License at\n");
-jj.write("*\n");
-jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
-jj.write("*\n");
-jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
-jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
-jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either 
express or implied.\n");
-jj.write("* See the License for the specific language governing 
permissions and\n");
-jj.write("* limitations under the License.\n");
-jj.write("*/\n");
-jj.write("\n");
-jj.write("package "+getJavaPackage()+";\n\n");
-jj.write("import org.apache.jute.*;\n");
-jj.write("public class "+getName()+" implements Record {\n");
-for (Iterator i = mFields.iterator(); i.hasNext();) {
-JField jf = i.next();
-jj.write(jf.genJavaDecl());
-}
-jj.write("  public "+getName()+"() {\n");
-jj.write("  }\n");
+try (FileWriter jj = new FileWriter(new File(pkgdir, 
getName()+".java"))) {
+jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
+jj.write("/**\n");
+jj.write("* Licensed to the Apache Software Foundation (ASF) 
under one\n");
+jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
+jj.write("* distributed with this work for additional 
information\n");
+jj.write("* regarding copyright ownership.  The ASF licenses 
this file\n");
+jj.write("* to you under the Apache License, Version 2.0 
(the\n");
+jj.write("* \"License\"); you may not use this file except in 
compliance\n");
+jj.write("* with the License.  You may obtain a copy of the 
License at\n");
+jj.write("*\n");
+jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
+jj.write("*\n");
+jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
+jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
+jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, 
either express or implied.\n");
+jj.write("* See the License for the specific language 
governing permissions and\n");
+jj.write("* limitations under the License.\n");
+jj.write("*/\n");
+jj.write("\n");
+jj.write("package " + getJavaPackage() + ";\n\n");
+jj.write("import org.apache.jute.*;\n");
+jj.write("public class " + getName() + " implements Record 
{\n");
+for (Iterator i = mFields.iterator(); i.hasNext(); ) {
+JField jf = i.next();
+jj.write(jf.genJavaDecl());
+}
+jj.write("  public " + getName() + "() {\n");
+jj.write("  }\n");
 
-jj.write("  public "+getName()+"(\n");
-int fIdx = 0;
-int fLen = mFields.size();
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689656#comment-15689656
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89284431
  
--- Diff: src/java/test/config/findbugsExcludeFile.xml ---
@@ -144,4 +144,10 @@
 
   
 
+  
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689655#comment-15689655
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89283699
  
--- Diff: src/java/main/org/apache/zookeeper/cli/DeleteCommand.java ---
@@ -56,14 +56,7 @@ public CliCommand parse(String[] cmdArgs) throws 
CliParseException {
 }
 
 private void retainCompatibility(String[] cmdArgs) throws 
CliParseException {
-// delete path [version]
 if (args.length > 2) {
-// rewrite to option
-String [] newCmd = new String[4];
--- End diff --

Why are we removing this rewrite?


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15689664#comment-15689664
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/102
  
@hanm I've left a few comments, but in general looks pretty good. I noticed 
that there are also unaddressed comments from @lvfangmin and @eribeiro . Could 
you take care of those so that we can check this in, please?


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690710#comment-15690710
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/102
  
@fpj Thank you for taking time to double check the patch.

>> I noticed that there are also unaddressed comments from @lvfangmin and 
@eribeiro .
The comments were about using Interface type instead of implementation 
type, and I addressed the comments by creating ZOOKEEPER-2630 because this 
issue is not a regression and is not specifically about findbug warnings, and I 
think it might be better to address them separately.

I've left replies on your other comments in PR, please take a look.


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690676#comment-15690676
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89356018
  
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -576,174 +580,174 @@ public void genCsharpCode(File outputDirectory) 
throws IOException {
 } else if (!outputDirectory.isDirectory()) {
 throw new IOException(outputDirectory + " is not a 
directory.");
 }
-File csharpFile = new File(outputDirectory, getName()+".cs");
-FileWriter cs = new FileWriter(csharpFile);
-cs.write("// File generated by hadoop record compiler. Do not 
edit.\n");
-cs.write("/**\n");
-cs.write("* Licensed to the Apache Software Foundation (ASF) under 
one\n");
-cs.write("* or more contributor license agreements.  See the 
NOTICE file\n");
-cs.write("* distributed with this work for additional 
information\n");
-cs.write("* regarding copyright ownership.  The ASF licenses this 
file\n");
-cs.write("* to you under the Apache License, Version 2.0 (the\n");
-cs.write("* \"License\"); you may not use this file except in 
compliance\n");
-cs.write("* with the License.  You may obtain a copy of the 
License at\n");
-cs.write("*\n");
-cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
-cs.write("*\n");
-cs.write("* Unless required by applicable law or agreed to in 
writing, software\n");
-cs.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
-cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either 
express or implied.\n");
-cs.write("* See the License for the specific language governing 
permissions and\n");
-cs.write("* limitations under the License.\n");
-cs.write("*/\n");
-cs.write("\n");
-cs.write("using System;\n");
-cs.write("using Org.Apache.Jute;\n");
-cs.write("\n");
-cs.write("namespace "+getCsharpNameSpace()+"\n");
-cs.write("{\n");
-
-String className = getCsharpName();
-cs.write("public class "+className+" : IRecord, IComparable \n");
-cs.write("{\n");
-cs.write("  public "+ className +"() {\n");
-cs.write("  }\n");
-
-cs.write("  public "+className+"(\n");
-int fIdx = 0;
-int fLen = mFields.size();
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
-cs.write((fLen-1 == fIdx)?"":",\n");
-}
-cs.write(") {\n");
-fIdx = 0;
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
-}
-cs.write("  }\n");
-fIdx = 0;
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-cs.write(jf.genCsharpGetSet(fIdx));
+
+try (FileWriter cs = new FileWriter(new File(outputDirectory, 
getName() + ".cs"));) {
+cs.write("// File generated by hadoop record compiler. Do not 
edit.\n");
+cs.write("/**\n");
+cs.write("* Licensed to the Apache Software Foundation (ASF) 
under one\n");
+cs.write("* or more contributor license agreements.  See the 
NOTICE file\n");
+cs.write("* distributed with this work for additional 
information\n");
+cs.write("* regarding copyright ownership.  The ASF licenses 
this file\n");
+cs.write("* to you under the Apache License, Version 2.0 
(the\n");
+cs.write("* \"License\"); you may not use this file except in 
compliance\n");
+cs.write("* with the License.  You may obtain a copy of the 
License at\n");
+cs.write("*\n");
+cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
+cs.write("*\n");
+cs.write("* Unless required by applicable law or agreed to in 
writing, software\n");
+cs.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
+cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, 
either express or implied.\n");
+cs.write("* See the License for the specific language 

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690675#comment-15690675
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/102#discussion_r89355986
  
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) 
throws IOException {
 } else if (!pkgdir.isDirectory()) {
 throw new IOException(pkgpath + " is not a directory.");
 }
-File jfile = new File(pkgdir, getName()+".java");
-FileWriter jj = new FileWriter(jfile);
-jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
-jj.write("/**\n");
-jj.write("* Licensed to the Apache Software Foundation (ASF) under 
one\n");
-jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
-jj.write("* distributed with this work for additional 
information\n");
-jj.write("* regarding copyright ownership.  The ASF licenses this 
file\n");
-jj.write("* to you under the Apache License, Version 2.0 (the\n");
-jj.write("* \"License\"); you may not use this file except in 
compliance\n");
-jj.write("* with the License.  You may obtain a copy of the 
License at\n");
-jj.write("*\n");
-jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
-jj.write("*\n");
-jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
-jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
-jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either 
express or implied.\n");
-jj.write("* See the License for the specific language governing 
permissions and\n");
-jj.write("* limitations under the License.\n");
-jj.write("*/\n");
-jj.write("\n");
-jj.write("package "+getJavaPackage()+";\n\n");
-jj.write("import org.apache.jute.*;\n");
-jj.write("public class "+getName()+" implements Record {\n");
-for (Iterator i = mFields.iterator(); i.hasNext();) {
-JField jf = i.next();
-jj.write(jf.genJavaDecl());
-}
-jj.write("  public "+getName()+"() {\n");
-jj.write("  }\n");
+try (FileWriter jj = new FileWriter(new File(pkgdir, 
getName()+".java"))) {
+jj.write("// File generated by hadoop record compiler. Do not 
edit.\n");
+jj.write("/**\n");
+jj.write("* Licensed to the Apache Software Foundation (ASF) 
under one\n");
+jj.write("* or more contributor license agreements.  See the 
NOTICE file\n");
+jj.write("* distributed with this work for additional 
information\n");
+jj.write("* regarding copyright ownership.  The ASF licenses 
this file\n");
+jj.write("* to you under the Apache License, Version 2.0 
(the\n");
+jj.write("* \"License\"); you may not use this file except in 
compliance\n");
+jj.write("* with the License.  You may obtain a copy of the 
License at\n");
+jj.write("*\n");
+jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n;);
+jj.write("*\n");
+jj.write("* Unless required by applicable law or agreed to in 
writing, software\n");
+jj.write("* distributed under the License is distributed on an 
\"AS IS\" BASIS,\n");
+jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, 
either express or implied.\n");
+jj.write("* See the License for the specific language 
governing permissions and\n");
+jj.write("* limitations under the License.\n");
+jj.write("*/\n");
+jj.write("\n");
+jj.write("package " + getJavaPackage() + ";\n\n");
+jj.write("import org.apache.jute.*;\n");
+jj.write("public class " + getName() + " implements Record 
{\n");
+for (Iterator i = mFields.iterator(); i.hasNext(); ) {
+JField jf = i.next();
+jj.write(jf.genJavaDecl());
+}
+jj.write("  public " + getName() + "() {\n");
+jj.write("  }\n");
 
-jj.write("  public "+getName()+"(\n");
-int fIdx = 0;
-int fLen = mFields.size();
-for (Iterator i = mFields.iterator(); i.hasNext(); fIdx++) 
{
-JField jf = i.next();
-

[jira] [Commented] (ZOOKEEPER-2636) Fix C build break.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15694257#comment-15694257
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2636:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/115

ZOOKEEPER-2636: Fix C client build break.

JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2636

Fix build break by reverting the changes made to JRecord.genCCode in 
ZOOKEEPER-2686, which prematurely closed the file writers that prevents 
zookeeper.jute.h and zookeeper.jute.c files from complete generation.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2636

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/115.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #115


commit 8b2df1dca353946cd265c217e4010d3f080943ba
Author: Michael Han 
Date:   2016-11-24T21:32:16Z

Fix C client build break caused by ZOOKEEPER-2628.




> Fix C build break.
> --
>
> Key: ZOOKEEPER-2636
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2636
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: jute
>Affects Versions: 3.5.3
>Reporter: Michael Han
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3, 3.6.0
>
>
> C client build is broken after ZOOKEEPER-2628 is merged in. After a little 
> debug, I find out that the build is broken because the zookeeper.jute.h and 
> zookeeper.jute.c are not completely generated. 
> * The culprit is the code change introduced in ZOOKEEPER-2628, where we wraps 
> {code}JRecord.genCCode{code} with a try / catch / finally block and the file 
> writers were prematurely closed in finally block which prevents remaining of 
> the zookeeper.jute.h/c file being generated. 
> * This fix is made because a find bug warning was directly associated with 
> the code. Due to the subtlety of the file writer ownership, we did not 
> capture the issue during code review. 
> * The build break was not captured in pre-commit builds as well ([an 
> example|https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/72//console]),
>  where we get all tests passed including C client tests. I suspect we might 
> have another bug with cached generated files that should be regenerated but 
> we don't - need more investigation on this one.
> * The fix is simple by revert the change to this specific method. Findbug 
> does not complain anymore because the previous warning that appertain to this 
> code block was fixed at the call site of {code}JRecord.genCCode{code}. So by 
> reverting the change we still have zero find bug warnings.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2636) Fix C build break.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15694951#comment-15694951
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2636:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/115


> Fix C build break.
> --
>
> Key: ZOOKEEPER-2636
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2636
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: jute
>Affects Versions: 3.5.3
>Reporter: Michael Han
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3, 3.6.0
>
>
> C client build is broken after ZOOKEEPER-2628 is merged in. After a little 
> debug, I find out that the build is broken because the zookeeper.jute.h and 
> zookeeper.jute.c are not completely generated. 
> * The culprit is the code change introduced in ZOOKEEPER-2628, where we wraps 
> {code}JRecord.genCCode{code} with a try / catch / finally block and the file 
> writers were prematurely closed in finally block which prevents remaining of 
> the zookeeper.jute.h/c file being generated. 
> * The fix to {code}JRecord.genCCode{code} in ZOOKEEPER-2628 was made because 
> a find bug warning was directly associated with the code. Due to the subtlety 
> of the file writer ownership, we did not capture the issue during code 
> review. 
> * The build break was not captured in pre-commit builds as well ([an 
> example|https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/72//console]),
>  where we get all tests passed including C client tests. I suspect we might 
> have another bug with cached generated files that should be regenerated but 
> we don't - need more investigation on this one.
> * The fix is simple by revert the change to this specific method. Findbug 
> does not complain anymore because the previous warning that appertain to this 
> code block was fixed at the call site of {code}JRecord.genCCode{code}. So by 
> reverting the change we still have zero find bug warnings.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-390) zookeeper url scheme

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-390?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701753#comment-15701753
 ] 

ASF GitHub Bot commented on ZOOKEEPER-390:
--

Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-library/pull/78
  
LGTM; merging.
(p.s. https://issues.apache.org/jira/browse/ZOOKEEPER-390 contains a little 
bit of info about the `zk` url scheme.)


> zookeeper url scheme
> 
>
> Key: ZOOKEEPER-390
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-390
> Project: ZooKeeper
>  Issue Type: Improvement
>Reporter: Benjamin Reed
>
> we need a URL scheme for zookeeper. i think such a scheme could encode 
> various forms of server lists as well as chroot.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2632) Add option to inform JIRA_PASSWORD at CLI prompt

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15693645#comment-15693645
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2632:
---

Github user eribeiro closed the pull request at:

https://github.com/apache/zookeeper/pull/103


> Add option to inform JIRA_PASSWORD at CLI prompt 
> -
>
> Key: ZOOKEEPER-2632
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2632
> Project: ZooKeeper
>  Issue Type: Improvement
>Reporter: Edward Ribeiro
>Assignee: Edward Ribeiro
>Priority: Trivial
> Fix For: 3.6.0
>
>
> Adds the option to prompt for the JIRA password if JIRA_USERNAME is set, but 
> JIRA_PASSWORD is not. Also, asks if the user wants to continue the merge 
> process if the python jira lib is not installed.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15693655#comment-15693655
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/102


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1932) org.apache.zookeeper.test.LETest.testLE fails once in a while

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701070#comment-15701070
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1932:
---

Github user lvfangmin commented on the issue:

https://github.com/apache/zookeeper/pull/106
  
Since we're deprecating the LeaderElection, should we also remove 
LeaderElection.java itself? Any reason we keep it there even no code is 
referencing it?


> org.apache.zookeeper.test.LETest.testLE fails once in a while
> -
>
> Key: ZOOKEEPER-1932
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1932
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: leaderElection
>Affects Versions: 3.5.0
>Reporter: Michi Mutsuzaki
>Assignee: Michael Han
> Fix For: 3.6.0
>
> Attachments: TEST-org.apache.zookeeper.test.LETest.txt, 
> ZOOKEEPER-1932.patch, ZOOKEEPER-1932.patch
>
>
> org.apache.zookeeper.test.LETest.testLE is failing on trunk once in a while. 
> I'm not able to reproduce the failure on my box. I looked at the log, but I 
> couldn't quite figure out what's going on. 
> https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/2315/testReport/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2574) PurgeTxnLog can inadvertently delete required txn log files

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701054#comment-15701054
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2574:
---

Github user lvfangmin commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/111#discussion_r89722055
  
--- Diff: src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java ---
@@ -108,9 +141,11 @@ public boolean accept(File f){
 // remove the old files
 for(File f: files)
 {
-System.out.println("Removing file: "+
+final String msg = "Removing file: "+
 DateFormat.getDateTimeInstance().format(f.lastModified())+
-"\t"+f.getPath());
+"\t"+f.getPath();
+LOG.info(msg);
+System.out.println(msg);
--- End diff --

do we need to keep both system.out and log4j logging? 


> PurgeTxnLog can inadvertently delete required txn log files
> ---
>
> Key: ZOOKEEPER-2574
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2574
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.7, 3.4.8, 3.5.0, 3.5.1, 3.5.2
> Environment: Zookeeper 3.4.8, standalone, and 3-server quorum
>Reporter: Abhishek Rai
>Assignee: Abhishek Rai
> Fix For: 3.4.10, 3.5.3
>
> Attachments: ZOOKEEPER-2574.2.patch, ZOOKEEPER-2574.3.patch, 
> ZOOKEEPER-2574.4.patch, ZOOKEEPER-2574.5.patch, ZOOKEEPER-2574.6.patch, 
> ZOOKEEPER-2574.patch
>
>
> As part of the fix for ZOOKEEPER-1797, the call to 
> FileTxnSnapLog.getSnapshotLogs() was removed from PurgeTxnLog.java.  As a 
> result, some old-looking but required txn log files can be deleted, resulting 
> in data corruption or loss.
> For example, consider the following:
> 1. Configuration:
> autopurge.snapRetainCount=3
> 2. Following files exist:
> log.100 spans transactions from zxid=100 till zxid=140 (inclusive)
> snapshot.110 - snapshot as of zxid=110
> snapshot.120 - snapshot as of zxid=120
> snapshot.130 - snapshot as of zxid=130
> Above scenario is possible when snapshotting has happened multiple times but 
> without accompanying log rollover, which is possible if the server was 
> running as a learner.
> 3. PurgeTxnLog retains all snapshots but deletes log.100 because its zxid is 
> older than the zxid of the oldest snapshot (110).  This results in loss of 
> transactions in the range 131-140.
> Before the fix for ZOOKEEPER-1797, this was avoided by the call to 
> FileTxnSnapLog.getSnapshotLogs() which finds and retains the newest txn log 
> file with starting zxid < oldest retained snapshot's highest zxid.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2574) PurgeTxnLog can inadvertently delete required txn log files

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701053#comment-15701053
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2574:
---

Github user lvfangmin commented on the issue:

https://github.com/apache/zookeeper/pull/111
  
minor comments, others look good to me.


> PurgeTxnLog can inadvertently delete required txn log files
> ---
>
> Key: ZOOKEEPER-2574
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2574
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.7, 3.4.8, 3.5.0, 3.5.1, 3.5.2
> Environment: Zookeeper 3.4.8, standalone, and 3-server quorum
>Reporter: Abhishek Rai
>Assignee: Abhishek Rai
> Fix For: 3.4.10, 3.5.3
>
> Attachments: ZOOKEEPER-2574.2.patch, ZOOKEEPER-2574.3.patch, 
> ZOOKEEPER-2574.4.patch, ZOOKEEPER-2574.5.patch, ZOOKEEPER-2574.6.patch, 
> ZOOKEEPER-2574.patch
>
>
> As part of the fix for ZOOKEEPER-1797, the call to 
> FileTxnSnapLog.getSnapshotLogs() was removed from PurgeTxnLog.java.  As a 
> result, some old-looking but required txn log files can be deleted, resulting 
> in data corruption or loss.
> For example, consider the following:
> 1. Configuration:
> autopurge.snapRetainCount=3
> 2. Following files exist:
> log.100 spans transactions from zxid=100 till zxid=140 (inclusive)
> snapshot.110 - snapshot as of zxid=110
> snapshot.120 - snapshot as of zxid=120
> snapshot.130 - snapshot as of zxid=130
> Above scenario is possible when snapshotting has happened multiple times but 
> without accompanying log rollover, which is possible if the server was 
> running as a learner.
> 3. PurgeTxnLog retains all snapshots but deletes log.100 because its zxid is 
> older than the zxid of the oldest snapshot (110).  This results in loss of 
> transactions in the range 131-140.
> Before the fix for ZOOKEEPER-1797, this was avoided by the call to 
> FileTxnSnapLog.getSnapshotLogs() which finds and retains the newest txn log 
> file with starting zxid < oldest retained snapshot's highest zxid.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2574) PurgeTxnLog can inadvertently delete required txn log files

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701055#comment-15701055
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2574:
---

Github user lvfangmin commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/111#discussion_r89721901
  
--- Diff: docs/zookeeperAdmin.html ---
@@ -1201,8 +1205,10 @@ Configuration Parameters
 (Java system property: zookeeper.snapCount)
 ZooKeeper logs transactions to a transaction
   log. After snapCount transactions are written to a log
-  file a snapshot is started and a new transaction log
-  file is created. The default snapCount is
+  file a snapshot is started. It also influences rollover
+ of the current transaction log to a new file. However,
--- End diff --

keep indention?


> PurgeTxnLog can inadvertently delete required txn log files
> ---
>
> Key: ZOOKEEPER-2574
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2574
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.7, 3.4.8, 3.5.0, 3.5.1, 3.5.2
> Environment: Zookeeper 3.4.8, standalone, and 3-server quorum
>Reporter: Abhishek Rai
>Assignee: Abhishek Rai
> Fix For: 3.4.10, 3.5.3
>
> Attachments: ZOOKEEPER-2574.2.patch, ZOOKEEPER-2574.3.patch, 
> ZOOKEEPER-2574.4.patch, ZOOKEEPER-2574.5.patch, ZOOKEEPER-2574.6.patch, 
> ZOOKEEPER-2574.patch
>
>
> As part of the fix for ZOOKEEPER-1797, the call to 
> FileTxnSnapLog.getSnapshotLogs() was removed from PurgeTxnLog.java.  As a 
> result, some old-looking but required txn log files can be deleted, resulting 
> in data corruption or loss.
> For example, consider the following:
> 1. Configuration:
> autopurge.snapRetainCount=3
> 2. Following files exist:
> log.100 spans transactions from zxid=100 till zxid=140 (inclusive)
> snapshot.110 - snapshot as of zxid=110
> snapshot.120 - snapshot as of zxid=120
> snapshot.130 - snapshot as of zxid=130
> Above scenario is possible when snapshotting has happened multiple times but 
> without accompanying log rollover, which is possible if the server was 
> running as a learner.
> 3. PurgeTxnLog retains all snapshots but deletes log.100 because its zxid is 
> older than the zxid of the oldest snapshot (110).  This results in loss of 
> transactions in the range 131-140.
> Before the fix for ZOOKEEPER-1797, this was avoided by the call to 
> FileTxnSnapLog.getSnapshotLogs() which finds and retains the newest txn log 
> file with starting zxid < oldest retained snapshot's highest zxid.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1525) Plumb ZooKeeperServer object into auth plugins

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1525?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15669468#comment-15669468
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1525:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/84


> Plumb ZooKeeperServer object into auth plugins
> --
>
> Key: ZOOKEEPER-1525
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1525
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.5.0
>Reporter: Warren Turkal
>Assignee: Jordan Zimmerman
> Fix For: 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch
>
>
> I want to plumb the ZooKeeperServer object into the auth plugins so that I 
> can store authentication data in zookeeper itself. With access to the 
> ZooKeeperServer object, I also have access to the ZKDatabase and can look up 
> entries in the local copy of the zookeeper data.
> In order to implement this, I make sure that a ZooKeeperServer instance is 
> passed in to the ProviderRegistry.initialize() method. Then initialize() will 
> try to find a constructor for the AuthenticationProvider that takes a 
> ZooKeeperServer instance. If the constructor is found, it will be used. 
> Otherwise, initialize() will look for a constructor that takes no arguments 
> and use that instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-761) Remove *synchronous* calls from the *single-threaded* C clieant API, since they are documented not to work

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15669747#comment-15669747
 ] 

ASF GitHub Bot commented on ZOOKEEPER-761:
--

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/90#discussion_r88183747
  
--- Diff: src/c/src/zookeeper.c ---
@@ -4052,6 +3958,76 @@ int zoo_amulti(zhandle_t *zh, int count, const 
zoo_op_t *ops,
 return (rc < 0) ? ZMARSHALLINGERROR : ZOK;
 }
 
+
+int zoo_aremove_watchers(zhandle_t *zh, const char *path, ZooWatcherType 
wtype,
+watcher_fn watcher, void *watcherCtx, int local,
+void_completion_t *completion, const void *data)
+{
+char *server_path = prepend_string(zh, path);
+int rc;
+struct oarchive *oa;
+struct RequestHeader h = { get_xid(), ZOO_REMOVE_WATCHES };
+struct RemoveWatchesRequest req =  { (char*)server_path, wtype };
+watcher_deregistration_t *wdo;
+
+if (!zh || !isValidPath(server_path, 0)) {
+rc = ZBADARGUMENTS;
+goto done;
+}
+
+if (!local && is_unrecoverable(zh)) {
+rc = ZINVALIDSTATE;
+goto done;
+}
+
+if (!pathHasWatcher(zh, server_path, wtype, watcher, watcherCtx)) {
+rc = ZNOWATCHER;
+goto done;
+}
+
+if (local) {
+removeWatchers(zh, server_path, wtype, watcher, watcherCtx);
+#ifdef THREADED
+notify_sync_completion((struct sync_completion *)data);
--- End diff --

@breed @fpj btw -- sorry for the confusing code. `zoo_aremove_watchers` is 
sui generis given that it's the only public method that can return `ZOK` 
without scheduling a remote call (for which then, the callback would be 
naturally dispatched). Thus, this horrible hack of calling 
`notify_sync_completion()`. 


> Remove *synchronous* calls from the *single-threaded* C clieant API, since 
> they are documented not to work
> --
>
> Key: ZOOKEEPER-761
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-761
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: c client
>Affects Versions: 3.1.1, 3.2.2
> Environment: RHEL 4u8 (Linux).  The issue is not OS-specific though.
>Reporter: Jozef Hatala
>Assignee: Benjamin Reed
>Priority: Minor
> Fix For: 3.5.3, 3.6.0
>
> Attachments: fix-sync-apis-in-st-adaptor.patch, 
> fix-sync-apis-in-st-adaptor.v2.patch
>
>
> Since the synchronous calls are 
> [known|http://hadoop.apache.org/zookeeper/docs/current/zookeeperProgrammers.html#Using+the+C+Client]
>  to be unimplemented in the single threaded version of the client library 
> libzookeeper_st.so, I believe that it would be helpful towards users of the 
> library if that information was also obvious from the header file.
> Anecdotally more than one of us here made the mistake of starting by using 
> the synchronous calls with the single-threaded library, and we found 
> ourselves debugging it.  An early warning would have been greatly appreciated.
> 1. Could you please add warnings to the doxygen blocks of all synchronous 
> calls saying that they are not available in the single-threaded API.  This 
> cannot be safely done with {{#ifdef THREADED}}, obviously, because the same 
> header file is included whichever client library implementation one is 
> compiling for.
> 2. Could you please bracket the implementation of all synchronous calls in 
> zookeeper.c with {{#ifdef THREADED}} and {{#endif}}, so that those symbols 
> are not present in libzookeeper_st.so?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2460) Remove javacc dependency from public Maven pom

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15677800#comment-15677800
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2460:
---

Github user eolivelli commented on the issue:

https://github.com/apache/zookeeper/pull/76
  
Now can zookeeper accepts PRs what should I do  to make the QA bot pick 
this one?


> Remove javacc dependency from public Maven pom
> --
>
> Key: ZOOKEEPER-2460
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2460
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.5.2
>Reporter: Enrico Olivelli
>Priority: Critical
>
> during the vote of 3.5.2-ALPHA RC 0 we found a Maven dependency to javacc in 
> published pom for zookeeper
> {code}
> 
> net.java.dev.javacc
> javacc
> 5.0compile
> 
> {code}
> this dependency appears not to be useful and should be removed
> this was the tested pom: 
> https://repository.apache.org/content/groups/staging/org/apache/zookeeper/zookeeper/3.5.2-alpha/zookeeper-3.5.2-alpha.pom



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2460) Remove javacc dependency from public Maven pom

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15677908#comment-15677908
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2460:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/76
  
Any update to the PR (including close - reopen) should trigger a pre-commit 
build.


> Remove javacc dependency from public Maven pom
> --
>
> Key: ZOOKEEPER-2460
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2460
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.5.2
>Reporter: Enrico Olivelli
>Priority: Critical
>
> during the vote of 3.5.2-ALPHA RC 0 we found a Maven dependency to javacc in 
> published pom for zookeeper
> {code}
> 
> net.java.dev.javacc
> javacc
> 5.0compile
> 
> {code}
> this dependency appears not to be useful and should be removed
> this was the tested pom: 
> https://repository.apache.org/content/groups/staging/org/apache/zookeeper/zookeeper/3.5.2-alpha/zookeeper-3.5.2-alpha.pom



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2633) Build failure in contrib/zkfuse with gcc 6.x

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2633?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15677980#comment-15677980
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2633:
---

GitHub user ronin13 opened a pull request:

https://github.com/apache/zookeeper/pull/110

ZOOKEEPER-2633: contrib/zkfuse build fix with gcc 6.2.

The build fails in two places: 
https://gist.github.com/ronin13/3e08569dd6c69bf2ad92fa39fa85f7ee

One is boost related, and other is due to false being passed where NULL is
required.

JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2633

Author: Raghavendra Prabhu 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ronin13/zookeeper c++-build-fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/110.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #110


commit 726a8eda08e4022fcbcb0581ec2650e07e39910b
Author: Raghavendra Prabhu 
Date:   2016-11-16T01:06:00Z

ZOOKEEPER-2633: contrib/zkfuse build fix with gcc 6.2.

The build fails in two places: 
https://gist.github.com/ronin13/3e08569dd6c69bf2ad92fa39fa85f7ee

One is boost related, and other is due to false being passed where NULL is
required.

JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2633

Author: Raghavendra Prabhu 




> Build failure in contrib/zkfuse with gcc 6.x
> 
>
> Key: ZOOKEEPER-2633
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2633
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: contrib-zkfuse
> Environment: gcc --version
> gcc (GCC) 6.2.1 20160830
> Copyright (C) 2016 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> g++ --version
> g++ (GCC) 6.2.1 20160830
> Copyright (C) 2016 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> CFLAGS, CXXFLAGS, and LDFLAGS are unset, hence default.
> uname -a
> Linux lative 4.8.8-1-ARCH #1 SMP PREEMPT Tue Nov 15 08:25:24 CET 2016 x86_64 
> GNU/Linux
>Reporter: Raghavendra Prabhu
>Priority: Minor
>
> The build in contrib/zkfuse fails with
> {noformat}
> make
> (CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh 
> /home/raghu/zookeeper/src/contrib/zkfuse/missing autoheader)
> rm -f stamp-h1
> touch config.h.in
> cd . && /bin/sh ./config.status config.h
> config.status: creating config.h
> config.status: config.h is unchanged
> make  all-recursive
> make[1]: Entering directory '/home/raghu/zookeeper/src/contrib/zkfuse'
> Making all in src
> make[2]: Entering directory '/home/raghu/zookeeper/src/contrib/zkfuse/src'
> g++ -DHAVE_CONFIG_H -I. -I..
> -I/home/raghu/zookeeper/src/contrib/zkfuse/../../c/include 
> -I/home/raghu/zookeeper/src/contrib/zkfuse/../../c/generated -I../include 
> -I/usr/include -D_FILE_OFFSET_BITS=64 -D_REENTRANT -march=x86-64 
> -mtune=generic -O2 -pipe -fstack-protector-strong -MT zkfuse.o -MD -MP -MF 
> .deps/zkfuse.Tpo -c -o zkfuse.o zkfuse.cc
> g++ -DHAVE_CONFIG_H -I. -I..
> -I/home/raghu/zookeeper/src/contrib/zkfuse/../../c/include 
> -I/home/raghu/zookeeper/src/contrib/zkfuse/../../c/generated -I../include 
> -I/usr/include -D_FILE_OFFSET_BITS=64 -D_REENTRANT -march=x86-64 
> -mtune=generic -O2 -pipe -fstack-protector-strong -MT zkadapter.o -MD -MP -MF 
> .deps/zkadapter.Tpo -c -o zkadapter.o zkadapter.cc
> In file included from zkadapter.h:34:0,
>  from zkadapter.cc:24:
> event.h:216:9: error: reference to ‘shared_ptr’ is ambiguous
>  shared_ptr m_eventWrapper;
>  ^~
> In file included from /usr/include/boost/throw_exception.hpp:42:0,
>  from /usr/include/boost/smart_ptr/shared_ptr.hpp:27,
>  from /usr/include/boost/shared_ptr.hpp:17,
>  from event.h:30,
>  from zkadapter.h:34,
>  from zkadapter.cc:24:
> /usr/include/boost/exception/exception.hpp:148:11: note: candidates are: 
> template class boost::shared_ptr
>  class shared_ptr;
>^~
> In file included from /usr/include/c++/6.2.1/bits/shared_ptr.h:52:0,
>  from /usr/include/c++/6.2.1/memory:82,
>  from /usr/include/boost/config/no_tr1/memory.hpp:21,
>  from /usr/include/boost/smart_ptr/shared_ptr.hpp:23,
>  from 

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15675758#comment-15675758
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user rakeshadr commented on the issue:

https://github.com/apache/zookeeper/pull/98
  
@eribeiro ,, appreciate your feedback. Thanks!


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15675912#comment-15675912
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user rakeshadr commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r88604035
  
--- Diff: src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java 
---
@@ -36,7 +38,30 @@
 /***/
 @Test
 public void testMajQuorums() throws Throwable {
-   
+LOG.info("Verify QuorumPeer#electionTimeTaken jmx bean attribute");
+
+ArrayList peers = getPeerList();
+for (int i = 1; i <= peers.size(); i++) {
+QuorumPeer qp = peers.get(i - 1);
+Long electionTimeTaken = -1L;
+String bean = "";
+if (qp.getPeerState() == ServerState.FOLLOWING) {
+bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + 
i
++ ",name1=replica." + i + ",name2=Follower";
--- End diff --

Thanks @rgs1, I've have modified the test code as per your suggestion. 
Please take another look at the new commits.


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15675882#comment-15675882
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r88602842
  
--- Diff: src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java 
---
@@ -36,7 +38,30 @@
 /***/
 @Test
 public void testMajQuorums() throws Throwable {
-   
+LOG.info("Verify QuorumPeer#electionTimeTaken jmx bean attribute");
+
+ArrayList peers = getPeerList();
+for (int i = 1; i <= peers.size(); i++) {
+QuorumPeer qp = peers.get(i - 1);
+Long electionTimeTaken = -1L;
+String bean = "";
+if (qp.getPeerState() == ServerState.FOLLOWING) {
+bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + 
i
++ ",name1=replica." + i + ",name2=Follower";
--- End diff --

hyper nit: `String.format()` reads better than `+`


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15675883#comment-15675883
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user rgs1 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r88602850
  
--- Diff: src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java 
---
@@ -36,7 +38,30 @@
 /***/
 @Test
 public void testMajQuorums() throws Throwable {
-   
+LOG.info("Verify QuorumPeer#electionTimeTaken jmx bean attribute");
+
+ArrayList peers = getPeerList();
+for (int i = 1; i <= peers.size(); i++) {
+QuorumPeer qp = peers.get(i - 1);
+Long electionTimeTaken = -1L;
+String bean = "";
+if (qp.getPeerState() == ServerState.FOLLOWING) {
+bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + 
i
++ ",name1=replica." + i + ",name2=Follower";
+electionTimeTaken = (Long) JMXEnv.ensureBeanAttribute(bean,
+"ElectionTimeTaken");
+Assert.assertTrue("Wrong electionTimeTaken value!",
+electionTimeTaken >= 0);
+} else if (qp.getPeerState() == ServerState.LEADING) {
+bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + 
i
++ ",name1=replica." + i + ",name2=Leader";
--- End diff --

ditto


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2631) Make issue extraction in the git pull request script more robust

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15660668#comment-15660668
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2631:
---

Github user breed commented on the issue:

https://github.com/apache/zookeeper/pull/104
  
+1


> Make issue extraction in the git pull request script more robust
> 
>
> Key: ZOOKEEPER-2631
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2631
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> The QA build is failing for some pull requests because the issue title isn't 
> following the expected format. The issue extraction right now is a bit 
> fragile, so this is to fix the issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-761) Remove *synchronous* calls from the *single-threaded* C clieant API, since they are documented not to work

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15658066#comment-15658066
 ] 

ASF GitHub Bot commented on ZOOKEEPER-761:
--

Github user breed commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/90#discussion_r87652058
  
--- Diff: src/c/src/zookeeper.c ---
@@ -4052,6 +3958,76 @@ int zoo_amulti(zhandle_t *zh, int count, const 
zoo_op_t *ops,
 return (rc < 0) ? ZMARSHALLINGERROR : ZOK;
 }
 
+
+int zoo_aremove_watchers(zhandle_t *zh, const char *path, ZooWatcherType 
wtype,
+watcher_fn watcher, void *watcherCtx, int local,
+void_completion_t *completion, const void *data)
+{
+char *server_path = prepend_string(zh, path);
+int rc;
+struct oarchive *oa;
+struct RequestHeader h = { get_xid(), ZOO_REMOVE_WATCHES };
+struct RemoveWatchesRequest req =  { (char*)server_path, wtype };
+watcher_deregistration_t *wdo;
+
+if (!zh || !isValidPath(server_path, 0)) {
+rc = ZBADARGUMENTS;
+goto done;
+}
+
+if (!local && is_unrecoverable(zh)) {
+rc = ZINVALIDSTATE;
+goto done;
+}
+
+if (!pathHasWatcher(zh, server_path, wtype, watcher, watcherCtx)) {
+rc = ZNOWATCHER;
+goto done;
+}
+
+if (local) {
+removeWatchers(zh, server_path, wtype, watcher, watcherCtx);
+#ifdef THREADED
+notify_sync_completion((struct sync_completion *)data);
--- End diff --

so, just to be clear. is this change correct? we don't need to call 
notify_sync_completion in the non-threaded case. right?


> Remove *synchronous* calls from the *single-threaded* C clieant API, since 
> they are documented not to work
> --
>
> Key: ZOOKEEPER-761
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-761
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: c client
>Affects Versions: 3.1.1, 3.2.2
> Environment: RHEL 4u8 (Linux).  The issue is not OS-specific though.
>Reporter: Jozef Hatala
>Assignee: Benjamin Reed
>Priority: Minor
> Fix For: 3.5.3, 3.6.0
>
> Attachments: fix-sync-apis-in-st-adaptor.patch, 
> fix-sync-apis-in-st-adaptor.v2.patch
>
>
> Since the synchronous calls are 
> [known|http://hadoop.apache.org/zookeeper/docs/current/zookeeperProgrammers.html#Using+the+C+Client]
>  to be unimplemented in the single threaded version of the client library 
> libzookeeper_st.so, I believe that it would be helpful towards users of the 
> library if that information was also obvious from the header file.
> Anecdotally more than one of us here made the mistake of starting by using 
> the synchronous calls with the single-threaded library, and we found 
> ourselves debugging it.  An early warning would have been greatly appreciated.
> 1. Could you please add warnings to the doxygen blocks of all synchronous 
> calls saying that they are not available in the single-threaded API.  This 
> cannot be safely done with {{#ifdef THREADED}}, obviously, because the same 
> header file is included whichever client library implementation one is 
> compiling for.
> 2. Could you please bracket the implementation of all synchronous calls in 
> zookeeper.c with {{#ifdef THREADED}} and {{#endif}}, so that those symbols 
> are not present in libzookeeper_st.so?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2014) Only admin should be allowed to reconfig a cluster

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15662002#comment-15662002
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/96


> Only admin should be allowed to reconfig a cluster
> --
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.0
>Reporter: Raul Gutierrez Segales
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1932) org.apache.zookeeper.test.LETest.testLE fails once in a while

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15667974#comment-15667974
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1932:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/106

ZOOKEEPER-1932: Remove deprecated LeaderElection class.

The motivation of removing LeaderElection class:
* It has been long deprecated and no one uses it.
* Tests around it is flaky.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-1932

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/106.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #106


commit f6f1ad497c71b54ae8944e7d1b1726e66b8153f6
Author: Michael Han 
Date:   2016-11-15T18:57:04Z

ZOOKEEPER-1932: Remove deprecated LeaderElection class.




> org.apache.zookeeper.test.LETest.testLE fails once in a while
> -
>
> Key: ZOOKEEPER-1932
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1932
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: leaderElection
>Affects Versions: 3.5.0
>Reporter: Michi Mutsuzaki
>Assignee: Flavio Junqueira
> Fix For: 3.6.0
>
> Attachments: TEST-org.apache.zookeeper.test.LETest.txt, 
> ZOOKEEPER-1932.patch, ZOOKEEPER-1932.patch
>
>
> org.apache.zookeeper.test.LETest.testLE is failing on trunk once in a while. 
> I'm not able to reproduce the failure on my box. I looked at the log, but I 
> couldn't quite figure out what's going on. 
> https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/2315/testReport/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15668655#comment-15668655
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user hanm closed the pull request at:

https://github.com/apache/zookeeper/pull/102


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15668685#comment-15668685
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

GitHub user hanm reopened a pull request:

https://github.com/apache/zookeeper/pull/102

ZOOKEEPER-2628: Fix findbug warnings.

This PR fixed 19 find bug warnings except this one, which might require 
interface change that potentially could break client side apps.

Malicious code vulnerability Warnings
org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE is a mutable collection
Bug type MS_MUTABLE_COLLECTION (click for details) 
In class org.apache.zookeeper.ZooDefs$Ids
Field org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE
At ZooDefs.java:[line 116]

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2628

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/102.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #102


commit fbb0957167f42ba64ab3eab15fae4b8dce9b0f4b
Author: Michael Han 
Date:   2016-11-15T23:09:02Z

ZOOKEEPER-2628: Fix findbug warnings.




> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2628) Investigate and fix findbug warnings

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15668696#comment-15668696
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2628:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/102
  
Update: disable "Malicious code vulnerability Warnings" appertains to 
org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE, and we will use 
ZOOKEEPER-1362 for this work.

Can we get this merged? Would be good to have findbug clean again for 
pre-commit builds.


> Investigate and fix findbug warnings
> 
>
> Key: ZOOKEEPER-2628
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2628
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
> Fix For: 3.5.3
>
>
> Findbug tool used by Jenkins bot is upgraded to 3.0.1 from 2.0.3 according to 
> Infra team, and this leads to 20 new warnings produced by findbug. The 
> warning reports can be found on [pre commit 
> builds|https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/] with build 
> number >= 3513. These warnings need to be triaged and fixed if they are 
> legitimate.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2574) PurgeTxnLog can inadvertently delete required txn log files

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15679478#comment-15679478
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2574:
---

GitHub user abhishekrai opened a pull request:

https://github.com/apache/zookeeper/pull/111

ZOOKEEPER-2574: PurgeTxnLog can inadvertently delete required txn log files

… files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/abhishekrai/zookeeper ZOOKEEPER-2574

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/111.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #111


commit 4bc4a77800c25ab5bcdaf1149c28b1912d29064f
Author: Abhishek Rai 
Date:   2016-11-18T18:42:51Z

ZOOKEEPER-2574: PurgeTxnLog can inadvertently delete required txn log files

This fix includes patch from Ed Rowe for ZOOKEEPER-2420, which is the same
issue as ZOOKEEPER-2574.




> PurgeTxnLog can inadvertently delete required txn log files
> ---
>
> Key: ZOOKEEPER-2574
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2574
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.7, 3.4.8, 3.5.0, 3.5.1, 3.5.2
> Environment: Zookeeper 3.4.8, standalone, and 3-server quorum
>Reporter: Abhishek Rai
>Assignee: Abhishek Rai
> Fix For: 3.4.10, 3.5.3
>
> Attachments: ZOOKEEPER-2574.2.patch, ZOOKEEPER-2574.3.patch, 
> ZOOKEEPER-2574.4.patch, ZOOKEEPER-2574.5.patch, ZOOKEEPER-2574.6.patch, 
> ZOOKEEPER-2574.patch
>
>
> As part of the fix for ZOOKEEPER-1797, the call to 
> FileTxnSnapLog.getSnapshotLogs() was removed from PurgeTxnLog.java.  As a 
> result, some old-looking but required txn log files can be deleted, resulting 
> in data corruption or loss.
> For example, consider the following:
> 1. Configuration:
> autopurge.snapRetainCount=3
> 2. Following files exist:
> log.100 spans transactions from zxid=100 till zxid=140 (inclusive)
> snapshot.110 - snapshot as of zxid=110
> snapshot.120 - snapshot as of zxid=120
> snapshot.130 - snapshot as of zxid=130
> Above scenario is possible when snapshotting has happened multiple times but 
> without accompanying log rollover, which is possible if the server was 
> running as a learner.
> 3. PurgeTxnLog retains all snapshots but deletes log.100 because its zxid is 
> older than the zxid of the oldest snapshot (110).  This results in loss of 
> transactions in the range 131-140.
> Before the fix for ZOOKEEPER-1797, this was avoided by the call to 
> FileTxnSnapLog.getSnapshotLogs() which finds and retains the newest txn log 
> file with starting zxid < oldest retained snapshot's highest zxid.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1525) Plumb ZooKeeperServer object into auth plugins

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1525?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15630321#comment-15630321
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1525:
---

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/84
  
@Randgalt I'm sorry that QA isn't working for pull requests yet, could 
please create a diff patch and upload to the jira so that we can get QA to run 
on it?


> Plumb ZooKeeperServer object into auth plugins
> --
>
> Key: ZOOKEEPER-1525
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1525
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.5.0
>Reporter: Warren Turkal
>Assignee: Jordan Zimmerman
> Fix For: 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch
>
>
> I want to plumb the ZooKeeperServer object into the auth plugins so that I 
> can store authentication data in zookeeper itself. With access to the 
> ZooKeeperServer object, I also have access to the ZKDatabase and can look up 
> entries in the local copy of the zookeeper data.
> In order to implement this, I make sure that a ZooKeeperServer instance is 
> passed in to the ProviderRegistry.initialize() method. Then initialize() will 
> try to find a constructor for the AuthenticationProvider that takes a 
> ZooKeeperServer instance. If the constructor is found, it will be used. 
> Otherwise, initialize() will look for a constructor that takes no arguments 
> and use that instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'fleTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15632209#comment-15632209
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

GitHub user rakeshadr opened a pull request:

https://github.com/apache/zookeeper/pull/98

ZOOKEEPER-2479



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rakeshadr/zookeeper-1 ZK-2479

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/98.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #98


commit e4fe6cd5a1728c0847a392893b56860dbde1f7bd
Author: Rakesh Radhakrishnan 
Date:   2016-11-03T09:33:03Z

ZOOKEEPER-2479




> Add 'fleTimeTaken' value in LeaderMXBean and FollowerMXBean
> ---
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633352#comment-15633352
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/97#discussion_r86386008
  
--- Diff: build.xml ---
@@ -1622,6 +1623,26 @@ 
xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">

  
 
+ 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
--- End diff --

yes, it was intentionally left out. the `defect` number is the jira number. 
in the jira qa, that is being set by the trigger of the jira patch. if you look 
further down in the script, you'll see that I'm setting the `defect` variable 
by extracting the number from the PR title. I expect the PR title to contain 
the jira title `ZOOKEEPER-: Bla bla bla`. 


> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633358#comment-15633358
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

Github user fpj commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/97#discussion_r86386241
  
--- Diff: src/java/test/bin/test-github-pr.sh ---
@@ -0,0 +1,607 @@
+#!/usr/bin/env bash
+#   Licensed 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.
+
+
+#set -x
+
+### Setup some variables.
+### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch 
process
+### Read variables from properties file
+. `dirname $0`/test-patch.properties
+

+###
+parseArgs() {
+  case "$1" in
+QABUILD)
+  ### Set QABUILD to true to indicate that this script is being run by 
Hudson
+  QABUILD=true
+  if [[ $# != 14 ]] ; then
+echo "ERROR: usage $0 QABUILD
  
   "
+cleanupAndExit 0
+  fi
+  PATCH_DIR=$2
+  PS=$3
+  WGET=$4
+  JIRACLI=$5
+  GIT=$6
+  GREP=$7
+  PATCH=$8
+  FINDBUGS_HOME=$9
+  FORREST_HOME=${10}
+  BASEDIR=${11}
+  JIRA_PASSWD=${12}
+  JAVA5_HOME=${13}
+  CURL=${14}
+  if [ ! -e "$PATCH_DIR" ] ; then
+mkdir -p $PATCH_DIR
+  fi
+
+  ;;
+DEVELOPER)
+  ### Set QABUILD to false to indicate that this script is being run 
by a developer
+  QABUILD=false
+  if [[ $# != 10 ]] ; then
+echo "ERROR: usage $0 DEVELOPER   
 
 "
+cleanupAndExit 0
+  fi
+  ### PATCH_FILE contains the location of the patchfile
+  PATCH_FILE=$2
+  if [[ ! -e "$PATCH_FILE" ]] ; then
+echo "Unable to locate the patch file $PATCH_FILE"
+cleanupAndExit 0
+  fi
+  PATCH_DIR=$3
+  ### Check if $PATCH_DIR exists. If it does not exist, create a new 
directory
+  if [[ ! -e "$PATCH_DIR" ]] ; then
+   mkdir "$PATCH_DIR"
+   if [[ $? == 0 ]] ; then
+ echo "$PATCH_DIR has been created"
+   else
+ echo "Unable to create $PATCH_DIR"
+ cleanupAndExit 0
+   fi
+  fi
+  GIT=$4
+  GREP=$5
+  PATCH=$6
+  FINDBUGS_HOME=$7
+  FORREST_HOME=$8
+  BASEDIR=$9
+  JAVA5_HOME=${10}
+  ### Obtain the patch filename to append it to the version number
+  defect=`basename $PATCH_FILE`
--- End diff --

not really, we care about it to write the report to the jira.


> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633354#comment-15633354
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/97#discussion_r86386121
  
--- Diff: src/java/test/bin/test-github-pr.sh ---
@@ -0,0 +1,607 @@
+#!/usr/bin/env bash
+#   Licensed 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.
+
+
+#set -x
+
+### Setup some variables.
+### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch 
process
+### Read variables from properties file
+. `dirname $0`/test-patch.properties
+

+###
+parseArgs() {
+  case "$1" in
+QABUILD)
+  ### Set QABUILD to true to indicate that this script is being run by 
Hudson
+  QABUILD=true
+  if [[ $# != 14 ]] ; then
+echo "ERROR: usage $0 QABUILD
  
   "
+cleanupAndExit 0
+  fi
+  PATCH_DIR=$2
+  PS=$3
+  WGET=$4
+  JIRACLI=$5
+  GIT=$6
+  GREP=$7
+  PATCH=$8
+  FINDBUGS_HOME=$9
+  FORREST_HOME=${10}
+  BASEDIR=${11}
+  JIRA_PASSWD=${12}
+  JAVA5_HOME=${13}
+  CURL=${14}
+  if [ ! -e "$PATCH_DIR" ] ; then
+mkdir -p $PATCH_DIR
+  fi
+
+  ;;
+DEVELOPER)
+  ### Set QABUILD to false to indicate that this script is being run 
by a developer
+  QABUILD=false
+  if [[ $# != 10 ]] ; then
+echo "ERROR: usage $0 DEVELOPER   
 
 "
+cleanupAndExit 0
+  fi
+  ### PATCH_FILE contains the location of the patchfile
+  PATCH_FILE=$2
+  if [[ ! -e "$PATCH_FILE" ]] ; then
+echo "Unable to locate the patch file $PATCH_FILE"
+cleanupAndExit 0
+  fi
+  PATCH_DIR=$3
+  ### Check if $PATCH_DIR exists. If it does not exist, create a new 
directory
+  if [[ ! -e "$PATCH_DIR" ]] ; then
+   mkdir "$PATCH_DIR"
+   if [[ $? == 0 ]] ; then
+ echo "$PATCH_DIR has been created"
+   else
+ echo "Unable to create $PATCH_DIR"
+ cleanupAndExit 0
+   fi
+  fi
+  GIT=$4
+  GREP=$5
+  PATCH=$6
+  FINDBUGS_HOME=$7
+  FORREST_HOME=$8
+  BASEDIR=$9
+  JAVA5_HOME=${10}
+  ### Obtain the patch filename to append it to the version number
+  defect=`basename $PATCH_FILE`
--- End diff --

With git flow, I guess we don't care about the patch file name (e.g. 
ZOOKEEPER-1234.patch), so this could be removed?


> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633361#comment-15633361
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/97
  
The failure is actually expected because the script contains the `@author` 
string.


> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633784#comment-15633784
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r86415195
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -520,6 +520,12 @@ public synchronized void setCurrentVote(Vote v){
 protected boolean quorumListenOnAllIPs = false;
 
 /**
+ * Keeps time taken for leader election in milliseconds. Sets the 
value to
+ * this variable only after the completion of leader election.
+ */
+private long electionTimeTaken = -1;
--- End diff --

sincere question: does it make sense to make this field `volatile`? I know 
there are some concurrency guarantees but not sure if it's worth change it.


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-761) Remove *synchronous* calls from the *single-threaded* C clieant API, since they are documented not to work

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15612119#comment-15612119
 ] 

ASF GitHub Bot commented on ZOOKEEPER-761:
--

Github user breed commented on the issue:

https://github.com/apache/zookeeper/pull/90
  
@fpj on the plus side that failure validates that the change is correct :) 
(it is trying to use a sync API with a non threaded test)


> Remove *synchronous* calls from the *single-threaded* C clieant API, since 
> they are documented not to work
> --
>
> Key: ZOOKEEPER-761
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-761
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: c client
>Affects Versions: 3.1.1, 3.2.2
> Environment: RHEL 4u8 (Linux).  The issue is not OS-specific though.
>Reporter: Jozef Hatala
>Assignee: Benjamin Reed
>Priority: Minor
> Fix For: 3.5.3, 3.6.0
>
> Attachments: fix-sync-apis-in-st-adaptor.patch, 
> fix-sync-apis-in-st-adaptor.v2.patch
>
>
> Since the synchronous calls are 
> [known|http://hadoop.apache.org/zookeeper/docs/current/zookeeperProgrammers.html#Using+the+C+Client]
>  to be unimplemented in the single threaded version of the client library 
> libzookeeper_st.so, I believe that it would be helpful towards users of the 
> library if that information was also obvious from the header file.
> Anecdotally more than one of us here made the mistake of starting by using 
> the synchronous calls with the single-threaded library, and we found 
> ourselves debugging it.  An early warning would have been greatly appreciated.
> 1. Could you please add warnings to the doxygen blocks of all synchronous 
> calls saying that they are not available in the single-threaded API.  This 
> cannot be safely done with {{#ifdef THREADED}}, obviously, because the same 
> header file is included whichever client library implementation one is 
> compiling for.
> 2. Could you please bracket the implementation of all synchronous calls in 
> zookeeper.c with {{#ifdef THREADED}} and {{#endif}}, so that those symbols 
> are not present in libzookeeper_st.so?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2622) ZooTrace.logQuorumPacket does nothing

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2622?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15614872#comment-15614872
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2622:
---

GitHub user fpj opened a pull request:

https://github.com/apache/zookeeper/pull/95

ZOOKEEPER-2622: ZooTrace.logQuorumPacket does nothing

DO NOT MERGE

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/fpj/zookeeper ZK-2622

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/95.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #95


commit 76681a075e045107f149e5ab67756991faf1d8ba
Author: fpj 
Date:   2016-10-28T09:14:51Z

ZOOKEEPER-2622: ZooTrace.logQuorumPacket does nothing




> ZooTrace.logQuorumPacket does nothing
> -
>
> Key: ZOOKEEPER-2622
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2622
> Project: ZooKeeper
>  Issue Type: Bug
>Reporter: Flavio Junqueira
>Priority: Trivial
>  Labels: newbie
> Fix For: 3.5.3, 3.6.0
>
>
> The method simply returns and there is some code commented out:
> {code}
> // if (isTraceEnabled(log, mask)) {
> // logTraceMessage(LOG, mask, direction + " "
> // + FollowerHandler.packetToString(qp));
> // }
> {code}
> There are calls to this trace method, so I think we should fix it.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2014) Only admin should be allowed to reconfig a cluster

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15621149#comment-15621149
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/96

ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster.

This PR implements ZOOKEEPER-2014. For details, please refer to

JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
Review board: https://reviews.apache.org/r/51546/

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2014

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/96.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #96


commit 6d18cffde99d4cf5298e045d6c0f23b36fd62925
Author: Michael Han 
Date:   2016-10-31T03:58:11Z

ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster.




> Only admin should be allowed to reconfig a cluster
> --
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.0
>Reporter: Raul Gutierrez Segales
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2014) Only admin should be allowed to reconfig a cluster

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15621145#comment-15621145
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/94
  
@fpj Sure, closing and resubmitting with a new one that also addressing 
Rakesh's comments today.


> Only admin should be allowed to reconfig a cluster
> --
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.0
>Reporter: Raul Gutierrez Segales
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15621992#comment-15621992
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

GitHub user fpj opened a pull request:

https://github.com/apache/zookeeper/pull/97

ZOOKEEPER-2624: Add test script for pull requests



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/fpj/zookeeper ZK-2624

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/97.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #97


commit 1929614a7ef076d9e2bd1191b3341207f2e4411e
Author: fpj 
Date:   2016-10-31T12:13:59Z

Initial cut of the script and changes to build.xml




> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2014) Only admin should be allowed to reconfig a cluster

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15620024#comment-15620024
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
---

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/94
  
@hanm since there is no comment, would you mind closing this PR and 
resubmitting it to see if QA picks it up?


> Only admin should be allowed to reconfig a cluster
> --
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.0
>Reporter: Raul Gutierrez Segales
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1525) Plumb ZooKeeperServer object into auth plugins

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1525?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15630473#comment-15630473
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1525:
---

Github user Randgalt commented on the issue:

https://github.com/apache/zookeeper/pull/84
  
latest patch added


> Plumb ZooKeeperServer object into auth plugins
> --
>
> Key: ZOOKEEPER-1525
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1525
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.5.0
>Reporter: Warren Turkal
>Assignee: Jordan Zimmerman
> Fix For: 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch
>
>
> I want to plumb the ZooKeeperServer object into the auth plugins so that I 
> can store authentication data in zookeeper itself. With access to the 
> ZooKeeperServer object, I also have access to the ZKDatabase and can look up 
> entries in the local copy of the zookeeper data.
> In order to implement this, I make sure that a ZooKeeperServer instance is 
> passed in to the ProviderRegistry.initialize() method. Then initialize() will 
> try to find a constructor for the AuthenticationProvider that takes a 
> ZooKeeperServer instance. If the constructor is found, it will be used. 
> Otherwise, initialize() will look for a constructor that takes no arguments 
> and use that instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15635624#comment-15635624
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r86502042
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -520,6 +520,12 @@ public synchronized void setCurrentVote(Vote v){
 protected boolean quorumListenOnAllIPs = false;
 
 /**
+ * Keeps time taken for leader election in milliseconds. Sets the 
value to
+ * this variable only after the completion of leader election.
+ */
+private long electionTimeTaken = -1;
--- End diff --

Yup, agree. :) Thanks for explaining!


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2479) Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2479?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15635085#comment-15635085
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2479:
---

Github user rakeshadr commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/98#discussion_r86483860
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -520,6 +520,12 @@ public synchronized void setCurrentVote(Vote v){
 protected boolean quorumListenOnAllIPs = false;
 
 /**
+ * Keeps time taken for leader election in milliseconds. Sets the 
value to
+ * this variable only after the completion of leader election.
+ */
+private long electionTimeTaken = -1;
--- End diff --

Thanks @eribeiro ,

LeaderMXBean, FollowerMXBean will be available only after the quorum leader 
election and the value won't be changed until next LE. I think, adding 
'volatile' doesn't make any difference, right?

code reference:


https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/Leader.java#L417

https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/Follower.java#L70

Leader.java
zk.registerJMX(new LeaderBean(this, zk), self.jmxLocalPeerBean);

Follower.java
fzk.registerJMX(new FollowerBean(this, zk), self.jmxLocalPeerBean);


> Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
> 
>
> Key: ZOOKEEPER-2479
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2479
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: quorum
>Reporter: Rakesh R
>Assignee: Rakesh R
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, ZOOKEEPER-2479.patch, 
> ZOOKEEPER-2479.patch
>
>
> The idea of this jira is to expose {{time taken}} for the leader election via 
> jmx Leader, Follower beans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2624) Add test script for pull requests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15636179#comment-15636179
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2624:
---

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/97
  
@hanm The `DEVELOPER` mode is for running locally, so initially I left it 
mostly unchanged and the original script takes a patch file to test. Because of 
your feedback, I thought that it might be better to take a pull request URL 
instead to test, so I changed the script for the developer mode a bit to do it.


> Add test script for pull requests
> -
>
> Key: ZOOKEEPER-2624
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2624
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: scripts
>Reporter: Flavio Junqueira
>Assignee: Flavio Junqueira
>
> We need a script similar to {{test-patch.sh}} to handle QA builds for pull 
> requests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1525) Plumb ZooKeeperServer object into auth plugins

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1525?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15626775#comment-15626775
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1525:
---

Github user Randgalt commented on the issue:

https://github.com/apache/zookeeper/pull/84
  
@fpj any updates on this?


> Plumb ZooKeeperServer object into auth plugins
> --
>
> Key: ZOOKEEPER-1525
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1525
> Project: ZooKeeper
>  Issue Type: Improvement
>Affects Versions: 3.5.0
>Reporter: Warren Turkal
>Assignee: Jordan Zimmerman
> Fix For: 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch, 
> ZOOKEEPER-1525.patch, ZOOKEEPER-1525.patch
>
>
> I want to plumb the ZooKeeperServer object into the auth plugins so that I 
> can store authentication data in zookeeper itself. With access to the 
> ZooKeeperServer object, I also have access to the ZKDatabase and can look up 
> entries in the local copy of the zookeeper data.
> In order to implement this, I make sure that a ZooKeeperServer instance is 
> passed in to the ProviderRegistry.initialize() method. Then initialize() will 
> try to find a constructor for the AuthenticationProvider that takes a 
> ZooKeeperServer instance. If the constructor is found, it will be used. 
> Otherwise, initialize() will look for a constructor that takes no arguments 
> and use that instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2481) Flaky Test: testZeroWeightQuorum

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15609043#comment-15609043
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2481:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/93#discussion_r85172473
  
--- Diff: README.txt ---
@@ -35,3 +35,7 @@ dist-maven directory are deployed to the central 
repository after the release
 is voted on and approved by the Apache ZooKeeper PMC:
 
   https://repo1.maven.org/maven2/org/apache/zookeeper/zookeeper/
+
--- End diff --

3rd comment made on git.


> Flaky Test: testZeroWeightQuorum
> 
>
> Key: ZOOKEEPER-2481
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2481
> Project: ZooKeeper
>  Issue Type: Test
>  Components: server, tests
>Affects Versions: 3.4.8, 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
>  Labels: flaky, flaky-test
> Fix For: 3.5.3
>
>
> See https://builds.apache.org/job/ZooKeeper-trunk-openjdk7/1098/
> {noformat}
> Error Message
> Threads didn't join
> Stacktrace
> junit.framework.AssertionFailedError: Threads didn't join
>   at 
> org.apache.zookeeper.test.FLEZeroWeightTest.testZeroWeightQuorum(FLEZeroWeightTest.java:167)
>   at 
> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)
> Standard Output
> 2016-07-21 04:24:14,065 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,158 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,176 [myid:] - INFO  [main:ZKTestCase$1@55] - STARTING 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  
> [main:JUnit4ZKTestRunner$LoggedInvokeMethod@77] - RUNNING TEST METHOD 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  [main:FLEZeroWeightTest@143] - 
> TestZeroWeightQuorum: testZeroWeightQuorum, 9
> 2016-07-21 04:24:14,183 [myid:] - INFO  [main:PortAssignment@157] - Single 
> test process using ports from 11221 - 32767.
> 2016-07-21 04:24:14,187 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11222 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11223 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11224 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11225 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11226 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11227 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11228 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11229 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11230 from range 11221 - 32767.
> 2016-07-21 04:24:14,221 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11231 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11232 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11233 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11234 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11235 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11236 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11237 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11238 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11239 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11240 from range 11221 - 32767.
> 2016-07-21 04:24:14,228 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11241 from range 11221 - 32767.
> 2016-07-21 04:24:14,228 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11242 from range 11221 - 32767.
> 2016-07-21 04:24:14,229 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11243 from range 11221 - 32767.
> 2016-07-21 04:24:14,229 [myid:] - INFO  [main:PortAssignment@85] 

[jira] [Commented] (ZOOKEEPER-2597) Add script to merge PR from Apache git repo to Github

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15609163#comment-15609163
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2597:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/85
  
+1 great job Eddie.


> Add script to merge PR from Apache git repo to Github
> -
>
> Key: ZOOKEEPER-2597
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2597
> Project: ZooKeeper
>  Issue Type: Improvement
>Reporter: Edward Ribeiro
>Assignee: Edward Ribeiro
>Priority: Minor
> Attachments: ZOOKEEPER-2597.patch
>
>
> A port of kafka-merge-pr.py to workon on ZooKeeper repo.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2014) Only admin should be allowed to reconfig a cluster

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15612555#comment-15612555
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2014:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/94

ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster.

This PR implements ZOOKEEPER-2014. For details, please refer to
* JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
* Previous review board: https://reviews.apache.org/r/51546/

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2014

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/94.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #94


commit 616e1275ac38890c2bf1e3ac27465172cf1c52d5
Author: Michael Han 
Date:   2016-10-27T16:16:27Z

ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster.




> Only admin should be allowed to reconfig a cluster
> --
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.0
>Reporter: Raul Gutierrez Segales
>Assignee: Michael Han
>Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, 
> ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We 
> should, at the very least, ensure that only the Admin can reconfigure a 
> cluster. Perhaps restricting access to /zookeeper/config as well, though this 
> is debatable. Surely one could ensure Admin only access via an ACL, but that 
> would leave everyone who doesn't use ACLs unprotected. We could also force a 
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run 
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure 
> if everyone does, or how would it work with other authentication providers). 
> Review board https://reviews.apache.org/r/51546/



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2049) Yosemite build failure: htonll conflict

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15583366#comment-15583366
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2049:
---

Github user parthchandra commented on the issue:

https://github.com/apache/drill/pull/602
  
OK. The Mac issue I see is caused by MacOS -  
https://issues.apache.org/jira/browse/ZOOKEEPER-2049
Just to confirm, what version of MacOS and Zookeeper did you build with?
Should we upgrade the client to use Zookeeper 3.4.7?





> Yosemite build failure: htonll conflict
> ---
>
> Key: ZOOKEEPER-2049
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2049
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.4.5, 3.4.6, 3.5.0
> Environment: OSX 10.10 (BETA3), Apple LLVM version 6.0 
> (clang-600.0.51) (based on LLVM 3.5svn)
>Reporter: Till Toenshoff
>Assignee: Till Toenshoff
> Fix For: 3.4.7, 3.5.1, 3.6.0
>
> Attachments: ZOOKEEPER-2049.noprefix.branch-3.4.patch, 
> ZOOKEEPER-2049.noprefix.branch-3.5.patch, ZOOKEEPER-2049.noprefix.trunk.patch
>
>
> recordio.h defines {{htonll}} which conflicts with Apple's equally named 
> implementation from within arpa/inet.h.
> {noformat}
>  gcc -DHAVE_CONFIG_H -I. -I. -I. -I./include -I./tests -I./generated -Wall 
> -Werror -g -O2 -D_GNU_SOURCE -MT zk_log.lo -MD -MP -MF .deps/zk_log.Tpo -c 
> src/zk_log.c  -fno-common -DPIC -o zk_log.o
> In file included from src/recordio.c:19:
> ./include/recordio.h:76:9: error: expected ')'
> int64_t htonll(int64_t v);
> ^
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:30: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
>  ^
> ./include/recordio.h:76:9: note: to match this '('
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:5: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
> ^
> In file included from src/recordio.c:19:
> ./include/recordio.h:76:9: error: conflicting types for '__builtin_constant_p'
> int64_t htonll(int64_t v);
> ^
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:6: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
>  ^
> ./include/recordio.h:76:9: note: '__builtin_constant_p' is a builtin with 
> type 'int ()'
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:6: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
>  ^
> In file included from generated/zookeeper.jute.c:20:
> In file included from ./generated/zookeeper.jute.h:21:
> ./include/recordio.h:76:9: error: expected ')'
> int64_t htonll(int64_t v);
> ^
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:30: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
>  ^
> ./include/recordio.h:76:9: note: to match this '('
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:5: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
> ^
> In file included from generated/zookeeper.jute.c:20:
> In file included from ./generated/zookeeper.jute.h:21:
> ./include/recordio.h:76:9: error: conflicting types for '__builtin_constant_p'
> int64_t htonll(int64_t v);
> ^
> /usr/include/sys/_endian.h:141:25: note: expanded from macro 'htonll'
> #define htonll(x)   __DARWIN_OSSwapInt64(x)
> ^
> /usr/include/libkern/_OSByteOrder.h:78:6: note: expanded from macro 
> '__DARWIN_OSSwapInt64'
> (__builtin_constant_p(x) ? __DARWIN_OSSwapConstInt64(x) : _OSSwapInt64(x))
>  ^
> ./include/recordio.h:76:9: note: 

[jira] [Commented] (ZOOKEEPER-2481) Flaky Test: testZeroWeightQuorum

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15583510#comment-15583510
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2481:
---

Github user hanm closed the pull request at:

https://github.com/apache/zookeeper/pull/88


> Flaky Test: testZeroWeightQuorum
> 
>
> Key: ZOOKEEPER-2481
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2481
> Project: ZooKeeper
>  Issue Type: Test
>  Components: server, tests
>Affects Versions: 3.4.8, 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
>  Labels: flaky, flaky-test
> Fix For: 3.5.3
>
>
> See https://builds.apache.org/job/ZooKeeper-trunk-openjdk7/1098/
> {noformat}
> Error Message
> Threads didn't join
> Stacktrace
> junit.framework.AssertionFailedError: Threads didn't join
>   at 
> org.apache.zookeeper.test.FLEZeroWeightTest.testZeroWeightQuorum(FLEZeroWeightTest.java:167)
>   at 
> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)
> Standard Output
> 2016-07-21 04:24:14,065 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,158 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,176 [myid:] - INFO  [main:ZKTestCase$1@55] - STARTING 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  
> [main:JUnit4ZKTestRunner$LoggedInvokeMethod@77] - RUNNING TEST METHOD 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  [main:FLEZeroWeightTest@143] - 
> TestZeroWeightQuorum: testZeroWeightQuorum, 9
> 2016-07-21 04:24:14,183 [myid:] - INFO  [main:PortAssignment@157] - Single 
> test process using ports from 11221 - 32767.
> 2016-07-21 04:24:14,187 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11222 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11223 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11224 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11225 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11226 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11227 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11228 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11229 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11230 from range 11221 - 32767.
> 2016-07-21 04:24:14,221 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11231 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11232 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11233 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11234 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11235 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11236 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11237 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11238 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11239 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11240 from range 11221 - 32767.
> 2016-07-21 04:24:14,228 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11241 from range 11221 - 32767.
> 2016-07-21 04:24:14,228 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11242 from range 11221 - 32767.
> 2016-07-21 04:24:14,229 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11243 from range 11221 - 32767.
> 2016-07-21 04:24:14,229 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11244 from range 11221 - 32767.
> 2016-07-21 04:24:14,229 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11245 from range 11221 - 32767.
> 2016-07-21 04:24:14,230 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11246 from range 11221 - 32767.
> 2016-07-21 04:24:14,230 [myid:] - INFO  [main:PortAssignment@85] - 

[jira] [Commented] (ZOOKEEPER-2481) Flaky Test: testZeroWeightQuorum

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15583516#comment-15583516
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2481:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/89

ZOOKEEPER-2481: Flaky Test: testZeroWeightQuorum / Don't merge, this is 
just a test please discard.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2481

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/89.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #89


commit b0c87dbd0ec070195c8a4f98e88d33ff567230ca
Author: Michael Han 
Date:   2016-10-17T21:13:19Z

This is just a test please discard.




> Flaky Test: testZeroWeightQuorum
> 
>
> Key: ZOOKEEPER-2481
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2481
> Project: ZooKeeper
>  Issue Type: Test
>  Components: server, tests
>Affects Versions: 3.4.8, 3.5.2
>Reporter: Michael Han
>Assignee: Michael Han
>  Labels: flaky, flaky-test
> Fix For: 3.5.3
>
>
> See https://builds.apache.org/job/ZooKeeper-trunk-openjdk7/1098/
> {noformat}
> Error Message
> Threads didn't join
> Stacktrace
> junit.framework.AssertionFailedError: Threads didn't join
>   at 
> org.apache.zookeeper.test.FLEZeroWeightTest.testZeroWeightQuorum(FLEZeroWeightTest.java:167)
>   at 
> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:79)
> Standard Output
> 2016-07-21 04:24:14,065 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,158 [myid:] - INFO  [main:JUnit4ZKTestRunner@47] - No 
> test.method specified. using default methods.
> 2016-07-21 04:24:14,176 [myid:] - INFO  [main:ZKTestCase$1@55] - STARTING 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  
> [main:JUnit4ZKTestRunner$LoggedInvokeMethod@77] - RUNNING TEST METHOD 
> testZeroWeightQuorum
> 2016-07-21 04:24:14,180 [myid:] - INFO  [main:FLEZeroWeightTest@143] - 
> TestZeroWeightQuorum: testZeroWeightQuorum, 9
> 2016-07-21 04:24:14,183 [myid:] - INFO  [main:PortAssignment@157] - Single 
> test process using ports from 11221 - 32767.
> 2016-07-21 04:24:14,187 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11222 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11223 from range 11221 - 32767.
> 2016-07-21 04:24:14,189 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11224 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11225 from range 11221 - 32767.
> 2016-07-21 04:24:14,218 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11226 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11227 from range 11221 - 32767.
> 2016-07-21 04:24:14,219 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11228 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11229 from range 11221 - 32767.
> 2016-07-21 04:24:14,220 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11230 from range 11221 - 32767.
> 2016-07-21 04:24:14,221 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11231 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11232 from range 11221 - 32767.
> 2016-07-21 04:24:14,224 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11233 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11234 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11235 from range 11221 - 32767.
> 2016-07-21 04:24:14,225 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11236 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11237 from range 11221 - 32767.
> 2016-07-21 04:24:14,226 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11238 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11239 from range 11221 - 32767.
> 2016-07-21 04:24:14,227 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11240 from range 11221 - 32767.
> 2016-07-21 04:24:14,228 [myid:] - INFO  [main:PortAssignment@85] - Assigned 
> port 11241 from 

[jira] [Commented] (ZOOKEEPER-1634) A new feature proposal to ZooKeeper: authentication enforcement

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1634?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15707003#comment-15707003
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1634:
---

GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/118

ZOOKEEPER-1634: hardening security by teaching server to enforce client 
authentication.

## Motivation
Previously ZooKeeper server is open to the world as the server does not 
enforce client authentication - anonymous clients are allowed to establish 
session with server. This behavior raises a couple of issues from the 
perspective of performance and security for example:
* It is easy to launch a DDoS attack to server, by having a fleet of 
anonymous clients connect to the ensemble, as each session would consume 
valuable resources (socket, memory, etc) from server. 
* It is cumbersome to enforce certain security models with the presence of 
anonymous clients login - for example as clients are not trusted the root ACL 
has to be disabled for writes to world, among other configurations an admin has 
to do to secure a cluster in a multi-tenant environment.

So the goal here is to address such issue by hardening ZooKeeper security 
to provide a more confined access option that user could opt-in, which in 
addition to the existing ACLs together could lead to more secured / resource 
optimal ensemble. 

## Design Abstract
* Introduce a new server side Java property that if set, ZooKeeper server 
will only accept connections and requests from clients that have authenticated 
with server via SASL.
* Clients that are not configured with SASL authentication, or configured 
with SASL but fail authentication (i.e. with invalid credential) will not be 
able to establish a session with server. A typed error code (-124) will be 
delivered in such case, both Java and C client will close the session with 
server thereafter, without further attempts on retrying to reconnect.
* This feature overrules the server property 
"zookeeper.allowSaslFailedClients". So even if server is configured to allow 
clients that fail SASL authentication to login, client will not be able to 
establish a session with server if this feature is enabled.
* Only support SASL because only SASL authentication has the property that 
no operations will happen until SASL authentication process finished. Thus, the 
decision of whether to close the session or not can be quickly made on server 
side upon receiving a client connection request. We could later add support for 
other auth scheme via add_auth_info if that's desired (if we do, then a session 
has to be maintained until add_auth_info is invoked.).
* As a side benefit, this PR fixes an issue mentioned in ZOOKEEPER-2346 by 
correctly propagate events from server to client side so a SASL auth failure 
will manifest as an auth / config failure rather than generic ConnectionLoss 
event.

JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-1634
The PR also covers (or part of):
https://issues.apache.org/jira/browse/ZOOKEEPER-2462
https://issues.apache.org/jira/browse/ZOOKEEPER-2526
https://issues.apache.org/jira/browse/ZOOKEEPER-2346





You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-1634

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/118.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #118


commit 90beaa0396cb2238b40e4b93764bd1a396b9047b
Author: Michael Han 
Date:   2016-11-29T23:19:38Z

ZOOKEEPER-1634: teach server to enforce client authentication.




> A new feature proposal to ZooKeeper: authentication enforcement
> ---
>
> Key: ZOOKEEPER-1634
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1634
> Project: ZooKeeper
>  Issue Type: New Feature
>  Components: security, server
>Affects Versions: 3.4.5
>Reporter: Jaewoong Choi
>Assignee: Michael Han
> Fix For: 3.6.0
>
> Attachments: 
> zookeeper_3.4.5_patch_for_authentication_enforcement.patch
>
>   Original Estimate: 72h
>  Remaining Estimate: 72h
>
> Up to the version of 3.4.5, ZooKeeperServer doesn't force the authentication 
> if the client doesn't give any auth-info through ZooKeeper#addAuthInfo method 
> invocation.  Hence, every znode should have at least one ACL assigned 
> otherwise any unauthenticated client can do anything on it.
> The current authentication/authorization mechanism of ZooKeeper described 
> above has several points at issue:
> 1. 

[jira] [Commented] (ZOOKEEPER-2325) Data inconsistency if all snapshots empty or missing

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15706783#comment-15706783
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2325:
---

GitHub user breed opened a pull request:

https://github.com/apache/zookeeper/pull/117

ZOOKEEPER-2325: Data inconsistency if all snapshots empty or missing



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/breed/zookeeper ZOOKEEPER-2325

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/117.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #117


commit 02bf3d57786d51da205e78a070a45703da21f916
Author: Benjamin Reed 
Date:   2016-11-29T22:08:22Z

ZOOKEEPER-2325: Data inconsistency if all snapshots empty or missing




> Data inconsistency if all snapshots empty or missing
> 
>
> Key: ZOOKEEPER-2325
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2325
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.6
>Reporter: Andrew Grasso
>Assignee: Andrew Grasso
>Priority: Critical
> Attachments: ZOOKEEPER-2325-test.patch, ZOOKEEPER-2325.001.patch, 
> zk.patch
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> When loading state from snapshots on startup, FileTxnSnapLog.java ignores the 
> result of FileSnap.deserialize, which is -1L if no valid snapshots are found. 
> Recovery proceeds with dt.lastProcessed == 0, its initial value.
> The result is that Zookeeper will process the transaction logs and then begin 
> serving requests with a different state than the rest of the ensemble.
> To reproduce:
> In a healthy zookeeper cluster of size >= 3, shut down one node.
> Either delete all snapshots for this node or change all to be empty files.
> Restart the node.
> We believe this can happen organically if a node runs out of disk space.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-261) Reinitialized servers should not participate in leader election

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15733423#comment-15733423
 ] 

ASF GitHub Bot commented on ZOOKEEPER-261:
--

Github user enixon commented on the issue:

https://github.com/apache/zookeeper/pull/120
  
- add documentation on 'zookeeper.db.autocreate' to zookeeperAdmin.xml
- extend  bin/zkServer-initialize.sh to create the initialize file
- treat failure to delete initialization file as fatal, throw IOException 
instead of logging a warning


> Reinitialized servers should not participate in leader election
> ---
>
> Key: ZOOKEEPER-261
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-261
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: leaderElection, quorum
>Reporter: Benjamin Reed
>
> A server that has lost its data should not participate in leader election 
> until it has resynced with a leader. Our leader election algorithm and 
> NEW_LEADER commit assumes that the followers voting on a leader have not lost 
> any of their data. We should have a flag in the data directory saying whether 
> or not the data is preserved so that the the flag will be cleared if the data 
> is ever cleared.
> Here is the problematic scenario: you have have ensemble of machines A, B, 
> and C. C is down. the last transaction seen by C is z. a transaction, z+1, is 
> committed on A and B. Now there is a power outage. B's data gets 
> reinitialized. when power comes back up, B and C comes up, but A does not. C 
> will be elected leader and transaction z+1 is lost. (note, this can happen 
> even if all three machines are up and C just responds quickly. in that case C 
> would tell A to truncate z+1 from its log.) in theory we haven't violated our 
> 2f+1 guarantee, since A is failed and B still hasn't recovered from failure, 
> but it would be nice if when we don't have quorum that system stops working 
> rather than works incorrectly if we lose quorum.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2517) jute.maxbuffer is ignored

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15733566#comment-15733566
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2517:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/113
  
@rgs1 this is the blocker issue we were talking about, please take a look 
at it.


> jute.maxbuffer is ignored
> -
>
> Key: ZOOKEEPER-2517
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2517
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.2
>Reporter: Benjamin Jaton
>Assignee: Arshad Mohammad
>Priority: Blocker
> Fix For: 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2517-01.patch, ZOOKEEPER-2517-02.patch, 
> ZOOKEEPER-2517-03.patch, ZOOKEEPER-2517-04.patch, ZOOKEEPER-2517.patch
>
>
> In ClientCnxnSocket.java the parsing of the system property is erroneous:
> {code}packetLen = Integer.getInteger(
>   clientConfig.getProperty(ZKConfig.JUTE_MAXBUFFER),
>   ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT
> );{code}
> Javadoc of Integer.getInteger states "The first argument is treated as the 
> name of a system property", whereas here the value of the property is passed.
> Instead I believe the author meant to write something like:
> {code}packetLen = Integer.parseInt(
>   clientConfig.getProperty(
> ZKConfig.JUTE_MAXBUFFER,
> String.valueOf(ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT)
>   )
> );{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2251) Add Client side packet response timeout to avoid infinite wait.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15734817#comment-15734817
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2251:
---

Github user wuwen5 commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/119#discussion_r91682496
  
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1495,10 +1504,21 @@ public ReplyHeader submitRequest(RequestHeader h, 
Record request,
 Packet packet = queuePacket(h, r, request, response, null, null, 
null,
 null, watchRegistration, watchDeregistration);
 synchronized (packet) {
+long waitStartTime = System.currentTimeMillis();
 while (!packet.finished) {
-packet.wait();
+packet.wait(requestTimeout);
+if (!packet.finished && ((System.currentTimeMillis()
--- End diff --

System.currentTimeMillis() is dependent on System clock. It probably 
micro-corrected by an external programme..

I think using System.nanoTime() to measure elapsed time is the correct 
solution.


> Add Client side packet response timeout to avoid infinite wait.
> ---
>
> Key: ZOOKEEPER-2251
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2251
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.4.9, 3.5.2
>Reporter: nijel
>Assignee: Arshad Mohammad
>Priority: Critical
>  Labels: fault
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2251-01.patch, ZOOKEEPER-2251-02.patch, 
> ZOOKEEPER-2251-03.patch, ZOOKEEPER-2251-04.patch
>
>
> I came across one issue related to Client side packet response timeout In my 
> cluster many packet drops happened for some time.
> One observation is the zookeeper client got hanged. As per the thread dump it 
> is waiting for the response/ACK for the operation performed (synchronous API 
> used here).
> I am using 
> zookeeper.serverCnxnFactory=org.apache.zookeeper.server.NIOServerCnxnFactory
> Since only few packets missed there is no DISCONNECTED event occurred.
> Need add a "response time out" for the operations or packets.
> *Comments from [~rakeshr]*
> My observation about the problem:-
> * Can use tools like 'Wireshark' to simulate the artificial packet loss.
> * Assume there is only one packet in the 'outgoingQueue' and unfortunately 
> the server response packet lost. Now, client will enter into infinite 
> waiting. 
> https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java#L1515
> * Probably we can discuss more about this problem and possible solutions(add 
> packet ACK timeout or another better approach) in the jira.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-261) Reinitialized servers should not participate in leader election

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15734431#comment-15734431
 ] 

ASF GitHub Bot commented on ZOOKEEPER-261:
--

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/120
  
lgtm +1


> Reinitialized servers should not participate in leader election
> ---
>
> Key: ZOOKEEPER-261
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-261
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: leaderElection, quorum
>Reporter: Benjamin Reed
>
> A server that has lost its data should not participate in leader election 
> until it has resynced with a leader. Our leader election algorithm and 
> NEW_LEADER commit assumes that the followers voting on a leader have not lost 
> any of their data. We should have a flag in the data directory saying whether 
> or not the data is preserved so that the the flag will be cleared if the data 
> is ever cleared.
> Here is the problematic scenario: you have have ensemble of machines A, B, 
> and C. C is down. the last transaction seen by C is z. a transaction, z+1, is 
> committed on A and B. Now there is a power outage. B's data gets 
> reinitialized. when power comes back up, B and C comes up, but A does not. C 
> will be elected leader and transaction z+1 is lost. (note, this can happen 
> even if all three machines are up and C just responds quickly. in that case C 
> would tell A to truncate z+1 from its log.) in theory we haven't violated our 
> 2f+1 guarantee, since A is failed and B still hasn't recovered from failure, 
> but it would be nice if when we don't have quorum that system stops working 
> rather than works incorrectly if we lose quorum.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2251) Add Client side packet response timeout to avoid infinite wait.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15734534#comment-15734534
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2251:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/119#discussion_r91666945
  
--- Diff: src/java/main/org/apache/zookeeper/KeeperException.java ---
@@ -387,6 +389,8 @@ public void setCode(int code) {
 EPHEMERALONLOCALSESSION (EphemeralOnLocalSession),
 /** Attempts to remove a non-existing watcher */
 NOWATCHER (-121),
+/** Not received packet timely */
+TIMEOUT (-122),
--- End diff --

Similar error code needs to be added to C client to make both client 
library compatible.


> Add Client side packet response timeout to avoid infinite wait.
> ---
>
> Key: ZOOKEEPER-2251
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2251
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.4.9, 3.5.2
>Reporter: nijel
>Assignee: Arshad Mohammad
>Priority: Critical
>  Labels: fault
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2251-01.patch, ZOOKEEPER-2251-02.patch, 
> ZOOKEEPER-2251-03.patch, ZOOKEEPER-2251-04.patch
>
>
> I came across one issue related to Client side packet response timeout In my 
> cluster many packet drops happened for some time.
> One observation is the zookeeper client got hanged. As per the thread dump it 
> is waiting for the response/ACK for the operation performed (synchronous API 
> used here).
> I am using 
> zookeeper.serverCnxnFactory=org.apache.zookeeper.server.NIOServerCnxnFactory
> Since only few packets missed there is no DISCONNECTED event occurred.
> Need add a "response time out" for the operations or packets.
> *Comments from [~rakeshr]*
> My observation about the problem:-
> * Can use tools like 'Wireshark' to simulate the artificial packet loss.
> * Assume there is only one packet in the 'outgoingQueue' and unfortunately 
> the server response packet lost. Now, client will enter into infinite 
> waiting. 
> https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java#L1515
> * Probably we can discuss more about this problem and possible solutions(add 
> packet ACK timeout or another better approach) in the jira.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2251) Add Client side packet response timeout to avoid infinite wait.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15734533#comment-15734533
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2251:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/119#discussion_r91666999
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -1538,6 +1538,24 @@ public abstract class ServerAuthenticationProvider 
implements AuthenticationProv
 Specifies path to kinit binary. Default is 
"/usr/bin/kinit".
 
 
+
+zookeeper.request.timeout
+
+
+New in 3.5.3:
--- End diff --

This probably would not make to 3.5.3. 3.6.0 would be a more realistic 
value.


> Add Client side packet response timeout to avoid infinite wait.
> ---
>
> Key: ZOOKEEPER-2251
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2251
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.4.9, 3.5.2
>Reporter: nijel
>Assignee: Arshad Mohammad
>Priority: Critical
>  Labels: fault
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2251-01.patch, ZOOKEEPER-2251-02.patch, 
> ZOOKEEPER-2251-03.patch, ZOOKEEPER-2251-04.patch
>
>
> I came across one issue related to Client side packet response timeout In my 
> cluster many packet drops happened for some time.
> One observation is the zookeeper client got hanged. As per the thread dump it 
> is waiting for the response/ACK for the operation performed (synchronous API 
> used here).
> I am using 
> zookeeper.serverCnxnFactory=org.apache.zookeeper.server.NIOServerCnxnFactory
> Since only few packets missed there is no DISCONNECTED event occurred.
> Need add a "response time out" for the operations or packets.
> *Comments from [~rakeshr]*
> My observation about the problem:-
> * Can use tools like 'Wireshark' to simulate the artificial packet loss.
> * Assume there is only one packet in the 'outgoingQueue' and unfortunately 
> the server response packet lost. Now, client will enter into infinite 
> waiting. 
> https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java#L1515
> * Probably we can discuss more about this problem and possible solutions(add 
> packet ACK timeout or another better approach) in the jira.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2251) Add Client side packet response timeout to avoid infinite wait.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15734535#comment-15734535
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2251:
---

Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/119#discussion_r9147
  
--- Diff: src/java/main/org/apache/zookeeper/ClientCnxn.java ---
@@ -1495,10 +1504,21 @@ public ReplyHeader submitRequest(RequestHeader h, 
Record request,
 Packet packet = queuePacket(h, r, request, response, null, null, 
null,
 null, watchRegistration, watchDeregistration);
 synchronized (packet) {
+long waitStartTime = System.currentTimeMillis();
 while (!packet.finished) {
-packet.wait();
+packet.wait(requestTimeout);
--- End diff --

This unconditionally adds a timeout is a pretty significant semantic 
change. Whether or not we should wait indefinitely is a separate issue to 
discuss later, but from a compatibility point of view we'd at least provide a 
configuration option to let user opt in this feature rather than having this 
turned on by default. So by default there is no timeout and client still waits 
indefinitely, user who wants to opt in to turn on the timeout needs to explicit 
say so and provide a value they think make sense to them. Otherwise, we'd have 
to solve the problem of what the default timeout value we should set in ZK 
config if this feature is turned on by default, and that problem is hard and 
very likely there is no default value that makes everyone happy.


> Add Client side packet response timeout to avoid infinite wait.
> ---
>
> Key: ZOOKEEPER-2251
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2251
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: java client
>Affects Versions: 3.4.9, 3.5.2
>Reporter: nijel
>Assignee: Arshad Mohammad
>Priority: Critical
>  Labels: fault
> Fix For: 3.4.10, 3.5.3, 3.6.0
>
> Attachments: ZOOKEEPER-2251-01.patch, ZOOKEEPER-2251-02.patch, 
> ZOOKEEPER-2251-03.patch, ZOOKEEPER-2251-04.patch
>
>
> I came across one issue related to Client side packet response timeout In my 
> cluster many packet drops happened for some time.
> One observation is the zookeeper client got hanged. As per the thread dump it 
> is waiting for the response/ACK for the operation performed (synchronous API 
> used here).
> I am using 
> zookeeper.serverCnxnFactory=org.apache.zookeeper.server.NIOServerCnxnFactory
> Since only few packets missed there is no DISCONNECTED event occurred.
> Need add a "response time out" for the operations or packets.
> *Comments from [~rakeshr]*
> My observation about the problem:-
> * Can use tools like 'Wireshark' to simulate the artificial packet loss.
> * Assume there is only one packet in the 'outgoingQueue' and unfortunately 
> the server response packet lost. Now, client will enter into infinite 
> waiting. 
> https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java#L1515
> * Probably we can discuss more about this problem and possible solutions(add 
> packet ACK timeout or another better approach) in the jira.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2642) ZOOKEEPER-2014 breaks existing clients for little benefit

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15738368#comment-15738368
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2642:
---

GitHub user Randgalt opened a pull request:

https://github.com/apache/zookeeper/pull/122

[ZOOKEEPER-2642] Resurrect the reconfig() methods that were in 
ZooKeeper.java.

Resurrect the reconfig() methods that were in ZooKeeper.java. While 3.5.x 
is an alpha there are clients in production that are relying on the current 
specification. The reconfig() methods are marked deprecated to inform that they 
will be removed later. The "new" methods in the new class ZooKeeperAdmin were 
renamed reconfigure() to avoid getting the deprecation annotation inherited and 
to provide clarity. Both reconfig() and reconfigure() are implemented via 
protected methods in ZooKeeper.java

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Randgalt/zookeeper ZOOKEEPER-2642

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/122.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #122


commit 3611d18eb73bac5760293730087d2b0c3241b2bc
Author: randgalt 
Date:   2016-12-10T19:33:58Z

Resurrect the reconfig() methods that were in ZooKeeper.java. While 3.5.x 
is an alpha there are clients in production that are relying
on the current specification. The reconfig() methods are marked deprecated 
to inform that they will be removed later. The "new" methods
in the new class ZooKeeperAdmin were renamed reconfigure() to avoid getting 
the deprecation annotation inherited and to provide clarity.
Both reconfig() and reconfigure() are implemented via protected methods in 
ZooKeeper.java




> ZOOKEEPER-2014 breaks existing clients for little benefit
> -
>
> Key: ZOOKEEPER-2642
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2642
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: c client, java client
>Affects Versions: 3.5.2
>Reporter: Jordan Zimmerman
>
> ZOOKEEPER-2014 moved the reconfig() methods into a new class, ZooKeeperAdmin. 
> It appears this was done to document that these are methods have access 
> restrictions. However, this change breaks Apache Curator (and possibly other 
> clients). Curator APIs will have to be changed and/or special methods need to 
> be added. A breaking change of this kind should only be done when the benefit 
> is overwhelming. In this case, the same information can be conveyed with 
> documentation and possibly a deprecation notice.
> Revert the creation of the ZooKeeperAdmin class and move the reconfig() 
> methods back to the ZooKeeper class with additional documentation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-761) Remove *synchronous* calls from the *single-threaded* C clieant API, since they are documented not to work

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15735934#comment-15735934
 ] 

ASF GitHub Bot commented on ZOOKEEPER-761:
--

Github user fpj commented on the issue:

https://github.com/apache/zookeeper/pull/90
  
It sounds like there is a conflict in `TestReconfigServer`, could you 
resolve it, please @breed ?


> Remove *synchronous* calls from the *single-threaded* C clieant API, since 
> they are documented not to work
> --
>
> Key: ZOOKEEPER-761
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-761
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: c client
>Affects Versions: 3.1.1, 3.2.2
> Environment: RHEL 4u8 (Linux).  The issue is not OS-specific though.
>Reporter: Jozef Hatala
>Assignee: Benjamin Reed
>Priority: Minor
> Fix For: 3.5.3, 3.6.0
>
> Attachments: fix-sync-apis-in-st-adaptor.patch, 
> fix-sync-apis-in-st-adaptor.v2.patch
>
>
> Since the synchronous calls are 
> [known|http://hadoop.apache.org/zookeeper/docs/current/zookeeperProgrammers.html#Using+the+C+Client]
>  to be unimplemented in the single threaded version of the client library 
> libzookeeper_st.so, I believe that it would be helpful towards users of the 
> library if that information was also obvious from the header file.
> Anecdotally more than one of us here made the mistake of starting by using 
> the synchronous calls with the single-threaded library, and we found 
> ourselves debugging it.  An early warning would have been greatly appreciated.
> 1. Could you please add warnings to the doxygen blocks of all synchronous 
> calls saying that they are not available in the single-threaded API.  This 
> cannot be safely done with {{#ifdef THREADED}}, obviously, because the same 
> header file is included whichever client library implementation one is 
> compiling for.
> 2. Could you please bracket the implementation of all synchronous calls in 
> zookeeper.c with {{#ifdef THREADED}} and {{#endif}}, so that those symbols 
> are not present in libzookeeper_st.so?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746735#comment-15746735
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92269683
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
--- End diff --

Method name doesn't follow camel case convention (``nioSetup`` instead of 
``NIOSetup``).


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746737#comment-15746737
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92283709
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746744#comment-15746744
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92297692
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746747#comment-15746747
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92296352
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
--- End diff --

nit: it's nice to have an ordering on the definition of methods: public, 
protected, private. That is, it would be nice to move the private methods to 
the end of the file.


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() 

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746749#comment-15746749
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92298117
  
--- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java ---
@@ -694,7 +690,7 @@ public void sendResponse(ReplyHeader h, Record r, 
String tag) {
 }
 }
  } catch(Exception e) {
-LOG.warn("Unexpected exception. Destruction averted.", e);
+throw new IOException(e);
--- End diff --

Just a suggestion: wdyt about bubbling up a more custom message with the 
IOException instead of just encapsulate the Exception? I mean, something like:

``
throw new IOException("sendMessage exception: blah blah", e);
``


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746750#comment-15746750
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92296064
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/MockNettyServerCnxn.java ---
@@ -0,0 +1,65 @@
+/**
+ * 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;
+
+import org.apache.jute.Record;
+import org.apache.zookeeper.proto.ReplyHeader;
+import org.jboss.netty.channel.Channel;
+
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
+/**
+ * Helper class to test different scenarios in NettyServerCnxn
+ */
+public class MockNettyServerCnxn extends NettyServerCnxn {
--- End diff --

In this file, tab is indented with 2 spaces while the rest of the ZooKeeper 
files use 4 spaces (I only discovered this 'cause my IDE complained about it).


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746741#comment-15746741
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92297258
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746739#comment-15746739
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92282690
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
--- End diff --

It is a good practice (even tough I dunno we use consistently on ZK 
codebase) that we use an idiom like the one below:

```
  private String previousProperty = null;

  @Before
  public void setUp() {
 previous = System.getProperty(MY_PROPERTY_NAME);
 System.setProperty(MY_PROPERTY_NAME, "new_value");
  }

 @After
 public void tearDown() {
  System.setProperty(MY_PROPERTY_NAME, previousValue);
 }
```

This preserves the previous value of the ``System.property()``, **afaik**.


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746751#comment-15746751
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92297351
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746746#comment-15746746
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92297567
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746745#comment-15746745
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92296453
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
--- End diff --

Aren't we missing a ``try-catch`` around ``zk.close()`` with an explicit 
comment to ignore any error message?


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746733#comment-15746733
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92264074
  
--- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java ---
@@ -716,7 +716,12 @@ public void process(WatchedEvent event) {
 // Convert WatchedEvent to a type that can be sent over the wire
 WatcherEvent e = event.getWrapper();
 
-sendResponse(h, e, "notification");
+try {
+sendResponse(h, e, "notification");
+} catch (IOException ex) {
+LOG.debug("Problem sending to " + getRemoteSocketAddress(), 
ex);
--- End diff --

nit: I would use a modern debug format:

``
LOG.debug("Problem sending to {}", getRemoteSocketAddress(), ex);
``


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746748#comment-15746748
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92297538
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+   

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746738#comment-15746738
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92274772
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
--- End diff --

We usually put 1 minute (6 ms) timeout, why decrease here? If **I** 
understood correctly, test timeouts define a maximum execution threshold, but 
they won't execute faster if we decrease, right?


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: 

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746742#comment-15746742
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92269582
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
--- End diff --

Method name doesn't follow camel case convention (``nettySetup`` instead of 
``NettySetup``).


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746736#comment-15746736
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92274850
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
--- End diff --

Same as above: why decrease the timeout?


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746734#comment-15746734
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92296029
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
--- End diff --

In this file, tab is indented with 2 spaces while the rest of the ZooKeeper 
files use 4 spaces (I only discovered this 'cause my IDE complained about it).


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746731#comment-15746731
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92271380
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
--- End diff --

1. Why we need to log the message here?

2. Why we need to serialize ``cle``? Would it be ``cle.getMessage()``?

3. Wouldn't be better to let it throw the 
``KeeperException.ConnectionLossException`` and make the test catch it with 
``expected`` as below?

```
@Test(timeout = 6, expected = KeeperException.ConnectionLossException)
```


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> 

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746732#comment-15746732
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92268597
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
--- End diff --

I didn't get why we need the lines 87-90, because ``fail()`` throws an 
``AssertionError`` that will interrupt the processing flow, so those lines are 
effectively unreachable, right? There's should be nothing more after the 
`fail()`, I **guess**.

Also, I would suggest to put a more meaningful message, something along the 
lines of ``sendResponse should have thrown IOException and failed this test.``


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue 

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746743#comment-15746743
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92266393
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
--- End diff --

``exceptionType`` is never used!


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746740#comment-15746740
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92266304
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
--- End diff --

typo: ``scenarios``


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746729#comment-15746729
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user eribeiro commented on the issue:

https://github.com/apache/zookeeper/pull/99
  
I like this patch, but I think the whole reflection/mock thing is kind of 
reinventing a fault injection inside the test classes. If so, why not use a 
production ready framework as Byteman? I wrote a PR that strips the boilerplate 
stuff while leaving the feature of this PR: 
https://github.com/apache/zookeeper/pull/123 

Still a PoC, so any suggestions are welcome. :)


> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread
> --
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.5.1
>Reporter: Yuliya Feldman
>Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch, 
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549-5.patch, 
> ZOOKEEPER-2549.patch, ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it 
> can stop main ZK requests processing thread and make Zookeeper server look 
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() , 
> convert them to IOException and allow it propagating up



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1364) Add orthogonal fault injection mechanism/framework

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746928#comment-15746928
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1364:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/123#discussion_r92309188
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,160 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarios in sendResponse
+ */
+@RunWith(BMUnitRunner.class)
+public class ServerCxnExceptionsTest extends ClientBase {
+
+private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+private static String previousFactory = null;
+
+@BeforeClass
+public static void setUpBeforeClass() throws Exception {
+previousFactory = 
System.getProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+}
+
+@AfterClass
+public static void tearDownAfterClass() throws Exception {
+if (previousFactory != null) {
+
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, 
previousFactory);
+}
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
--- End diff --

switch annotations


> Add orthogonal fault injection mechanism/framework
> --
>
> Key: ZOOKEEPER-1364
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1364
> Project: ZooKeeper
>  Issue Type: Test
>  Components: tests
>Reporter: Andrei Savu
>Assignee: Andrei Savu
>
> Hadoop has a mechanism for doing fault injection (HDFS-435). I think it would 
> be useful if something similar would be available for ZooKeeper. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1364) Add orthogonal fault injection mechanism/framework

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746927#comment-15746927
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1364:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/123#discussion_r92309178
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,160 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarios in sendResponse
+ */
+@RunWith(BMUnitRunner.class)
+public class ServerCxnExceptionsTest extends ClientBase {
+
+private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+private static String previousFactory = null;
+
+@BeforeClass
+public static void setUpBeforeClass() throws Exception {
+previousFactory = 
System.getProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+}
+
+@AfterClass
+public static void tearDownAfterClass() throws Exception {
+if (previousFactory != null) {
+
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, 
previousFactory);
+}
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "IOExceptionNetty",
+targetClass = "org.apache.zookeeper.server.NettyServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new IOException(\"Test IOException from 
ServerCxnExceptionsTest with Netty\");",
+targetLocation = "AT ENTRY"
+)
+public void testIOExceptionNetty() throws Exception {
+tearDown();
+nettySetup();
+testZKHelper(true);
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
--- End diff --

switch annotations


> Add orthogonal fault injection mechanism/framework
> --
>
> Key: ZOOKEEPER-1364
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1364
> Project: ZooKeeper
>  Issue Type: Test
>  Components: tests
>Reporter: Andrei Savu
>Assignee: Andrei Savu
>
> Hadoop has a mechanism for doing fault injection (HDFS-435). I think it would 
> be useful if something similar would be available for ZooKeeper. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-1364) Add orthogonal fault injection mechanism/framework

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746925#comment-15746925
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1364:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/123#discussion_r92309054
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,160 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarios in sendResponse
+ */
+@RunWith(BMUnitRunner.class)
+public class ServerCxnExceptionsTest extends ClientBase {
+
+private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+private static String previousFactory = null;
+
+@BeforeClass
+public static void setUpBeforeClass() throws Exception {
+previousFactory = 
System.getProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+}
+
+@AfterClass
+public static void tearDownAfterClass() throws Exception {
+if (previousFactory != null) {
+
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, 
previousFactory);
+}
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "IOExceptionNetty",
+targetClass = "org.apache.zookeeper.server.NettyServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new IOException(\"Test IOException from 
ServerCxnExceptionsTest with Netty\");",
+targetLocation = "AT ENTRY"
+)
+public void testIOExceptionNetty() throws Exception {
+tearDown();
+nettySetup();
+testZKHelper(true);
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "IOExceptionNIO",
+targetClass = "org.apache.zookeeper.server.NIOServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new IOException(\"Test IOException from 
ServerCxnExceptionsTest with NIO\");",
+targetLocation = "AT ENTRY"
+)
+public void testIOExceptionNIO() throws Exception {
+tearDown();
+nioSetup();
+testZKHelper(true);
+}
+
+@Test(timeout = 6)
+public void testNoExceptionNetty() throws Exception {
+tearDown();
+nettySetup();
+testZKHelper(false);
+}
+
+@Test(timeout = 6)
+public void testNoExceptionNIO() throws Exception {
+tearDown();
+nioSetup();
+testZKHelper(false);
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "RuntimeException Netty",
+targetClass = "org.apache.zookeeper.server.NettyServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new RuntimeException(\"Test RuntimeException 
from ServerCxnExceptionsTest\")",
+targetLocation = "AT ENTRY"
+)
+public void testNettyRunTimeException() throws Exception {
+  

[jira] [Commented] (ZOOKEEPER-1364) Add orthogonal fault injection mechanism/framework

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15746924#comment-15746924
 ] 

ASF GitHub Bot commented on ZOOKEEPER-1364:
---

Github user eribeiro commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/123#discussion_r92309025
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,160 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarios in sendResponse
+ */
+@RunWith(BMUnitRunner.class)
+public class ServerCxnExceptionsTest extends ClientBase {
+
+private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+private static String previousFactory = null;
+
+@BeforeClass
+public static void setUpBeforeClass() throws Exception {
+previousFactory = 
System.getProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+}
+
+@AfterClass
+public static void tearDownAfterClass() throws Exception {
+if (previousFactory != null) {
+
System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, 
previousFactory);
+}
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "IOExceptionNetty",
+targetClass = "org.apache.zookeeper.server.NettyServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new IOException(\"Test IOException from 
ServerCxnExceptionsTest with Netty\");",
+targetLocation = "AT ENTRY"
+)
+public void testIOExceptionNetty() throws Exception {
+tearDown();
+nettySetup();
+testZKHelper(true);
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "IOExceptionNIO",
+targetClass = "org.apache.zookeeper.server.NIOServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new IOException(\"Test IOException from 
ServerCxnExceptionsTest with NIO\");",
+targetLocation = "AT ENTRY"
+)
+public void testIOExceptionNIO() throws Exception {
+tearDown();
+nioSetup();
+testZKHelper(true);
+}
+
+@Test(timeout = 6)
+public void testNoExceptionNetty() throws Exception {
+tearDown();
+nettySetup();
+testZKHelper(false);
+}
+
+@Test(timeout = 6)
+public void testNoExceptionNIO() throws Exception {
+tearDown();
+nioSetup();
+testZKHelper(false);
+}
+
+@Test(timeout = 6, expected = 
KeeperException.ConnectionLossException.class)
+@BMRule(name = "RuntimeException Netty",
+targetClass = "org.apache.zookeeper.server.NettyServerCnxn",
+targetMethod = "sendResponse",
+action = "throw new RuntimeException(\"Test RuntimeException 
from ServerCxnExceptionsTest\")",
+targetLocation = "AT ENTRY"
+)
+public void testNettyRunTimeException() throws Exception {
+  

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15747369#comment-15747369
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user yufeldman commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92326078
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+  

[jira] [Commented] (ZOOKEEPER-2549) As NettyServerCnxn.sendResponse() allows all the exception to bubble up it can stop main ZK requests processing thread

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15747370#comment-15747370
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2549:
---

Github user yufeldman commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/99#discussion_r92326091
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/ServerCxnExceptionsTest.java ---
@@ -0,0 +1,170 @@
+/**
+ * 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;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests to test different exceptions scenarious in sendResponse
+ */
+public class ServerCxnExceptionsTest extends ClientBase {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ServerCxnExceptionsTest.class);
+
+  private String exceptionType;
+
+  private void NettySetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NettyServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNettyServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  private void NIOSetup() throws Exception {
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY,
+  "org.apache.zookeeper.server.NIOServerCnxnFactory");
+System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN, 
"org.apache.zookeeper.server.MockNIOServerCnxn");
+System.setProperty("exception.type", "NoException");
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
+System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN);
+System.clearProperty("exception.type");
+  }
+
+  @Test (timeout = 6)
+  public void testNettyIOException() throws Exception {
+tearDown();
+NettySetup();
+testIOExceptionHelper();
+  }
+
+  @Test (timeout = 6)
+  public void testNIOIOException() throws Exception {
+tearDown();
+NIOSetup();
+testIOExceptionHelper();
+  }
+
+  private void testIOExceptionHelper() throws Exception {
+System.setProperty("exception.type", "IOException");
+super.setUp();
+final ZooKeeper zk = createClient();
+final String path = "/a";
+try {
+  // make sure zkclient works
+  zk.create(path, "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+CreateMode.EPHEMERAL);
+  fail("Should not come here");
+  Stat stats = zk.exists(path, false);
+  if (stats != null) {
+int length = stats.getDataLength();
+  }
+} catch (KeeperException.ConnectionLossException cle) {
+  LOG.info("ConnectionLossException: {}", cle);
+} finally {
+  zk.close();
+}
+  }
+
+  @Test (timeout = 1)
+  public void testNettyNoException() throws Exception {
+tearDown();
+NettySetup();
+testZKNoExceptionHelper();
+  }
+
+  @Test (timeout = 1)
+  public void testNIONoException() throws Exception {
+tearDown();
+NIOSetup();
+testZKNoExceptionHelper();
+  }
+
+  private void testZKNoExceptionHelper() throws Exception {
+System.setProperty("exception.type", "NoException");
+super.setUp();
+final ZooKeeper zk = createClient();
+