Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 )
Change subject: KUDU-2797: the master aggregates tablet metrics ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/13426/6/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/13426/6/src/kudu/tserver/ts_tablet_manager.cc@1568 PS6, Line 1568: unique_ptr<ReportedTabletStatsPB> pb(new ReportedTabletStatsPB()) > /mnt/ddb/2/helif/apache/kudu/src/kudu/tserver/ts_tablet_manager.h:403:12: Hrm, IWYU unfortunately runs differently in different environments. Perhaps the build machines will be OK with it including master.pb.h in the header. And at worst, we can use "IWYU pragma: keep" to have it ignore the suggestion. http://gerrit.cloudera.org:8080/#/c/13426/6/src/kudu/tserver/ts_tablet_manager.cc@1600 PS6, Line 1600: for (const auto& tablet_id : dirty_tablets) { : MarkTabletDirty(tablet_id, "The tablet statistics have been changed"); : } > I think 'in batch' would be better than 'one by one'. What do you think? Ah yeah looking at the definition of MarkTabletDirty, in batch would allow us to do all of these under a single hold of the heartbeater thread lock. SGTM -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 6 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Wed, 12 Jun 2019 03:52:08 +0000 Gerrit-HasComments: Yes
