Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
Got a [green
build](https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193/).
Merging.
---
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
Thanks for rebase, @jtuple. Latest build failed, but those are flaky tests.
Kicking Jenkins again to get a green build.
@anmolnar Could you please also sign this off? I believe you raised a
Github user jtuple commented on the issue:
https://github.com/apache/zookeeper/pull/580
Rebased to resolve merge conflict.
---
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
From the conversation so far, it looks like we have reached a consensus
regarding the directions of this patch and future metrics related patch. For
this specific patch, it looks good to me, and I
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
Thanks for detailed reply, @jtuple.
Jenkins was not happy because one C client test failed (the 'Test Result'
only shows Java tests). I kicked a
[new
Github user jtuple commented on the issue:
https://github.com/apache/zookeeper/pull/580
Sorry for the delay, August was a super busy month.
I've rebased onto latest master + added basic unit tests for
`AvgMinMaxCounter` and `SimpleCounter`. I also resolved the findbugs issue
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
And yes like others commented, please rebase and resolve conflicts, and
reply to previous unattended comments. My previous comments need address are:
* Existing unit test failures and one
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
One question about gathering metrics using this system: let's say I have a
use case where I want to gather the number of requests processed by
FinalRequestProcessor for each request type (e.g. Ping,
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
We at Twitter manage large ZK clusters and we also have our own internal
metrics system (you can't really operate things like ZK without metrics at
large scale.). Our design is similar like this in
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/580
@jtuple do you have time to resolve the comments and rebase this onto
latest branch? It would be great if we can get this in this week.
---
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/580
Thanks @anmolnar, I've answered some of the questions here.
@jtuple can you resolve some of these comments and the conflict, we might
ready to go now, this will unblock other patches
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/580
@lvfangmin @eolivelli Thanks for the clarification. Let's do it the way
you're suggesting.
@jtuple I'm happy to commit your patch once you answered my questions and
resolved the conflicts.
Github user eolivelli commented on the issue:
https://github.com/apache/zookeeper/pull/580
I think it will be easier for FB guys to merge their work to community repo
and then we can move to the new system.
Otherwise the two forks will never converge.
So I will lean
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/580
@anmolnar If I understand correctly, @eolivelli's work is more about making
it easier to export ZooKeeper's internal metric to external systems, while FB
is focusing on adding a lot more useful
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/580
@eolivelli
I got that this is Facebook work on adding new metrics and at the same time
you're working on a generic long term solution for ZooKeeper metrics system.
My point is that
Github user eolivelli commented on the issue:
https://github.com/apache/zookeeper/pull/580
@anmolnar this change is a backport from Facebook fork.
in PR #582 I have started a brand new work, to design and implement a long
term ZK internal API to instrument server and client
Github user eolivelli commented on the issue:
https://github.com/apache/zookeeper/pull/580
@jtuple in my opinion it is better to allow all of changes coming from
Facebook to be merged in as soon as possible.
Having only one common codebase is necessary in order to work together.
Github user jtuple commented on the issue:
https://github.com/apache/zookeeper/pull/580
I'm happy to add tests for `AvgMinMaxCounter` and `SimpleCounter`, will go
ahead and update this PR today to include that for completeness. Since it's
easy change to make while we discuss any
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/580
@eolivelli @jtuple
I think the best would be to join the efforts and work together to
establish the new framework and implement these new metrics with a suitable
library like dropwizard. At
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/580
Finally this happens, thanks for contributing the change back @jtuple . My
quick comments inline.
In addition, there are two things:
* There is find bug warning. I believe it's the txn
20 matches
Mail list logo
