Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21142 )
Change subject: IMPALA-12737: Query columns in workload management tables. ...................................................................... Patch Set 44: (5 comments) http://gerrit.cloudera.org:8080/#/c/21142/41/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21142/41/be/src/service/impala-server.h@1661 PS41, Line 1661: boost::scoped_ptr<ThriftServer> hs2_http_server_; > It's still there in the latest patchset. It has been removed for real this time. http://gerrit.cloudera.org:8080/#/c/21142/40/be/src/service/workload-management-init.cc File be/src/service/workload-management-init.cc: http://gerrit.cloudera.org:8080/#/c/21142/40/be/src/service/workload-management-init.cc@354 PS40, Line 354: // concurrently on all coordinators. Running the same create and alter table statements > This is something we'll want to think more about in how we deploy Impala. All initialization of the workload management 'sys' db and live/log tables is now done in the catalog. http://gerrit.cloudera.org:8080/#/c/21142/49/be/src/service/workload-management-init.cc File be/src/service/workload-management-init.cc: http://gerrit.cloudera.org:8080/#/c/21142/49/be/src/service/workload-management-init.cc@105 PS49, Line 105: static void _setupTable(InternalServer* internal_server, > Was there a specific reason to rename this variable? I'm ok with modifying variable names now since workload management is not a well established feature, but after this change, I don't want there to be any more variable name modifications for precisely the reason of git history readability. http://gerrit.cloudera.org:8080/#/c/21142/41/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/21142/41/be/src/service/workload-management.cc@65 PS41, Line 65: static list<CompletedQuery> _completed_queries; > There's a difference between static functions (which have no side effects) This use of the "static" keyword when declaring variables came out of my misunderstanding what that meant when they keyword was applied to variables. My intention was to limit the scope of the variables to just this file. I removed the "static" keyword on the variables. http://gerrit.cloudera.org:8080/#/c/21142/44/be/src/util/string-join.h File be/src/util/string-join.h: http://gerrit.cloudera.org:8080/#/c/21142/44/be/src/util/string-join.h@39 PS44, Line 39: string JoinToString(const Container<Item>& container, const string delimiter, > This could probably just go in string-util.h. I completely removed this file since the JoinToString method was only used in 1 place to generate an error message in a very unlikely situation. -- 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: 44 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: Tue, 08 Oct 2024 22:51:49 +0000 Gerrit-HasComments: Yes
