Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15441 )
Change subject: rpc: use a lighter weight completion for sync RPCs ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/client/resource_metrics-internal.h File src/kudu/client/resource_metrics-internal.h: http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/client/resource_metrics-internal.h@28 PS1, Line 28: #include "kudu/client/resource_metrics.h" Isn't this intended for a different patch? http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/rpc/proxy.cc File src/kudu/rpc/proxy.cc: http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/rpc/proxy.cc@98 PS1, Line 98: #if 1 Probably want to remove the ifdefs now? http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/util/notification.h File src/kudu/util/notification.h: http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/util/notification.h@33 PS1, Line 33: // NOTE: this class is modeled after absl::Notification but re-implemented : // to not have dependencies on other absl-specific code. If absl is ever : // imported, this can be removed. I was going to ask about this. http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/util/notification.h@63 PS1, Line 63: // `WaitForNotificationWithTimeout()`. This method doesn't exist. http://gerrit.cloudera.org:8080/#/c/15441/1/src/kudu/util/notification.h@82 PS1, Line 82: DCHECK_EQ(s, NOT_NOTIFIED_HAS_WAITERS); Isn't it possible for this to be violated if two threads race in WaitForNotification (no notification yet), and in the "winner", the CAS results in a write of NOT_NOTIFIED_NO_WAITERS into 's'? -- To view, visit http://gerrit.cloudera.org:8080/15441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b65cce8bd48ee7edf6b2d08e96d00681c32aa97 Gerrit-Change-Number: 15441 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 16 Mar 2020 07:30:17 +0000 Gerrit-HasComments: Yes
