Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21059 )
Change subject: IMPALA-12426: QueryStateRecord Refactor ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/21059/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21059/7//COMMIT_MSG@11 PS7, Line 11: Since significant portions of this struct has data : that is also needed in workload management, it has been refactored. Please mentioned any new fields added to QueryStateRecord with this patch. I see at least this field is new: /// User set query options. TQueryOptions query_options; http://gerrit.cloudera.org:8080/#/c/21059/7/be/src/service/query-state-record.h File be/src/service/query-state-record.h: http://gerrit.cloudera.org:8080/#/c/21059/7/be/src/service/query-state-record.h@230 PS7, Line 230: struct QueryStateExpanded { I don't see any use of QueryStateExpanded in this patch. Not even in test file. Is it only used by the next patch? http://gerrit.cloudera.org:8080/#/c/21059/7/be/src/service/query-state-record.h@232 PS7, Line 232: /// Base Query State : const std::shared_ptr<QueryStateRecord> base_state; Are both QueryStateRecord and QueryStateRecordExpanded intended to be owned by ImpalaServer? How will this pointer interact with entries of ImpalaServer::query_log_? How it will behave if an entry is removed from ImpalaServer::query_log_? -- To view, visit http://gerrit.cloudera.org:8080/21059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I57d470db6fea71ec12e43f86e3fd62ed6259c83a Gerrit-Change-Number: 21059 Gerrit-PatchSet: 7 Gerrit-Owner: Jason Fehr <[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, 27 Feb 2024 19:47:10 +0000 Gerrit-HasComments: Yes
