Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 )
Change subject: KUDU-2797 p2: the master aggregates tablet statistics ...................................................................... Patch Set 27: (3 comments) http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@735 PS25, Line 735: > I just want to check the stats after leader-follower switch. So, it's neces Right, I meant changing these values to lower values so that the test doesn't take as long to run, since these Sleep() statements account for 1.5s each of test runtime. It's not that important, since it's only a few seconds total, but it could we could set the heartbeat interval to 100ms or something. http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc@732 PS27, Line 732: NO_FATALS(CheckStats(kRowsCount)); >From the test failure, it seems like this isn't getting updated for some >reason. I looked into it a bit, but haven't gotten the root cause. It seems >like all the tablet servers reported to the master, but the live row count >ended up being 0. The test failure was in TSAN mode, so it might be that the failure only triggers under slower environments. Running this test with --stress_cpu_threads=<some number> might help in debugging. http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc@831 PS25, Line 831: : // The following line of code should precede "lock_", : // otherwise a deadlock will result. > Line1402 in raft_consensus.cc and Line606 in tablet_replica.cc I see. There's a comment in TabletReplica::Start() that explains this a bit too. Could you update this to explain a bit more? Something like: We cannot hold 'lock_' while calling RaftConsensus::role() because it may invoke TabletReplica::StartFollowerTransaction() and lead to a deadlock. -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 27 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Wed, 07 Aug 2019 02:03:57 +0000 Gerrit-HasComments: Yes
