impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3902: Scheduler improvements for running multiple fragment instances on a single backend
Date Mon, 05 Sep 2016 19:09:17 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-3902: Scheduler improvements for running multiple fragment instances
on a single backend

Patch Set 3:

File be/src/runtime/

PS3, Line 466: request.query_ctx.request.query_options.mt_dop != 1
parentheses? Sometimes we have them in these cases.
File be/src/runtime/coordinator.h:

Line 562:   const TPlanFragment* GetCoordFragment(const TQueryExecRequest& request) const;

PS3, Line 565: GetTPlanFragments
Could this be static? Implementation doesn't seem to access instance members.

Line 569:   int GetNumRemoteInstances(const QuerySchedule& schedule) const;
File be/src/scheduling/

PS3, Line 321: auto
use auto only for iterators
File be/src/scheduling/query-schedule.h:

PS3, Line 73: sender_id
Initialize this?

PS3, Line 281:   void InitMtFragmentExecParams();
File be/src/scheduling/

PS3, Line 384: auto
auto only for iterators

PS3, Line 403: backend_config
nit: some of these parameters could go to the previous line, maybe saving a line.

PS3, Line 429: i + 1
Can you add a comment to explain the "+ 1"?

Line 434:              || sink.output_partition.type == TPartitionType::HASH_PARTITIONED
nit: formatting

Line 448:         src_params->destinations.emplace_back();
You could resize src_params->destinations and then call setters on the elements. The code
would be more compact and might be easier to read.

PS3, Line 478: MakeNetworkAddress
Replace with local_backend_descriptor_.address?

Line 509: /// - 
Incomplete comment?

PS3, Line 522:  
nit: extra space

Line 530:     // average number of bytes to each instance

PS3, Line 534: num_instances
can this be 0?

PS3, Line 535: int64
we often use int64_t for those types. Is there a difference?

PS3, Line 543: static_cast<int64>(avg_bytes_per_instance * (i + 1))
Maybe use a variable for this to make it more readable?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I240445199e22f009f4e72fdb8754eb8d77e3d680
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-HasComments: Yes

View raw message