You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Matthew Jacobs (Code Review)" <ge...@cloudera.org> on 2017/08/15 21:48:14 UTC
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Matthew Jacobs has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7681
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
IMPALA-4833: Compute precise per-host reservation size, pt2
Addresses some comments from the CR:
https://gerrit.cloudera.org/#/c/7630/8
Notably changes the name of initial_reservation_total_bytes
to initial_reservation_total_claims and attempts to clarify
comments.
Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
7 files changed, 36 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/7681/1
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
Patch Set 1: Code-Review+1
This looks good to me, but let's still discuss.
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
Patch Set 1:
> This looks good to me, but let's still discuss.
I made some notes from a back-channel communication.
Potential improvements/changes:
1) Consider renaming min_reservation_bytes -> initial_reservation_bytes?
This isn't obviously the right thing to unless we also change resource_profile_.min_reservation, and I don't think anyone feels strongly about that.
2) look into removing initial_reservation_bytes on the thrift structures. I.e. because it's a basically trivially calculation, compute it in the BE. This is interesting to me because the concept doesn't really make sense outside of the implementation in initial-reservation.cc so it's not ideal to keep in the very visible thrift structures. That said, Tim has pointed out that it does make sense for the FE to compute as much as possible, and for the BE to plumb it through. I'm on the fence, but I'll leave it for now and just try to improve the comments we have.
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
IMPALA-4833: Compute precise per-host reservation size, pt2
Addresses some comments from the CR:
https://gerrit.cloudera.org/#/c/7630/8
Notably changes the name of initial_reservation_total_bytes
to initial_reservation_total_claims and attempts to clarify
comments.
Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Reviewed-on: http://gerrit.cloudera.org:8080/7681
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
7 files changed, 39 insertions(+), 27 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Dan Hecht: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1095/
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Hello Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7681
to look at the new patch set (#2).
Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2
......................................................................
IMPALA-4833: Compute precise per-host reservation size, pt2
Addresses some comments from the CR:
https://gerrit.cloudera.org/#/c/7630/8
Notably changes the name of initial_reservation_total_bytes
to initial_reservation_total_claims and attempts to clarify
comments.
Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M common/thrift/ImpalaInternalService.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
7 files changed, 39 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/7681/2
--
To view, visit http://gerrit.cloudera.org:8080/7681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>