[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user phunt commented on the issue: https://github.com/apache/zookeeper/pull/466 +1, lgtm. Also tried running against a runtime cluster and it worked ok. committed to 3.5.5 and master. Thanks @anmolnar . ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @phunt Rebased and conflicts resolved. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user phunt commented on the issue: https://github.com/apache/zookeeper/pull/466 Currently listed as having a conflict, @anmolnar can you update the patch? ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @phunt We got a green build here. If you're happy with the change, it's ready to commit. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user eribeiro commented on the issue: https://github.com/apache/zookeeper/pull/466 Cool. :) FYI, https://issues.apache.org/jira/browse/ZOOKEEPER-2693 Particularly, this quote: _"A simple patch would provide a configuration option that disables these 4lw by default. We should include this in 3.4/3/5 code lines. Additionally in 3.5 Jetty metrics support was added, replacing the legacy 4lw. In 3.5.3 and later we should disable 4lw entirely (through a second config option) and encourage people to use Jetty instead."_ ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @eribeiro Thanks. In which case given that this PR is targeted for the master branch, I'll add them to Jetty and JMX commands. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user eribeiro commented on the issue: https://github.com/apache/zookeeper/pull/466 @anmolnar Patrick is mentioning the plan to retire 4lw CLI commands in favor of Restful web service version (based on Jetty). See https://issues.apache.org/jira/browse/ZOOKEEPER-1346 ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @phunt Sure, just let me try to clarify what needs to be done, because I think you wanted to link #415 instead of #466 which is this PR. - stats should be added to JMX (done, already in ZooKeeperBean), - stats should be added to stat and mntr 4lw commands (also done) What am I missing? What's legacy exactly? Also would we like to add Jetty support here, I've seen it mentioned in the other PR? ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user phunt commented on the issue: https://github.com/apache/zookeeper/pull/466 @anmolnar should we apply the same logic as my recent comment on #466 ? Add this to jmx and the mntr command output, but don't add it to the legacy/text 4lw. What do you think. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @phunt This is definitely for 3.4 too. I updated the Jira and will create a separate pull request for 3.4. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user phunt commented on the issue: https://github.com/apache/zookeeper/pull/466 is this intended for 3.4 or just 3.5+ ? The jira says 3.5+, is that right? ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @rakeshadr thanks, this is done. ---
[GitHub] zookeeper issue #466: ZOOKEEPER-2940. Deal with maxbuffer as it relates to l...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/466 @rakeshadr I updated the pull request with your code review comments: - stats class has been renamed, - added comments to unit test asserts, - replaced initial value magic number with a constant. Please take a look. ---