Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21142 )
Change subject: IMPALA-12737: Query columns in workload management tables. ...................................................................... Patch Set 52: (5 comments) http://gerrit.cloudera.org:8080/#/c/21142/52//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21142/52//COMMIT_MSG@16 PS52, Line 16: Since new columns are being added, the workload management init Also mention that the init process has been moved to occur on the primary catalogd instance, and any other relevant changes. http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/catalog/workload-management-init.cc File be/src/catalog/workload-management-init.cc: http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/catalog/workload-management-init.cc@427 PS52, Line 427: if (!ParseVersion(iter.second, &*schema_version_new).ok()) { If we find a new schema that parses, can we break early? http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/catalog/workload-management-init.cc@525 PS52, Line 525: // Create and/or update the query log table if needed. Should be "query live table". http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/service/workload-management-worker.h File be/src/service/workload-management-worker.h: http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/service/workload-management-worker.h@103 PS52, Line 103: const std::array<FieldParser, NumQueryTableColumns> FIELD_PARSERS = {{ I'm not sure why you moved defining these all back to a header file. This array will be duplicated across multiple objects (although maybe deduplicated during linking?). http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/service/workload-management-worker.cc File be/src/service/workload-management-worker.cc: http://gerrit.cloudera.org:8080/#/c/21142/52/be/src/service/workload-management-worker.cc@269 PS52, Line 269: ABORT_IF_ERROR(workloadmgmt::StartupChecks(&target_schema_version)); Should this wait and retry a bit to see if catalogd updates the schema version to the one we expect? Or is that not really an issue later? Is there somewhere we wait for the table to exist? -- To view, visit http://gerrit.cloudera.org:8080/21142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78f3670b067c0c192ee8a212fba95466fbcb51d7 Gerrit-Change-Number: 21142 Gerrit-PatchSet: 52 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 16 Oct 2024 21:48:54 +0000 Gerrit-HasComments: Yes
