Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10289 )
Change subject: IMPALA-6948: Delete catalog update topic entries upon catalog restart ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@213 PS1, Line 213: topic_update_log_.clear(); > Good point. I updated the metrics and the related fields. I don't think res makes sense to not update. pls add a comment, perhaps in the header, to state that this bit of history is intentionally retained. http://gerrit.cloudera.org:8080/#/c/10289/1/be/src/statestore/statestore.cc@665 PS1, Line 665: subscriber->SetLastTopicVersionProcessed(topic_it->first, update.from_version); > Hm, not following. What do you suggest we dcheck? basically asking if from_version can be set here and would that worth checking? http://gerrit.cloudera.org:8080/#/c/10289/2/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/10289/2/be/src/statestore/statestore.cc@216 PS2, Line 216: key_size_metric_->SetValue(0); : topic_size_metric_->SetValue(0); : value_size_metric_->SetValue(0); these are shared across topics, so this should more accurately subtract what was cleared. since we keep track of the total per topic, perhaps subtract that off? -- To view, visit http://gerrit.cloudera.org:8080/10289 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a8ade8e498ac35cb56d3775d2c67a86988d9b6 Gerrit-Change-Number: 10289 Gerrit-PatchSet: 2 Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 04 May 2018 23:07:41 +0000 Gerrit-HasComments: Yes