Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12049 )
Change subject: IMPALA-4555: Make QueryState's status reporting more robust ...................................................................... Patch Set 5: (9 comments) http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/coordinator-backend-state.cc@344 PS4, Line 344: for (const auto& stateful_report : instance_exec_status.stateful_report()) { > DCHECK_LE(stateful_report.report_seq_no(), report_seq_no()); Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.h File be/src/runtime/fragment-instance-state.h: http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.h@170 PS4, Line 170: :vector< > typo Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.h@171 PS4, Line 171: > nit: prev_stateful_reports_; Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.cc File be/src/runtime/fragment-instance-state.cc: http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.cc@253 PS4, Line 253: DCHECK(!final_report_sent_ > Please see comments below about some potential simplification we can do. Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.cc@279 PS4, Line 279: {prev_stateful_reports_.begin(), prev_stateful_reports_.end()}; > nit: indent 4 Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/fragment-instance-state.cc@298 PS4, Line 298: if (num_reports > 0 && prev_statef > As discussed in person, this could have been simpler by not storing the fin Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/query-state.h@410 PS4, Line 410: report' > stale ? Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/query-state.h@411 PS4, Line 411: profiles_forest'. > stale ? Done http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/12049/4/be/src/runtime/query-state.cc@563 PS4, Line 563: portExecStatus(); > Given this is quite clunky to write and used at more than one places, may b Done -- To view, visit http://gerrit.cloudera.org:8080/12049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6007013fc2c9e8eeba11b752ee58fb3038da971 Gerrit-Change-Number: 12049 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Fri, 01 Feb 2019 00:30:31 +0000 Gerrit-HasComments: Yes
