Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21683 )
Change subject: IMPALA-13304: Include aggregate instance-level metrics within experimental profile(V2) ...................................................................... Patch Set 8: Thank you for your detail explanation in https://gerrit.cloudera.org/c/21683/7..8#message-3eb9de84_6ee613aa I agree that these are corner cases that difficult to handle. I propose to go with 2nd options instead (skip instances with missing events) with follow up plan to improve RuntimeProfile::EventSequence(). We can enhance RuntimeProfile::EventSequence() constructor to deterministically specify what is the expected number of events in a sequence; and supply the expected event index when registering event through RuntimeProfile::EventSequence::MarkEvent(). Or, pass a fixed list of labels during RuntimeProfile::EventSequence() construction. The declaration might be easy with some macro helper. I believe the thrift code does not need to change to accommodate this determinism improvement. If you agree with this, please file a follow up JIRA with your example as explanation detail. -- To view, visit http://gerrit.cloudera.org:8080/21683 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49e18a7a7e1288e3e674e15b6fc86aad60a08214 Gerrit-Change-Number: 21683 Gerrit-PatchSet: 8 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Mon, 18 Nov 2024 17:39:22 +0000 Gerrit-HasComments: No
