[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Reviewed-on: http://gerrit.cloudera.org:8080/7834
Reviewed-by: Matthew Jacobs 
Tested-by: Impala Public Jenkins
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 57 insertions(+), 35 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1161/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 6: Code-Review+2

Agg memory changed in the test cases after Tim's recent patch for pre-agg 
reservations. Updated the tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Hello Philip Zeyliger, Impala Public Jenkins, Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 57 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/7834/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7834
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1159/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1159/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 5: Code-Review+1

Those messages make sense to me now, but let's see what others say.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Hello Philip Zeyliger,

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

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

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

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 57 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/7834/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7834
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Hello Philip Zeyliger,

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

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

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

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 57 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/7834/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7834
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7834/3/testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
File 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test:

PS3, Line 33: Mem available for buffer reservations
:  (based on mem_limit): 1.00 KB
I thought the mem_limit value would go inside the parentheses. Or did you 
intend the value shown to be the reservation limit?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Hello Philip Zeyliger,

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

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

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

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 57 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 2:

(1 comment)

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

PS2, Line 118: based on mem_limit: "
> Yeah that would be clearer for me.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 2:

(1 comment)

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

PS2, Line 118: based on mem_limit: "
> would it be more clear if I put (based on the mem_limit) in parens?
Yeah that would be clearer for me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 2:

(1 comment)

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

PS2, Line 118: based on mem_limit: "
> I find this wording a bit confusing since it's not clear to me upon first r
would it be more clear if I put (based on the mem_limit) in parens?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 2:

(1 comment)

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

PS2, Line 118: based on mem_limit: "
I find this wording a bit confusing since it's not clear to me upon first read 
whether the number is the mem_limit or the mem available for buffer 
reservations.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7834/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

Line 334: #vector.get_value('exec_option')['num_nodes'] = 1
> What's up here?
Yeah, fixed that in rev2 already. At first I was toying with this, but then 
realized the join strategy change is a moot point if it's always run on 1 node.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change.

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..


Patch Set 1:

(1 comment)

Looks like there are some stray comments?

http://gerrit.cloudera.org:8080/#/c/7834/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

Line 334: #vector.get_value('exec_option')['num_nodes'] = 1
What's up here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new patch set (#2).

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 54 insertions(+), 25 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5838: Improve errors on AC buffer mem rejection

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-5838: Improve errors on AC buffer mem rejection
..

IMPALA-5838: Improve errors on AC buffer mem rejection

The error message returned when a query is rejected due to
insufficient buffer memory is misleading. It recommended a
mem_limit which would be high enough, but changing the
mem_limit may result in changing the plan, which may result
in further changes to the buffer memory requirement.

In particular, this can happen when the planner compares the
expected hash table size to the mem_limit, and decides to
choose a partitioned join over a broadcast join.

While we might consider other code changes to improve this,
for now lets just be clear in the error message.

Testing:
* Adds tests that verify the expected behavior with the new
  error message.

Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
---
M be/src/scheduling/admission-controller.cc
A 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/custom_cluster/test_admission_controller.py
3 files changed, 56 insertions(+), 25 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3dc3517195508d86078a8a4b537ae7d2f52fbcb7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs