[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/628 Merged into master, thanks @enixon! ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/628 Thanks @enixon for doing the update and rebase, went through this again, looks legit to me. I also compared with internal version and made sure this has included all the improvement and bug fixes. Maybe consider to add the om_proposal_process_time_ms and om_commit_process_time_ms metric, but this is not a must have. +1 this should be ready to go. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/628 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2605/ ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/628 @enixon @hanm I think that's the same `free()` issue. My feeling is that it must be somewhere in test cleanup section (if there's any) when it tries to free some resource twice. The that I previously submitted was unrelated and I closed the PR without merging it. So, the issue is still open. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 The tests that fail are not entirely consistent. I've tried disabling TestLogClientEnv.cc, TestReadOnlyClient.cc, TestReconfigServer.cc, and TestWatchers.cc and this seems to work some of the time. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/628 >> This patch does not touch the c client or the default configurations for those tests so I'm unsure how to proceed. My feeling is the failure is a flaky test, and has nothing to do with this patch. Though, it would be good if we can identify the exact failing test case, and rule out the possibility that it's caused by this patch (since C client depends on same java server code.). Also, sorry for lagging on following up my previous review. I am resuming reviewing this patch this week. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/628 I am looking at https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2486/console, it was not clear to me which C test failed, aside from the ``./zktest-mt': free(): invalid pointer: 0x2b00d6385000` in the end. Note from the console log, it has: `[exec] Zookeeper_simpleSystem::testRemoveWatchers ZooKeeper server started : elapsed 4610 : OK`, so this might be a different failure that @anmolnar was fixing. Do we know which C test case is failing here? ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 @anmolnar : I see that you closed #660 without merging. Given that we're guessing it is the cause for the remaining test failures of this PR, is there something that I can do to to help address ZOOKEEPER-2320? ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/628 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2486/ ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/628 @enixon It is. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 @anmolnar the error in the C-client tests is: *** Error in `./zktest-mt': free(): invalid pointer: 0x2b971c71e000 *** Not sure if its the same issue as you're seeking to patch. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/628 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2422/ ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/628 @enixon If that was the free(): invalid pointer issue in testRemoveWatchers, I have a patch for that, but needs to be reviewed. #660 (and it's still failing to pass all tests) ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 Any suggestions on the C zktest-mt failures? This patch does not touch the c client or the default configurations for those tests so I'm unsure how to proceed. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 Going to fix this patch up by rebasing and reworking the base diff to operate against zookeeper-server/src/ and zookeeper-common/src/test instead of the old paradigm src/java. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/628 Looks like ObserverMasterTest.testAdminCommands is failing in the latest Jenkins build, @enixon can you take a look? ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/628 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2277/ ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user asfgit commented on the issue: https://github.com/apache/zookeeper/pull/628 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2276/ ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 The failing C tests seemed to come from TestMulti.cc and TestReadOnlyClient.cc but all tests in both suites passed when I ran them individually. I'm curious if Jenkins will clear them on this pass, otherwise it is probably some issue of cleanup when several tests are run back to back. ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/628 jenkins ran both java and C client tests. In this case, it's one of C client tests failed. `[exec] [exec] *** Error in `./zktest-mt': free(): invalid pointer: 0x2b971c71e000 ***` https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2184/console (scroll down) ---
[GitHub] zookeeper issue #628: ZOOKEEPER-3140: Allow Followers to host Observers
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/628 I'm not sure how to interpret the Jenkins results - it looks like all tests passed. ---
