Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10705 )
Change subject: IMPALA-6816: minimise calls to GetMinSubscriberTopicVersion() ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10705/3/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/10705/3/be/src/scheduling/admission-controller.cc@245 PS3, Line 245: StatestoreSubscriber::UpdateCallback cb = [this]( Nit: auto should be used as the type for lambda. Declaring it as std::function would invoke std::function's copy-constructor once. (Fun fact: It's not move-constructed). http://gerrit.cloudera.org:8080/#/c/10705/3/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/10705/3/be/src/statestore/statestore.cc@735 PS3, Line 735: if (deltas_needing_min_version.size() > 0) { nit: !deltas_needing_min_version.empty() http://gerrit.cloudera.org:8080/#/c/10705/3/be/src/statestore/statestore.cc@737 PS3, Line 737: typedef map<TopicId, TTopicDelta> TopicDeltaMap; No longer used http://gerrit.cloudera.org:8080/#/c/10705/3/common/thrift/StatestoreService.thrift File common/thrift/StatestoreService.thrift: http://gerrit.cloudera.org:8080/#/c/10705/3/common/thrift/StatestoreService.thrift@147 PS3, Line 147: 3: optional bool populate_min_subscriber_topic_version = false; Why optional? It's treated as required throughout this patch. http://gerrit.cloudera.org:8080/#/c/10705/3/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/10705/3/tests/statestore/test_statestore.py@614 PS3, Line 614: update_counts index into last_to_versions http://gerrit.cloudera.org:8080/#/c/10705/3/tests/statestore/test_statestore.py@615 PS3, Line 615: update_count unused -- To view, visit http://gerrit.cloudera.org:8080/10705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ee7cb2355ba1049b9081e0df344ac41aa4ebeb1 Gerrit-Change-Number: 10705 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 23 Jun 2018 02:03:50 +0000 Gerrit-HasComments: Yes
