[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-26 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 ping. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-29 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt thanks for giving suggestions, there is another option: we create a new tracer interface, like AdvancedTracerDriver which will expose more metrics like we're doing in this PR. What's

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-04 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Please take a look when you guys have time :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-06 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 kindly ping. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-22 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Thanks @cammckenzie for review, the TimerTrace is actually an internal helper class, it shouldn't have any backwards compatible problem. It is the TracerDriver that may have backwards compatible

[GitHub] curator pull request #164: [CURATOR-349] Expose extra metrics in TracerDrive...

2016-09-20 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/164 [CURATOR-349] Expose extra metrics in TracerDriver You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin/curator CURATOR-349

[GitHub] curator pull request #164: [CURATOR-349] Expose extra metrics in TracerDrive...

2016-09-20 Thread lvfangmin
Github user lvfangmin closed the pull request at: https://github.com/apache/curator/pull/164 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] curator pull request #165: [CURATOR-349] Expose extra metrics in TracerDrive...

2016-09-20 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/165 [CURATOR-349] Expose extra metrics in TracerDriver You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin/curator CURATOR-349

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-28 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt with new methods in the existing classes will also changes the interface the users are using, it has the same side effect as this one, right? --- If your project is set up for it, you

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-30 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Thanks @Randgalt, I'll update the PR based on this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-30 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt I've updated the diff based what we discussed, please take a look when you have time. And have a great weekends! --- If your project is set up for it, you can reply to this email

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-09-26 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @cammckenzie Thanks again for reviewing the code, I think the millis is good enough for tracing. I'll wait @Randgalt's review :) --- If your project is set up for it, you can reply

[GitHub] curator issue #172: [CURATOR-349] Trace the session id even when it is disco...

2016-11-09 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/172 Btw, I also like to get this in the next release :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator issue #172: [CURATOR-349] Trace the session id even when it is disco...

2016-11-09 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/172 (Not related with this diff) @Randgalt , do we have the schedule of Curator release, are we still blocked by ZOOKEEPER-1525? The ZOOKEEPER-1525 looks good to me except the new

[GitHub] curator pull request #172: [CURATOR-349] Trace the session id even when it i...

2016-11-07 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/172#discussion_r86910642 --- Diff: curator-client/src/main/java/org/apache/curator/ConnectionState.java --- @@ -213,16 +213,14 @@ private synchronized void checkTimeouts() throws

[GitHub] curator issue #172: [CURATOR-349] Trace the session id even when it is disco...

2016-11-10 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/172 Thanks @Randgalt, please let me know when the new release is published 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] curator issue #169: [CURATOR-349] Expose extra metrics in TracerDriver (CURA...

2016-10-17 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/169 Rebased on CURATOR-3.0 branch, same as the code in https://github.com/apache/curator/pull/165. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Yes, feel free to join remote, we'll post the ways to join online soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt @cammckenzie, Facebook is going to hosted a Zookeeper Meetup, there will be tech talks, swags, drinks and snacks, please join us if you're interested in: https://www.facebook.com/events

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt we haven't started the unifying progress yet, since we were blocked by this feature, it would be better to publicize it on Curator's wiki when we're actually using it inside FB, I'll

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 @Randgalt @cammckenzie Thanks for reviewing the code, we're planning to unify the Zookeeper java client to be Curator inside Facebook, so it's really important for us to track the client side

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Cool, thanks @Randgalt, let me know when we decided the released schedule. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-20 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Hi @Randgalt, I've opened another PR for 3.0 branch, could you please take a look? Let me know if there is any issue. --- If your project is set up for it, you can reply to this email and have

[GitHub] curator issue #172: [CURATOR-349] Trace the session id even when it is disco...

2016-11-14 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/172 Hi @Randgalt, hope you have a great weekend :) Sorry about this, but could you please give me some updates about the release, so I can plan this weeks' work about whether I can start

[GitHub] curator issue #172: [CURATOR-349] Trace the session id even when it is disco...

2016-11-14 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/172 Ah, just saw it, thanks @Randgalt! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] curator pull request #172: [CURATOR-349] Use zero as the default session id ...

2016-11-01 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/172 [CURATOR-349] Use zero as the default session id to keep consistent with ZooKeeper The default session id inside ZooKeeper is 0, we should keep consistent with it in tracker. You can merge

[GitHub] curator pull request #172: [CURATOR-349] Use zero as the default session id ...

2016-11-01 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/172#discussion_r86048959 --- Diff: curator-client/src/main/java/org/apache/curator/ConnectionState.java --- @@ -213,16 +213,14 @@ private synchronized void checkTimeouts() throws

[GitHub] curator pull request #165: [CURATOR-349] Expose extra metrics in TracerDrive...

2016-10-10 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/165#discussion_r82691347 --- Diff: curator-client/src/main/java/org/apache/curator/ConnectionState.java --- @@ -199,13 +202,28 @@ private synchronized void checkTimeouts() throws

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-12 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Hi @Randgalt, in case you're not aware, I've updated the code base are you opinion, let me know if you have other comments, thanks. --- If your project is set up for it, you can reply

[GitHub] curator issue #165: [CURATOR-349] Expose extra metrics in TracerDriver

2016-10-17 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/165 Thanks @Randgalt, I've tried to merge this, there is not much conflict, I'll send a separate PR based on CURATOR-3.0 today. --- If your project is set up for it, you can reply to this email

[GitHub] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...

2017-03-27 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/198 Thanks @akira, the new patch looks good to me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator pull request #209: add fangmin to the developers section

2017-03-28 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/209 add fangmin to the developers section You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin/curator add_developer Alternatively you can

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-03-27 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 Test passed on my dev server: $ mvn clean test --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-03-19 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 I had run compile, it works, haven't checked tests though, I'll fix the tests. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] curator issue #208: [CURATOR-394] UnrecognizedPropertyException: "enabled" i...

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/208 I see, do we have the client version in the request, if we have it then maybe we can make the server code backward compatible based on the client version. --- If your project is set up

[GitHub] curator issue #208: [CURATOR-394] UnrecognizedPropertyException: "enabled" i...

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/208 One concern is that this diff changed the default behavior to not passing 'enable', which will break the users who are already using service discovery server and client with the enable field

[GitHub] curator pull request #209: add fangmin to the developers section

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/209#discussion_r109095137 --- Diff: pom.xml --- @@ -277,6 +277,16 @@ -8 https://people.apache.org/~enis

[GitHub] curator pull request #208: [CURATOR-394] UnrecognizedPropertyException: "ena...

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/208#discussion_r109095711 --- Diff: curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/NewServiceInstance.java --- @@ -0,0 +1,148

[GitHub] curator issue #209: add fangmin to the developers section

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/209 yeah, ZooKeeper is moving really slow, thanks for the background, I've merged the master into CURATOR-3.0, going to build the website and publish it. (Was building based on master, and found lots

[GitHub] curator issue #209: add fangmin to the developers section

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/209 @Randgalt thanks for the comment, I was just wondering how we manage the CURATOR-3.0, always keep the same as master? --- If your project is set up for it, you can reply to this email and have

[GitHub] curator issue #209: add fangmin to the developers section

2017-04-01 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/209 Figured out, Clirr maven plugin can fail on Java 8, the bug is fixed in version 2.8, I'll provide a PR. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] curator issue #209: add fangmin to the developers section

2017-03-31 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/209 @Randgalt I got the following error when compile the code on CURATOR-3.0 branch, is it a known issue, or it's because of my build environment issue: [ERROR] Failed to execute goal

[GitHub] curator pull request #210: upgrade clirr to fix the check error on java 8

2017-04-01 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/210 upgrade clirr to fix the check error on java 8 Currently, the `mvn compile` failed on CURATOR-3.0 branch, it turns out clirr maven plugin has bug and can fail on Java 8, the latest 2.8 version

[GitHub] curator pull request #203: [CURATOR-255] Check empty connection string in Fi...

2017-03-11 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/203 [CURATOR-255] Check empty connection string in FixedEnsembleProvider You can merge this pull request into a Git repository by running: $ git pull https://github.com/lvfangmin/curator

[GitHub] curator pull request #202: [CURATOR-391] Cannot use only Stat version to det...

2017-03-11 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/202#discussion_r105545433 --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java --- @@ -694,14 +695,23 @@ private void

[GitHub] curator pull request #204: [CURATOR-378] Update mvn parent pom, plugin and o...

2017-03-11 Thread lvfangmin
GitHub user lvfangmin opened a pull request: https://github.com/apache/curator/pull/204 [CURATOR-378] Update mvn parent pom, plugin and other dependency version Update the dependencies version to the version which is most widely used. You can merge this pull request into a Git

[GitHub] curator pull request #202: [CURATOR-391] Cannot use only Stat version to det...

2017-03-11 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/202#discussion_r105545598 --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java --- @@ -694,14 +695,23 @@ private void

[GitHub] curator pull request #198: [CURATOR-386] Allow listener to be passed in to P...

2017-03-13 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/198#discussion_r105545812 --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java --- @@ -117,6 +120,7 @@ public void processResult

[GitHub] curator pull request #204: [CURATOR-378] Update mvn parent pom, plugin and o...

2017-03-13 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/204#discussion_r105804055 --- Diff: pom.xml --- @@ -62,22 +62,22 @@ 3.4.8 - 2.7 -2.3.7 -2.10.3

[GitHub] curator pull request #235: [CURATOR-430] deletingChildrenIfNeeded maybe cann...

2017-08-15 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/235#discussion_r133328503 --- Diff: curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java --- @@ -313,7 +313,13 @@ public static void deleteChildren(ZooKeeper

[GitHub] curator pull request #216: CURATOR-402: Curator should not use QuorumServer....

2017-07-12 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/216#discussion_r126874138 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java --- @@ -170,8 +170,10 @@ public static String

[GitHub] curator pull request #205: [CURATOR-392] Fixing connection string constructi...

2017-07-13 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127384585 --- Diff: curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java --- @@ -313,6 +313,38 @@ public void testNewMembers

[GitHub] curator pull request #205: [CURATOR-392] Fixing connection string constructi...

2017-07-13 Thread lvfangmin
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/curator/pull/205#discussion_r127384661 --- Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java --- @@ -166,12 +166,26 @@ public static String

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-07-09 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 There are tests failure, those tests also failed before this patch on master, @Randgalt just saw you created Jira: CURATOR-419, is it related? I'll take a look later today. --- If your project

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-07-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 @Randgalt sorry for the delay, rebased onto master, the test is still running on my mac, will make sure everything passed. --- If your project is set up for it, you can reply to this email

[GitHub] curator pull request #204: [CURATOR-378] Update mvn parent pom, plugin and o...

2017-07-21 Thread lvfangmin
Github user lvfangmin closed the pull request at: https://github.com/apache/curator/pull/204 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-07-21 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 Has merged this into master. (Renamed my local branch to CURATOR-378, and forgot to update here, that's why this is not show as closed, will manually close it.) --- If your project

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-07-21 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 Sure, was waiting for a LGTM, I'll merge it today. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] curator issue #204: [CURATOR-378] Update mvn parent pom, plugin and other de...

2017-07-19 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/204 Rebase again, all the tests passed: [INFO] [INFO] Reactor Summary: [INFO] [INFO] Apache Curator

[GitHub] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...

2017-04-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/198 @akira sorry for the lately reply, I was waiting for another LGTM, the tests look good on my Mac, I'm going to merge the code now. --- If your project is set up for it, you can reply

[GitHub] curator issue #216: CURATOR-402: Curator should not use QuorumServer.addr wh...

2017-04-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/216 This fix looks good to time, but I'd like @Randgalt to accept it since he has more context about the changes he made in CURATOR-345. --- If your project is set up for it, you can reply

[GitHub] curator issue #214: CURATOR-401: Make InterProcessMutex.isOwnedByCurrentThre...

2017-04-24 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/214 LGTM +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] curator issue #208: [CURATOR-394] UnrecognizedPropertyException: "enabled" i...

2017-05-07 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/curator/pull/208 @betalb we cannot fully control the version of Curator being used on client side during gradually roll out, but I suppose we we can control the version of Curator service running on the server