Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21059 )

Change subject: IMPALA-12426: QueryStateRecord Refactor
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/21059/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21059/5//COMMIT_MSG@7
PS5, Line 7: IMPALA-12426: QueryStateRecord Refactor
> In my mind this patch should also replace the old QueryStateRecord as part
Agreed, it makes more sense as part of this patch.


http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/runtime/coordinator.h@268
PS5, Line 268:   /// Return the backends and their associated resource 
utilization.
> nit: associated
Done


http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/service/client-request-state.h@285
PS5, Line 285:   /// num_rows_fetched_counter_, row_materialization_rate_, and 
row_materialization_timer_
> row_materialization_rate_ repeated, I think you meant the last one to be ro
Done


http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/service/query-state-record.h
File be/src/service/query-state-record.h:

http://gerrit.cloudera.org:8080/#/c/21059/5/be/src/service/query-state-record.h@219
PS5, Line 219:   EventsTimelineIterator begin();
> It's a little funny that this is both a container view and an iterator. But
Agreed, it's a combination of two patterns because there are two container 
object underlying the iterator instead of one.


http://gerrit.cloudera.org:8080/#/c/21059/5/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/21059/5/common/thrift/Query.thrift@995
PS5, Line 995:   15: optional i64 planner_per_host_mem_estimate;
> Can we pull in the Planner.java change that initializes this field?
Done



--
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 <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 05:26:42 +0000
Gerrit-HasComments: Yes

Reply via email to