You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2020/06/29 22:04:02 UTC

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState 

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16122


Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................

IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

This is the final patch in the refactor of QuerySchedule for the
single admission controller work. It was kept separate for ease of
reviewing.

This patch renames QuerySchedule and its related classes
(FInstanceExecParams, BackendExecParams, and FragmentExecParams) to
use the name 'ScheduleState', reflecting the fact that they are used
as containers for intermediate info about a query during scheduling.

The messages that are included in the QuerySchedulePB struct retain
the 'ExecParams' name, reflecting the fact that they are used by the
coordinator to start execution.

Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
---
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
R be/src/scheduling/schedule-state.cc
R be/src/scheduling/schedule-state.h
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
9 files changed, 564 insertions(+), 564 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/16122/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jul 2020 23:01:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jul 2020 04:07:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................

IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

This is the final patch in the refactor of QuerySchedule for the
single admission controller work. It was kept separate for ease of
reviewing.

This patch renames QuerySchedule and its related classes
(FInstanceExecParams, BackendExecParams, and FragmentExecParams) to
use the name 'ScheduleState', reflecting the fact that they are used
as containers for intermediate info about a query during scheduling.

The messages that are included in the QuerySchedulePB struct retain
the 'ExecParams' name, reflecting the fact that they are used by the
coordinator to start execution.

Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Reviewed-on: http://gerrit.cloudera.org:8080/16122
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
R be/src/scheduling/schedule-state.cc
R be/src/scheduling/schedule-state.h
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
9 files changed, 631 insertions(+), 625 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/6468/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 18:22:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16122

to look at the new patch set (#3).

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................

IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

This is the final patch in the refactor of QuerySchedule for the
single admission controller work. It was kept separate for ease of
reviewing.

This patch renames QuerySchedule and its related classes
(FInstanceExecParams, BackendExecParams, and FragmentExecParams) to
use the name 'ScheduleState', reflecting the fact that they are used
as containers for intermediate info about a query during scheduling.

The messages that are included in the QuerySchedulePB struct retain
the 'ExecParams' name, reflecting the fact that they are used by the
coordinator to start execution.

Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
---
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
R be/src/scheduling/schedule-state.cc
R be/src/scheduling/schedule-state.h
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
9 files changed, 631 insertions(+), 625 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/16122/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jul 2020 02:26:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 2:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller-test.cc@131
PS1, Line 131:       backend_schedule_state.exec_params->set_min_mem_reservation_bytes(
> line too long (99 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h@246
PS1, Line 246: /// afterwards call CanAdmitRequest() for each of the schedules. Executor groups are
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h@993
PS1, Line 993:   /// Backend is an Executor or a Coordinator.
> line too long (92 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.cc@675
PS1, Line 675:       largest_min_mem_reservation =
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h
File be/src/scheduling/schedule-state.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@61
PS1, Line 61:   explicit BackendScheduleState(BackendExecParamsPB* exec_params)
> line too long (95 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@65
PS1, Line 65: /// Map from an impalad backend address to the exec params for that backend.
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@83
PS1, Line 83:   /// Contains any info that needs to be sent back to the coordinator. Computed during
> line too long (94 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@134
PS1, Line 134: ///   pointers to corresponding FragmentExecParamsPBs created in the ExecParamsPB.
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@210
PS1, Line 210:     return per_backend_schedule_states_;
> line too long (97 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc
File be/src/scheduling/schedule-state.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc@163
PS1, Line 163:         count_map.insert(
> line too long (98 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc@204
PS1, Line 204:     const BackendScheduleState& bp = elem.second;
> line too long (93 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h@355
PS1, Line 355:   /// Computes execution parameters for all backends assigned in the query and always one
> line too long (93 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h@413
PS1, Line 413:       const std::vector<TPlanNodeId>& scan_ids,
> line too long (93 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@224
PS1, Line 224:       FragmentScheduleState* src_params =
> line too long (92 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@828
PS1, Line 828:   coord_min_reservation = coord_be_state.exec_params->min_mem_reservation_bytes();
> line too long (92 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@842
PS1, Line 842:   }
> line too long (96 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@853
PS1, Line 853:                            << ") ";
> line too long (98 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@857
PS1, Line 857:                                      TUnit::UNIT)
> line too long (94 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 17:02:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jul 2020 17:16:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/6458/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 22:33:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/6466/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 17:31:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6104/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jul 2020 23:01:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 4: Code-Review+2

carrying forward


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jul 2020 23:00:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16122

to look at the new patch set (#2).

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................

IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

This is the final patch in the refactor of QuerySchedule for the
single admission controller work. It was kept separate for ease of
reviewing.

This patch renames QuerySchedule and its related classes
(FInstanceExecParams, BackendExecParams, and FragmentExecParams) to
use the name 'ScheduleState', reflecting the fact that they are used
as containers for intermediate info about a query during scheduling.

The messages that are included in the QuerySchedulePB struct retain
the 'ExecParams' name, reflecting the fact that they are used by the
coordinator to start execution.

Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
---
M .clang-format
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
R be/src/scheduling/schedule-state.cc
R be/src/scheduling/schedule-state.h
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
10 files changed, 616 insertions(+), 611 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/16122/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 1:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller-test.cc@131
PS1, Line 131:       backend_schedule_state.exec_params->set_min_mem_reservation_bytes(min_mem_reservation_bytes);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h@246
PS1, Line 246: /// FindGroupToAdmitOrReject(). From there we call ComputeGroupScheduleStates() which calls
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.h@993
PS1, Line 993:   /// Get the amount of memory to admit for the Backend with the given BackendScheduleState.
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/admission-controller.cc@675
PS1, Line 675:     if (bp.exec_params->min_mem_reservation_bytes() > largest_min_mem_reservation.second) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h
File be/src/scheduling/schedule-state.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@61
PS1, Line 61:   explicit BackendScheduleState(BackendExecParamsPB* exec_params) : exec_params(exec_params) {}
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@65
PS1, Line 65: typedef std::unordered_map<NetworkAddressPB, BackendScheduleState> PerBackendScheduleState;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@83
PS1, Line 83:   /// BackendScheduleStatePB in Scheduler::ComputeBackendScheduleState(), after which it is no
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@134
PS1, Line 134: ///   finstance and assigned to hosts. The FInstanceScheduleState each have a corresponding
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.h@210
PS1, Line 210:   PerBackendScheduleState& per_backend_schedule_states() { return per_backend_schedule_states_; }
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc
File be/src/scheduling/schedule-state.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc@163
PS1, Line 163:         count_map.insert(make_pair(entry.second.exec_params->address(), map<TPlanNodeId, int>()));
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/schedule-state.cc@204
PS1, Line 204:     DCHECK(!bp.exec_params->instance_params().empty() || bp.exec_params->is_coord_backend());
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h@355
PS1, Line 355:   /// Computes BackendScheduleState for all backends assigned in the query and always one for
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.h@413
PS1, Line 413:       const std::vector<TPlanNodeId>& scan_ids, const FragmentScheduleState& fragment_params,
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@224
PS1, Line 224:       FragmentScheduleState* src_params = state->GetFragmentScheduleState(src_fragment.idx);
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@828
PS1, Line 828:   BackendScheduleState& coord_be_state = state->GetOrCreateBackendScheduleState(coord_addr);
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@842
PS1, Line 842:           max(largest_min_reservation, backend.second.exec_params->min_mem_reservation_bytes());
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@853
PS1, Line 853:                                   e.second.exec_params->min_mem_reservation_bytes(), TUnit::BYTES)
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/16122/1/be/src/scheduling/scheduler.cc@857
PS1, Line 857:                                   e.second.exec_params->instance_params().size(), TUnit::UNIT)
line too long (94 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jun 2020 22:04:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16122 )

Change subject: IMPALA-9692 (part 4): Rename QuerySchedule to ScheduleState
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6086/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57069c4a426f3e697df7e2a07754d063bdea26f7
Gerrit-Change-Number: 16122
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Jul 2020 21:27:46 +0000
Gerrit-HasComments: No