Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21351 )

Change subject: IMPALA-13012: Lower default query_log_max_queued
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21351/2/be/src/service/workload-management-flags.cc
File be/src/service/workload-management-flags.cc:

http://gerrit.cloudera.org:8080/#/c/21351/2/be/src/service/workload-management-flags.cc@82
PS2, Line 82: DEFINE_int32(query_log_max_queued, 5000
Put a comment to update the default value if TQueryTableColumn change.


http://gerrit.cloudera.org:8080/#/c/21351/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21351/2/be/src/service/workload-management.cc@428
PS2, Line 428: TQueryTableColumn::TABLES_QUERIED + 1
Can you make function or macro for this expression?
The point is to remind people to update the expression if num column change, 
much like what we have for query option:
https://github.com/apache/impala/blob/f620e5d/be/src/service/query-options.h#L55

Is this the only major change in this patch? Others looks like just indentation 
change, new timers, and logging.


http://gerrit.cloudera.org:8080/#/c/21351/2/be/src/service/workload-management.cc@441
PS2, Line 441: "gather_time=" << PrettyPrinter::Print(gather_time, 
TUnit::TIME_NS) << " "
             :           "exec_time=" << PrettyPrinter::Print(exec_time, 
TUnit::TIME_NS);
Might want to put them as Metric instead.



--
To view, visit http://gerrit.cloudera.org:8080/21351
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7
Gerrit-Change-Number: 21351
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[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, 24 Apr 2024 23:22:06 +0000
Gerrit-HasComments: Yes

Reply via email to