Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13264 )
Change subject: KUDU-2807 Crash when flush or compaction overlaps with another compaction ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13264/3/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/13264/3/src/kudu/tablet/tablet.cc@1058 PS3, Line 1058: UpdateAverageRowsetHeight(); Restricting this call to the locked version of the function doesn't seem like a particularly robust way to avoid calling it when swapping in the duplicating rowset. It also violates POLA in that people usually expect Foo and FooUnlocked to only differ in locking behavior. This is tough though, because ideally: 1. UpdateAverageRowsetHeight is called with every atomic rowset swap. 2. It may be called with component_lock_ held or not. 3. It'll take component_lock_ if necessary to get the list of components. 4. But then it drops the lock while recomputing the rowset height (at that point only compact_select_lock_ is needed). -- To view, visit http://gerrit.cloudera.org:8080/13264 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic255f0466aa2c158fa32e8e38428eddfcf901b99 Gerrit-Change-Number: 13264 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 08 May 2019 22:52:38 +0000 Gerrit-HasComments: Yes
