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 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23950/4/be/src/runtime/backend-hardware-info.h
File be/src/runtime/backend-hardware-info.h:

http://gerrit.cloudera.org:8080/#/c/23950/4/be/src/runtime/backend-hardware-info.h@29
PS4, Line 29:
> Since we include OS settings, can we change this to BackendMachineInfo?
Done


http://gerrit.cloudera.org:8080/#/c/23950/4/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/23950/4/be/src/runtime/coordinator.cc@1442
PS4, Line 1442:   // Add aggregated machine information
              :   if (machine_info_aggregator.HasData()) {
              :     string cpu_summary = 
machine_info_aggregator.GetCpuSummary();
              :     if (!cpu_summary.empty()) {
              :       query_profile_->AddInfoString("Backend CPU Info", 
cpu_summary);
              :     }
              :
              :     string os_summary = machine_info_aggregator.GetOsSummary();
              :     if (!os_summary.empty()) {
              :       query_profile_->AddInfoString("Backend OS Info", 
os_summary);
              :     }
              :   }
> This is ok for now. At some point when we have more fields, we may want the
True and since we have only a few fields right now, Im leaving this unchanged.


http://gerrit.cloudera.org:8080/#/c/23950/4/common/protobuf/statestore_service.proto
File common/protobuf/statestore_service.proto:

http://gerrit.cloudera.org:8080/#/c/23950/4/common/protobuf/statestore_service.proto@94
PS4, Line 94:   // local time zone.
            :   optional string process_start_time = 13;
            :
            :   // The pretty-printed string representation of program version 
and build version for
            :   // this backend.
            :   optional string version = 14;
            :
            :   // Unique identifier of an executor w
> One small thing: Let's go ahead and turn this into its own struct / message
Thanks for the tip, I added a new message `MachineInfoPB` and used that in 
`BackendDescriptorPB`.



--
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: 6
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: Fri, 13 Mar 2026 03:18:27 +0000
Gerrit-HasComments: Yes

Reply via email to