[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 HechtTested-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 Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size, pt2
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 JacobsGerrit-Reviewer: Dan Hecht Gerrit-HasComments: No
[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