Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16580 )
Change subject: KUDU-3149: don't block op registration on MM mutex ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/16580/8/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: http://gerrit.cloudera.org:8080/#/c/16580/8/src/kudu/util/maintenance_manager.cc@283 PS8, Line 283: CHECK_GT(num_erased_ops, 0); > Yep, but if it can be called concurrently, then it's possible to could cras One scenario that can lead to a crash on this CHECK() is the following: * have an op which is running * two threads call MaintenanceManager::UnregisterOp() on the op * one thread is awoken after waiting on the condition variable, and it removes the op and doesn't get into the 'if (ops_.erase(op) == 0)' clause * the other thread is awoken, and gets into the 'if (ops_.erase(op) == 0)' clause Does it make sense? If so, maybe we should remove the CHECK() and all if we expect this method to be called concurrently by multiple threads? Overall, what sort of invariant we want to preserve adding this CHECK_GT() at line 283? -- To view, visit http://gerrit.cloudera.org:8080/16580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b810f5b7ff6a22acc9b10b79ddffa8085c990 Gerrit-Change-Number: 16580 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 19 Oct 2020 22:30:45 +0000 Gerrit-HasComments: Yes
