[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort
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
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
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
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
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
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
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
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
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: (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: initialize > nit: typo Done 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 I think this case is already covered by various tests is test_spilling.py -- 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: Thu, 12 Apr 2018 19:34:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6713: Fix request for unneeded memory in partial sort
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
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