[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..

IMPALA-6713: Fix request for unneeded memory in partial sort

When a Sorter::Run is initialized, if it is an initial run and has
varlen data, it requests an extra buffer to have space to sort the
varlen data should it need to spill to disk.

This extra buffer is not needed in the case of partial sorts, which
do not spill, and because this extra buffer was not included in the
calculation of the minimum required reservation, requesting it caused
the partial sort to fail in cases where the partial sort only had its
minimum reservation available to use.

The solution is to not request the extra memory for partial sorts.

Testing:
- Added a test to test_sort.py that ensures the partial sort can
  complete successfully even if additional memory requests beyond its
  minimum reservation are denied.

Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Reviewed-on: http://gerrit.cloudera.org:8080/10031
Reviewed-by: Thomas Tauber-Marshall 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/sorter.cc
M tests/query_test/test_sort.py
2 files changed, 19 insertions(+), 2 deletions(-)

Approvals:
  Thomas Tauber-Marshall: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 21:20:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 3: Code-Review+2

carrying forward


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:32:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:32:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10031/2/tests/query_test/test_sort.py
File tests/query_test/test_sort.py:

http://gerrit.cloudera.org:8080/#/c/10031/2/tests/query_test/test_sort.py@209
PS2, Line 209:   """Test class to do functional validation of partial sorts."""
> I forget what the default test matrix is - does this only run with a single
Tests that don't take 'vector' as a parameter are just run a single time 
without any dimensions



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:27:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 2: Code-Review+2

(1 comment)

LGTM so long as we're not running the test with unnecessary dimensions.

http://gerrit.cloudera.org:8080/#/c/10031/2/tests/query_test/test_sort.py
File tests/query_test/test_sort.py:

http://gerrit.cloudera.org:8080/#/c/10031/2/tests/query_test/test_sort.py@209
PS2, Line 209:   """Test class to do functional validation of partial sorts."""
I forget what the default test matrix is - does this only run with a single 
table format?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 16:02:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-13 Thread Gabor Kaszab (Code Review) 
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 13 Apr 2018 06:54:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-12 Thread Thomas Tauber-Marshall (Code Review) 
Hello Gabor Kaszab, Tim Armstrong,

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

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

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

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..

IMPALA-6713: Fix request for unneeded memory in partial sort

When a Sorter::Run is initialized, if it is an initial run and has
varlen data, it requests an extra buffer to have space to sort the
varlen data should it need to spill to disk.

This extra buffer is not needed in the case of partial sorts, which
do not spill, and because this extra buffer was not included in the
calculation of the minimum required reservation, requesting it caused
the partial sort to fail in cases where the partial sort only had its
minimum reservation available to use.

The solution is to not request the extra memory for partial sorts.

Testing:
- Added a test to test_sort.py that ensures the partial sort can
  complete successfully even if additional memory requests beyond its
  minimum reservation are denied.

Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
---
M be/src/runtime/sorter.cc
M tests/query_test/test_sort.py
2 files changed, 19 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-12 Thread Gabor Kaszab (Code Review) 
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10031 )

Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10031/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10031/1//COMMIT_MSG@9
PS1, Line 9: initialzed
nit: typo


http://gerrit.cloudera.org:8080/#/c/10031/1/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:

http://gerrit.cloudera.org:8080/#/c/10031/1/be/src/runtime/sorter.cc@634
PS1, Line 634:   + (has_var_len_slots_ && initial_run_ && 
sorter_->enable_spilling_);
Should we also test that a regular Sort node with spilling turned off works 
well with the lowest memory allocation that was needed before this change? 
(e.g. picking any sorting query checking what the lowest memory limit it worked 
with spilling turned off, check that it still works with that limit)
There might be an existing test for this, though.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 12 Apr 2018 18:25:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort

2018-04-11 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10031


Change subject: IMPALA-6713: Fix request for unneeded memory in partial sort
..

IMPALA-6713: Fix request for unneeded memory in partial sort

When a Sorter::Run is initialzed, if it is an initial run and has
varlen data, it requests an extra buffer to have space to sort the
varlen data should it need to spill to disk.

This extra buffer is not needed in the case of partial sorts, which
do not spill, and because this extra buffer was not included in the
calculation of the minimum required reservation, requesting it caused
the partial sort to fail in cases where the partial sort only had its
minimum reservation available to use.

The solution is to not request the extra memory for partial sorts.

Testing:
- Added a test to test_sort.py that ensures the partial sort can
  complete successfully even if additional memory requests beyond its
  minimum reservation are denied.

Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
---
M be/src/runtime/sorter.cc
M tests/query_test/test_sort.py
2 files changed, 19 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d9c0863009021340d8b684669b371a2cfb1ecad
Gerrit-Change-Number: 10031
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall