You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@fluo.apache.org by GitBox <gi...@apache.org> on 2020/01/29 02:09:51 UTC

[GitHub] [fluo-muchos] billierinaldi opened a new pull request #316: Add support for ZooKeeper 3.5

billierinaldi opened a new pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] karthick-rn commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-581659678
 
 
   > It seems both 3.4 and 3.5 have a lib dir, but we are not currently adding ZK/lib in accumulo-env.sh. So I suspect the ZK 3.4 dependencies are currently supplied elsewhere on the classpath.
   
   The current classpath in `accumulo-env.sh` is correct, it is not required to add ZK/lib because in 3.4 the tarball extract places `zookeeper-3.4.14.jar` in `zookeeper_home` directory and not in lib dir. This is different in 3.5 where `zookeeper-3.5.6.jar` is placed inside the lib dir.
   I have tested both the options & it works.
   a) Add ZK/lib in `accumulo-env.sh` 
   b) Copy zookeeper*.jar from lib to `zookeeper_home` directory 
   
   > If not we always have the option of making ZK 3.5 the minimum supported version of ZK for muchos (we can add a sanity check during setup). If this makes maintaining muchos simpler, it may be a good way to go.
   
   Agree
   
   > This may not be an issue for Accumulo. I think the order in which items are listed on the classpath is important for deciding which classes are used (see specification order section at the end of this doc).
   
   It makes sense & in which case I think we don't need to worry about the zookeeper*.jar that ships with Hadoop. Btw, thanks for the link. 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372161473
 
 

 ##########
 File path: ansible/roles/zookeeper/templates/zoo.cfg
 ##########
 @@ -15,6 +15,10 @@ clientPort={{ zookeeper_client_port }}
 # the maximum number of client connections.
 # increase this if you need to handle more clients
 maxClientCnxns=100
+# support zk monitoring
+{% if zookeeper_version.startswith('3.5') %}
+4lw.commands.whitelist=mntr
+{% endif %}
 
 Review comment:
   I assume this config was added new in 3.5, but it is probably ignored in earlier versions. If so, there's probably not a need for the conditional, and I would instead just put update the comment:
   
   ```ini
   # support zk monitoring (for zk >= 3.5)
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r373072195
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -182,6 +183,10 @@ def _verify_config(self, action):
             if self.java_product_version() >= 11 and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) <= StrictVersion("1.9.3"):
                 exit("ERROR - Java 11 is not supported with Accumulo version '{0}'".format(self.version('accumulo')))
 
+            # validate and fail if we are using ZooKeeper 3.5.5 or greater and Accumulo 1.9.x or less
+            if StrictVersion(self.version('zookeeper')) >= StrictVersion("3.5.5") and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) < StrictVersion("1.10.0"):
 
 Review comment:
   The issues are minor, possibly a good first issue for someone, so I am thinking they might get fixed. The zookeeper jar is now in a lib directory, so start-all.sh and general.classpaths have to be updated. (I am not sure whether general.classpaths should be updated in Accumulo itself or in Fluo-Muchos, but at least the ZK jar check in start-all.sh would need to be fixed.)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-579875629
 
 
   I think 3.5 being default makes sense. I think it'd be good to have more testing on that, so we can build confidence recommending its use.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-581499134
 
 
   I am still on the fence about linking the jars into `ZOOKEEPER_HOME` vs. updating the classpath. I am also wondering if we should refine the addition of `${HADOOP_HOME}/share/hadoop/common/lib/*` to the classpath and only include the specific jars required for ADLS Gen2. The classpath for Accumulo 2.0 has purposely reduced dependencies on Hadoop lib jars (https://github.com/apache/accumulo/pull/614) and this approach would avoid having two ZooKeeper jars on the Accumulo classpath.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372164137
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -73,8 +73,8 @@
   'worker_data_dirs': None,
   'zookeeper_connect': "{% for host in groups['zookeepers'] %}{{ host }}:2181{% if not loop.last %},{% endif %}{% endfor %}",
   'zookeeper_client_port': '"2181"',
-  'zookeeper_home': '"{{ install_dir }}/zookeeper-{{ zookeeper_version }}"',
-  'zookeeper_tarball': 'zookeeper-{{ zookeeper_version }}.tar.gz',
+  'zookeeper_home': "{% if zookeeper_version.startswith('3.5') %}{{ install_dir }}/apache-zookeeper-{{ zookeeper_version }}-bin{% else %}{{ install_dir }}/zookeeper-{{ zookeeper_version }}{% endif %}",
+  'zookeeper_tarball': "{% if zookeeper_version.startswith('3.5') %}apache-zookeeper-{{ zookeeper_version }}-bin.tar.gz{% else %}zookeeper-{{ zookeeper_version }}.tar.gz{% endif %}",
 
 Review comment:
   Since the part that is different here is the same that is different in the `zookeeper_home` variable, it might make sense to put that in a new variable that can be injected into these two.
   
   Not sure if this would work, but maybe something like:
   
   ```suggestion
     'zk_naming': "{% if zookeeper_version.startswith('3.5') %}apache-zookeeper-{{ zookeeper_version }}-bin{% else %}zookeeper-{{ zookeeper_version }}{% endif %}",
     'zookeeper_home': "{{ install_dir }}/{{ zk_naming }}",
     'zookeeper_tarball': "{{ zk_naming }}.tar.gz",
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372161493
 
 

 ##########
 File path: conf/checksums
 ##########
 @@ -28,6 +28,8 @@ spark:2.3.4:0241cbe73d3ad4fec1c655d839facc16e647ff9a3825f51c78d9fb270a753da4
 spark:2.3.3:a5c4a54db5163df25110534f36e13d6ae65758d45d1bc8bfcd7e27304e716a23
 spark:2.3.2:3387107155d62f04ccf6bcaf2e00a69a0de5ae5df875348d93147743c206f0a8
 spark:2.2.2:023b2fea378b3dd0fee2d5d1de6bfaf2d8349aefe7be97a9cbcf03bbacc428d7
+zookeeper:3.5.6:db24700e453f2ad4b6b789d553fe828ccceef3977a7e6b36389580ec92397a7e
+zookeeper:3.5.5:c5ff531cbda56c157199ab80632dc50ffefa8b7cbe866a0431345d3c4d72bbd1
 
 Review comment:
   Adding 3.5.5 here seems like it's encouraging the use of a version that has known bugs already fixed in 3.5.6, which people should use instead. Is there a good reason to add 3.5.5 here, or is it just being added for "completeness"? If the latter, I would leave it out... the only reason we have so many listed already is because they built up over time. We should probably be pruning this list, especially if the listed versions have been aged out of the ASF mirrors system.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] karthick-rn commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-580935148
 
 
   We can either update the `CLASSPATH` in `accumulo-env.sh` to include zookeeper lib directory or copy the zookeeper-3.5.6.jar from `lib` to `ZOOKEEPER_HOME` directory. I prefer the latter, simply because it won't change the `CLASSPATH` & any previous Zookeeper versions would still work. 
   Also, I found there is another jar `zookeeper-jute-3.5.6.jar` which should be copied to `ZOOKEEPER_HOME` directory, otherwise Accumulo fails to start with `java.lang.ClassNotFoundException` exception.
   We may also have to copy these 2 zookeeper*.jar files to hadoop common lib & hadoop hdfs lib to make sure the same version of Zookeeper is being picked across the cluster. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372162787
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -73,8 +73,8 @@
   'worker_data_dirs': None,
   'zookeeper_connect': "{% for host in groups['zookeepers'] %}{{ host }}:2181{% if not loop.last %},{% endif %}{% endfor %}",
   'zookeeper_client_port': '"2181"',
-  'zookeeper_home': '"{{ install_dir }}/zookeeper-{{ zookeeper_version }}"',
-  'zookeeper_tarball': 'zookeeper-{{ zookeeper_version }}.tar.gz',
+  'zookeeper_home': "{% if zookeeper_version.startswith('3.5') %}{{ install_dir }}/apache-zookeeper-{{ zookeeper_version }}-bin{% else %}{{ install_dir }}/zookeeper-{{ zookeeper_version }}{% endif %}",
 
 Review comment:
   `{{ install_dir }}/` is a common prefix for both cases, so I would put it outside the conditional to simplify it slightly.
   
   ```suggestion
     'zookeeper_home': "{{ install_dir }}/{% if zookeeper_version.startswith('3.5') %}apache-zookeeper-{{ zookeeper_version }}-bin{% else %}zookeeper-{{ zookeeper_version }}{% endif %}",
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-580871095
 
 
   I found another classpath issue that needs to be addressed for ZooKeeper 3.5. In accumulo-env.sh the ZooKeeper lib directory is not on the classpath. I could not understand why my test run worked anyway, but then I noticed that I was running with use_adlsg2 = True and in that case Accumulo is picking up the ZooKeeper jar from the hadoop common lib dir. https://github.com/apache/fluo-muchos/blob/96e79dec692d20ab283d6ac529d141056f27db9e/ansible/roles/accumulo/templates/accumulo-env.sh#L43-L47 @ctubbsii @arvindshmicrosoft @karthick-rn 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-580018213
 
 
   > @arvindshmicrosoft pointed out that additional work needs to be done for Accumulo 1.9 to work with ZooKeeper 3.5, so I will add validation that those have not been configured together.
   
   Sounds reasonable. I haven't tried 3.5 yet on any version.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] keith-turner commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-581618750
 
 
   I took a look at what is in the zookeeper 3.4 and 3.5 dirs and found found the following. The contents of ZK/lib differs a lot between 3.4 and 3.5. 
   
   ```bash
   $ ls zookeeper-3.4.14/**/*.jar
   zookeeper-3.4.14/dist-maven/zookeeper-3.4.14.jar          zookeeper-3.4.14/lib/audience-annotations-0.5.0.jar  zookeeper-3.4.14/lib/slf4j-api-1.7.25.jar
   zookeeper-3.4.14/dist-maven/zookeeper-3.4.14-javadoc.jar  zookeeper-3.4.14/lib/jline-0.9.94.jar                zookeeper-3.4.14/lib/slf4j-log4j12-1.7.25.jar
   zookeeper-3.4.14/dist-maven/zookeeper-3.4.14-sources.jar  zookeeper-3.4.14/lib/log4j-1.2.17.jar
   zookeeper-3.4.14/dist-maven/zookeeper-3.4.14-tests.jar    zookeeper-3.4.14/lib/netty-3.10.6.Final.jar
   $ ls apache-zookeeper-3.5.6-bin/**/*.jar
   apache-zookeeper-3.5.6-bin/lib/audience-annotations-0.5.0.jar       apache-zookeeper-3.5.6-bin/lib/log4j-1.2.17.jar
   apache-zookeeper-3.5.6-bin/lib/commons-cli-1.2.jar                  apache-zookeeper-3.5.6-bin/lib/netty-buffer-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jackson-annotations-2.9.10.jar       apache-zookeeper-3.5.6-bin/lib/netty-codec-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jackson-core-2.9.10.jar              apache-zookeeper-3.5.6-bin/lib/netty-common-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jackson-databind-2.9.10.jar          apache-zookeeper-3.5.6-bin/lib/netty-handler-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/javax.servlet-api-3.1.0.jar          apache-zookeeper-3.5.6-bin/lib/netty-resolver-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-http-9.4.17.v20190418.jar      apache-zookeeper-3.5.6-bin/lib/netty-transport-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-io-9.4.17.v20190418.jar        apache-zookeeper-3.5.6-bin/lib/netty-transport-native-epoll-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-security-9.4.17.v20190418.jar  apache-zookeeper-3.5.6-bin/lib/netty-transport-native-unix-common-4.1.42.Final.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-server-9.4.17.v20190418.jar    apache-zookeeper-3.5.6-bin/lib/slf4j-api-1.7.25.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-servlet-9.4.17.v20190418.jar   apache-zookeeper-3.5.6-bin/lib/slf4j-log4j12-1.7.25.jar
   apache-zookeeper-3.5.6-bin/lib/jetty-util-9.4.17.v20190418.jar      apache-zookeeper-3.5.6-bin/lib/zookeeper-3.5.6.jar
   apache-zookeeper-3.5.6-bin/lib/jline-2.11.jar                       apache-zookeeper-3.5.6-bin/lib/zookeeper-jute-3.5.6.jar
   apache-zookeeper-3.5.6-bin/lib/json-simple-1.1.1.jar
   ```
   
   It seems both 3.4 and 3.5 have a lib dir, but we are not currently adding ZK/lib in accumulo-env.sh.  So I suspect the ZK 3.4 dependencies are currently supplied elsewhere on the classpath.  @karthick-rn suggested possibly always adding ZK/lib to the classpath, does anyone know if this works?
   
   >  I prefer the latter, simply because it won't change the CLASSPATH & any previous Zookeeper versions would still work.
   
   Are there good reasons to support ZK 3.4 and 3.5?  If not we always have the option of making ZK 3.5 the minimum supported version of ZK for muchos (we can add a sanity check during setup).  If this makes maintaining muchos simpler, it may be a good way to go.
   
   > We may also have to copy these 2 zookeeper*.jar files to hadoop common lib & hadoop hdfs lib to make sure the same version of Zookeeper is being picked across the cluster.
   
   This may not be an issue for Accumulo. I think the order in which items are listed on the classpath is important for deciding which classes are used (see specification order section at the end of [this doc](https://docs.oracle.com/javase/8/docs/technotes/tools/windows/classpath.html)).  So if two ZK versions are listed on the classpath, the one listed first will be chosen.  So if accumulo-env.sh puts ZK_HOME first on the classpath, then maybe it will always use that version even if Hadoop has it. However, not sure if this holds for java 11.
   
   >  I am also wondering if we should refine the addition of ${HADOOP_HOME}/share/hadoop/common/lib/* to the classpath and only include the specific jars required for ADLS Gen2.
   
   One tricky thing with that is when you change the version of Hadoop, the versions of deps will change and their transitive deps may also change.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii edited a comment on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii edited a comment on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-580018213
 
 
   > @arvindshmicrosoft pointed out that additional work needs to be done for Accumulo 1.9 to work with ZooKeeper 3.5, so I will add validation that those have not been configured together.
   
   Sounds reasonable. I haven't tried 3.5 yet on any version and wasn't aware there were known issues.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi merged pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi merged pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-579964108
 
 
   @arvindshmicrosoft pointed out that additional work needs to be done for Accumulo 1.9 to work with ZooKeeper 3.5, so I will add validation that those have not been configured together.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] billierinaldi commented on issue #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
billierinaldi commented on issue #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#issuecomment-579821865
 
 
   Thanks for the review @ctubbsii! I agree with your suggestions and will get working on those. I wanted to mention that I made the default version 3.5.6 in this patch because the 3.5.5 release notes say that it is recommended for production, but if anyone prefers the default version to stay 3.4 let me know.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] arvindshmicrosoft commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r373124214
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -182,6 +183,10 @@ def _verify_config(self, action):
             if self.java_product_version() >= 11 and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) <= StrictVersion("1.9.3"):
                 exit("ERROR - Java 11 is not supported with Accumulo version '{0}'".format(self.version('accumulo')))
 
+            # validate and fail if we are using ZooKeeper 3.5.5 or greater and Accumulo 1.9.x or less
+            if StrictVersion(self.version('zookeeper')) >= StrictVersion("3.5.5") and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) < StrictVersion("1.10.0"):
 
 Review comment:
   @ctubbsii one of us will submit a PR for the start-all.sh script early next week.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372161473
 
 

 ##########
 File path: ansible/roles/zookeeper/templates/zoo.cfg
 ##########
 @@ -15,6 +15,10 @@ clientPort={{ zookeeper_client_port }}
 # the maximum number of client connections.
 # increase this if you need to handle more clients
 maxClientCnxns=100
+# support zk monitoring
+{% if zookeeper_version.startswith('3.5') %}
+4lw.commands.whitelist=mntr
+{% endif %}
 
 Review comment:
   I assume this config was added new in 3.5, but it is probably ignored in earlier versions. If so, there's probably not a need for the conditional, and I would instead just put it in and update the comment:
   
   ```ini
   # support zk monitoring (for zk >= 3.5)
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #316: Add support for ZooKeeper 3.5
URL: https://github.com/apache/fluo-muchos/pull/316#discussion_r372694734
 
 

 ##########
 File path: lib/muchos/config/base.py
 ##########
 @@ -182,6 +183,10 @@ def _verify_config(self, action):
             if self.java_product_version() >= 11 and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) <= StrictVersion("1.9.3"):
                 exit("ERROR - Java 11 is not supported with Accumulo version '{0}'".format(self.version('accumulo')))
 
+            # validate and fail if we are using ZooKeeper 3.5.5 or greater and Accumulo 1.9.x or less
+            if StrictVersion(self.version('zookeeper')) >= StrictVersion("3.5.5") and StrictVersion(self.version('accumulo').replace('-SNAPSHOT','')) < StrictVersion("1.10.0"):
 
 Review comment:
   If it doesn't work with 1.9, I wouldn't expect it to work with 1.10 either... but perhaps that will be fixed before released.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services