Adar Dembo has posted comments on this change. Change subject: catalog_manager: fix unprotected data access in TableInfo::AddRemoveTablets ......................................................................
Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/master/catalog_manager-test.cc File src/kudu/master/catalog_manager-test.cc: PS2, Line 71: tablets.push_back(tablet); > If the call to table->AddRemoveTablets({}, tablets) at line 93 has gone, do Bear in mind that the TableInfo only has raw pointers to its TabletInfos. The expectation is that the CatalogManager stores strong refs to the TabletInfos in its maps; 'tablets' serves as a stand-in for those maps in this test. AFAICT, this is just test setup, so the AddRemoveTablets() call that dropped the tablets wasn't providing any value. http://gerrit.cloudera.org:8080/#/c/7995/2/src/kudu/util/cow_object.h File src/kudu/util/cow_object.h: PS2, Line 60: DCHECK(lock_.HasWriteLock()); > Why not to put that check right into WriteUnlock()/CommitUnlock()/UpgradeTo Good idea. I've added those checks into RWCLock itself, though I'm keeping these checks here because I think they're still useful (with the possibly exception of this particular check, though consistency bears out). -- To view, visit http://gerrit.cloudera.org:8080/7995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc5d3a9b985210f1fdd6f0495326fa3eb707841 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
