Surya Hebbar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23154 )

Change subject: IMPALA-9846: Enable Aggregated Runtime Profile by Default
......................................................................


Patch Set 22:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23154/22//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23154/22//COMMIT_MSG@65
PS22, Line 65: In order to present this information to the tests, the 'total' 
statistic
             : has been added to the text representation of averaged counters 
along with
             : the existing min/max/avg.
             :
The statistic "total" has not been added just to serve test purposes, it was 
added as it provides additional analytical value

When we used profile V1, the raw counter values were present per instance, with 
profile V2, this entire list of raw values and actual analytical value is 
hidden. So, it's important to have total there.


Also, please see one of the previous comments which describes it like this...

 > So statistic has been used as it also provides additional analytical value. 
 > Consider the following examples,

This comment will provide further info.

Also, this was not something added from patchset 22 or patchset 20, it existed 
from from nearly 5 months ago starting from patchset 3, when I posted this 
change.

Even after this, after multiple discussions with Me, Csaba and Kurt, later on 
call, later having implemented and tested other approaches in between, this was 
finally decided.

There was a DM conversation between us on Slack regarding this matter. I will 
be sure to add you to that conversation.

If you still feel, the counter should not exist, then we can make the test 
infrastructure pull the Verbose profile, but this again would trigger further 
test changes across the framework, where a different profile structure has been 
assumed.


http://gerrit.cloudera.org:8080/#/c/23154/22/common/thrift/RuntimeProfile.thrift
File common/thrift/RuntimeProfile.thrift:

http://gerrit.cloudera.org:8080/#/c/23154/22/common/thrift/RuntimeProfile.thrift@76
PS22, Line 76:   5: required TCounterCategory type
> This is a breaking change and should be avoided. Please keep RuntimeProfile
Sure, I will add it as optional.

But, even with that, would it still cause problems downstream?

If so, could you please tell me, what I could do to avoid/fix this for 
downstream?


http://gerrit.cloudera.org:8080/#/c/23154/22/testdata/impala-profiles/impala_profile_log_tpcds_compute_stats
File testdata/impala-profiles/impala_profile_log_tpcds_compute_stats:

http://gerrit.cloudera.org:8080/#/c/23154/22/testdata/impala-profiles/impala_profile_log_tpcds_compute_stats@1
PS22, Line 1: 1766945502520 6940723a5aa29446:c286e04f00000000 
eJyVVk9PG0cUHwgtFAgEFDu7hShTqQeQAt1dr/8sUg6ObcBKMK4NROptvPvWjLI748zMQqjU9or6AaocqvYrtIdKzaXqoYceIrX9Cj33kENU9ZRUu2sHQ3CV+mB53pv33u+993tvrAfa7Y8jECd4hXp3Co5tFK0cyRNiObZd2HCtUgEM2zf6n9XMhD6bHVtcGjtdrNbu7m/hnd1qDT8otxr1xlaYeuoJ7tMAsCuAKPDw8WF8EhFjlHUxwalhJ6KBh7mP62GPBGQd70vArdr9WrldS5USK45DIDISgB+lrkH4XISEubCua5dAWBr7Ek0hbbIdhSERJ5nxGO7C0vXTuQpnDNwYz74E8Y6MxAmZqXAuPMqI4uL9RLLWFOBSSTlby+cLxoZVNAxjugo+iQKFq51JL/05V4UAumTgDk1VI0EU5Sxj5ux10zFDiVfMnG06Jo7k6lSNeXiPhnDTMqz8mmmtWSVs5TZsc8O21vOWYRgFwzCWt+kRtEEcgbBwU3DFXR7gAxAxoPGDwmJaK5zWua2IgqnNeqPe3q5V5/q6/u19mhw9fJSecX7dWDfW2o1ys729u9dvwkrahQKxXMexfLuQJ6ZjGI6bLxYLpmM5jmtD0TcLhmuVTGN1vgHqmIuHuOx5AqScN61i7Hfd3LCNYskwUmS7vbgUEq9IULhzgl3OfNrtVwgT5uFeQBgDsZrZq+/UPtlt1O6UJSUf3ePBQ6LIh2/hZoTpzGW1mT0TRnJ89950et476cGVdm1vug0yqVG9+oFt27k8FHJOp2SVPMfa8PIOsaEEtmsV3TzpzA4ux9Yz2/WDWrvWOqi1rKvtR0EaNwSmJmPMJAim24oINbr3uVK+ZOYNw5g4o6X+z/UhSOcC/idDhmK9JtxrYg7lPFyjSyh1rloXaJWgvDBLF2bhIkeGpud8iYan7606/r/ZlWwD/ba2kBpWeNijQXJN3/zz219uvfjxK+v3b/6wnjx9flffvrUDinhEkXgrkSDABlakE4DELnEPwVsoMxKcSCqxTxmVh+DdLEfqkAv6aRp6IMYrjPPe6kIzhhHvvIECaXP9LtAQAspA33z228STv/7+7Nnz7z//9aeXX+jb8+kNGXVCqhR4b3qZa/FjickRoUEM79o+E9ClUoFItyRC2tSm4EwB8zIT+rKm1R6DGykutgSPerLCmaQeCPAyKDuO4v145XSxQhQJeBfH5KIuxLPgd/y865dM0zAd2yP5jZJbzNkedOyc6Vie493YFbRLGQnwXoxkQBKJFlrggwDmgpeqJNJLbwpH2V8CJm7l6ESQdm2gwt1Yh80M0pe193Yg5OJkhzzOvJt9+ar/GUtyHjudPADhUVeVylLSLoufG58KqfouOuCSSA6eHiox4wqTSPE16ZKk9Lo2cJHAO4uGtJlmysE683kGpa/Q2OlAGk/h/CD5vkzXhrX9h2w2Hb904jNI/1rTKgEFpjZBuYf3ufvwAaEqppPITGcfI+36kHpY9eKpRNpcIwpj8iTapP0IaTfOCzcFDysx31PtVKtZqfCIqQzKvou02Vaz0gLiDdx+FyLtaqtZeSCogoHw5x+uIC3b4sc7RIGgJOgPSIsoyIwnMd9UDoxR0hykT8UVeDIy20vzvJDfqMxe53Qum/NpjIA/Crhz+1xjBojiXSfT6mfHk++kD0tboOpsM6DdQ9VM/y0N30ZZNETW7Ksz3qLlDELoX8DgQC8=
> Please do not change the testcase thrift profile so we can see the differen
Yes, similar to previous comment, the thrift counter is the reason for updating 
this.



--
To view, visit http://gerrit.cloudera.org:8080/23154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If41d6322361fba82c946efd614cc7d28cb1c36e8
Gerrit-Change-Number: 23154
Gerrit-PatchSet: 22
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[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: Fri, 02 Jan 2026 18:43:58 +0000
Gerrit-HasComments: Yes

Reply via email to