Riza Suminto 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.
             :
Please don't add new "total" stats just to serve test.

Change the test instead by validating different part of profile, or make the 
test infrastructure to pull the verbose profile where all individual values are 
printed.

It does not make sense to have "total" for some counter such as *Rate or *Ratio.


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
> I will consider changing this to optional.
This is a breaking change and should be avoided. Please keep 
RuntimeProfile.thrift remain the same, because there are many downstream code 
that relies on this.


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 difference 
before and after patch.
I assume you change this because you modify the underlying thirft definition of 
TAggCounter?



--
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 16:26:04 +0000
Gerrit-HasComments: Yes

Reply via email to