Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8758 )
Change subject: IMPALA-6190/6246: Add instances tab and event sequence ...................................................................... Patch Set 13: (5 comments) Thanks for the review. Please see my inline comments and PS13. http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/common/atomic.h File be/src/common/atomic.h: http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/common/atomic.h@148 PS12, Line 148: > Can we add a static assert that the enum type is <= 32 bits. It seems like Done http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc File be/src/runtime/fragment-instance-state.cc: http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc@133 PS12, Line 133: profile()->AddEventSequence("Fragment Instance Lifecycle Event Timeline"); > We should be careful to document what the fragment instance timings are rel Done http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/runtime/fragment-instance-state.cc@515 PS12, Line 515: PRODUCING_DAT > Should we call this LAST_BATCH_SENT instead for consistency? Done http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/8758/12/be/src/util/runtime-profile-counters.h@357 PS12, Line 357: return event1.second < event2.second; > So this comment is incorrect about it being ordered? Yes, I updated it. http://gerrit.cloudera.org:8080/#/c/8758/12/www/query_finstances.tmpl File www/query_finstances.tmpl: http://gerrit.cloudera.org:8080/#/c/8758/12/www/query_finstances.tmpl@113 PS12, Line 113: clearInterval(intervalId); > The query could also not have started executing yet. I changed the error message, as well as the behavior to display the error message when the query has finished. I also noticed that query_backends freezes the last state of the table (by throwing an unhandled error in the JS code :o ), which I think leaves the user wondering why it stopped updating. Since the query will immediately be removed upon completion we won't be able to pull a fully completed table from the server, so I think it's cleaner to remove it upon completion. Let me know if you'd prefer to preserve the last state. -- To view, visit http://gerrit.cloudera.org:8080/8758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I626456b6afa9101eeeeffd5cda10c4096d63d7f9 Gerrit-Change-Number: 8758 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 22 Jan 2018 22:55:41 +0000 Gerrit-HasComments: Yes
