Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8345 )
Change subject: KUDU-2193 (part 1): switch to a waiting mutex in TSTabletManager ...................................................................... Patch Set 1: If we can guarantee that no other locks are taken while the TSTabletManager lock is held, I think we should keep it as a spinlock. Your part 2 patch ensures this in the PopulateReport functions. >From a quick scan, I see the following lock acquisitions: - RegisterTablet calls replica->tablet_metadata()->tablet_data_state() (acquires TabletMetadata.data_lock_. Should be moved outside the lock. - GetNumLiveTablets calls TabletReplica->state() (acquires TabletReplica.lock_). Also easily fixed if we make a copy of tablet_map_ under the lock. - RefreshTabletStateCacheAndReturnCount calls TabletReplica->state() also. That one is trickier, not 100% sure how to do it. Anyway, the offenders above are all spinlocks too, so I wouldn't expect to see the same long lock hold times. So maybe it's OK as is (with your part 2 fix)? -- To view, visit http://gerrit.cloudera.org:8080/8345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I763abddd74d8b1dabb618318dc84256b533077e3 Gerrit-Change-Number: 8345 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Fri, 20 Oct 2017 00:50:50 +0000 Gerrit-HasComments: No
