Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23950 )
Change subject: IMPALA-12191: Add hardware and OS details to runtime profile ...................................................................... Patch Set 3: (11 comments) Thanks for the comments. http://gerrit.cloudera.org:8080/#/c/23950/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23950/1//COMMIT_MSG@12 PS1, Line 12: 1. Per-node profiles > Add the example output in the commit message. Done http://gerrit.cloudera.org:8080/#/c/23950/1//COMMIT_MSG@14 PS1, Line 14: 2. Aggregated summary: Unique configurations are grouped with counts > The minicluster has 3 executors running, so a plan that runs across the 3 e Done http://gerrit.cloudera.org:8080/#/c/23950/1//COMMIT_MSG@20 PS1, Line 20: ccycloud.impala-host.root.comops.site:27000: > You need to update the profiles in testdata/impala-profiles for the impala- Yes the fields are optional and backward compatible. We dont need any changes here. http://gerrit.cloudera.org:8080/#/c/23950/1//COMMIT_MSG@22 PS1, Line 22: CPU Info: Intel(R) Xeon(R) Silver 4215R CPU @ 3.20GHz (32 cores) : - HostCpuIoWaitPercentage (50.000ms): 0, 0, 0, 0 : - HostCpuSysPercentage (50.000ms): 3, 4, 4, 4 : ... : ccycloud.impala-host.root.comops.site:27001: : OS Info: Ubuntu 22.04.5 LTS > I agree with Surya that we don't need to list the future work in detail. Done http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/runtime/coordinator.cc@1399 PS1, Line 1399: BackendHardwareInfo::FromBackendExecParams(backend_state->exec_params()); > Ah, I see the aggregation here. Done http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/runtime/coordinator.cc@1447 PS1, Line 1447: } > Let's keep it separate. It isn't really related to what this JIRA is about. Done http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/runtime/coordinator.cc@1453 PS1, Line 1453: } > I'm ok with adding OS / CPU to the per node profile. For future host inform Done. Added os and cpu info to the per node profile, and yeah its better to keep the per-node profile lean. http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/23950/1/be/src/service/impala-server.cc@2678 PS1, Line 2678: be_desc->set_process_start_time(CurrentTimeString()); : } : be_desc->set_version(GetBuildVersion(/* compact */ true) > Grouping these fields into their own struct would help keep the code clean, Thanks for the thorough explanation, Ive introduced a new struct BackendHardwareInfo and an aggregator class BackendHardwareInfoAggregator. Now new fields can be added cleanly. Ive also added unit tests for the newly added struct and the aggregator class. http://gerrit.cloudera.org:8080/#/c/23950/1/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/23950/1/tests/query_test/test_observability.py@1155 PS1, Line 1155: \ > You should print the entire profile in the error message as well. Done http://gerrit.cloudera.org:8080/#/c/23950/1/tests/query_test/test_observability.py@1168 PS1, Line 1168: > Same here. Done http://gerrit.cloudera.org:8080/#/c/23950/1/tests/query_test/test_observability.py@1171 PS1, Line 1171: assert os_info_match is not None, \ : "Could not extract Backend OS I > Write example of where multiple OS and multiple types of CPU info are prese I wrote examples for it. With my current knowledge, Im not sure how do I test this in such an env. I believe since its working for my current dev env, and the code changes are also made at the right places thanks to Joe's guidance, it should work on heterogenous env too. -- To view, visit http://gerrit.cloudera.org:8080/23950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5a093b6cf9dd7dfa4321b79aeea95d6fc748dd4 Gerrit-Change-Number: 23950 Gerrit-PatchSet: 3 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Sat, 28 Feb 2026 05:24:12 +0000 Gerrit-HasComments: Yes
