You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sahil Takiar (Code Review)" <ge...@cloudera.org> on 2019/07/17 01:39:00 UTC

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations 

Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13873


Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Since no new functionality has been added no tests were added
* Running core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
12 files changed, 366 insertions(+), 154 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 8
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 19:20:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 8
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jul 2019 01:34:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 19:18:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Added tests/query_test/test_result_spooling.py; currently only runs a
simple select limit 10 with SPOOL_QUERY_RESULTS = true and validates
that 0 rows are returned
* Ran core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Reviewed-on: http://gerrit.cloudera.org:8080/13873
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A tests/query_test/test_result_spooling.py
13 files changed, 408 insertions(+), 154 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3903/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 19:47:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3902/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 19:40:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3910/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 18:14:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 6: Code-Review+1

LGTM. I can upgrade to +2 but will wait to see if Michael also wants to take a look.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 20:22:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 3:

(6 comments)

Mostly looks good, just some comments about the feature flag and some code cleanup we should do to match newer best practices in the codebase

http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG@24
PS3, Line 24: * Since no new functionality has been added no tests were added
Can you add a test that exercises the option to confirm that it doesn't crash when you run a query? We don't want user-toggleable options that can cause instability.

I actually like this approach of doing the development behind a feature flag; just gotta make sure that we keep the code in a state where we could cut a release from it if needed.


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h
File be/src/exec/blocking-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@18
PS3, Line 18: #ifndef IMPALA_EXEC_BLOCKING_PLAN_ROOT_SINK_H
#pragma once, since that's what we're doing in new code


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@56
PS3, Line 56:   virtual Status Send(RuntimeState* state, RowBatch* batch);
Can you add override annotations - that's a good practice for new code.


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@19
PS3, Line 19: #define IMPALA_EXEC_BUFFERED_PLAN_ROOT_SINK_H
#pragma once


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@34
PS3, Line 34:   virtual Status Send(RuntimeState* state, RowBatch* batch);
Can you add override annotations - that's a good practice for new code.


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h@175
PS3, Line 175: ADVANCED
This should be a DEVELOPMENT query option at least until it's ready.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 17:39:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13873/4/tests/query_test/test_result_spooling.py
File tests/query_test/test_result_spooling.py:

http://gerrit.cloudera.org:8080/#/c/13873/4/tests/query_test/test_result_spooling.py@18
PS4, Line 18: import pytest
flake8: F401 'pytest' imported but unused


http://gerrit.cloudera.org:8080/#/c/13873/4/tests/query_test/test_result_spooling.py@22
PS4, Line 22: class TestResultSpooling(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13873/4/tests/query_test/test_result_spooling.py@31
PS4, Line 31:  
flake8: E203 whitespace before ':'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 19:01:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Since no new functionality has been added no tests were added
* Running core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
12 files changed, 369 insertions(+), 154 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Added tests/query_test/test_result_spooling.py; currently only runs a
simple select limit 10 with SPOOL_QUERY_RESULTS = true and validates
that 0 rows are returned
* Ran core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A tests/query_test/test_result_spooling.py
13 files changed, 397 insertions(+), 154 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3892/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 02:27:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Added tests/query_test/test_result_spooling.py; currently only runs a
simple select limit 10 with SPOOL_QUERY_RESULTS = true and validates
that 0 rows are returned
* Ran core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A tests/query_test/test_result_spooling.py
13 files changed, 408 insertions(+), 154 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/13873/7
-- 
To view, visit http://gerrit.cloudera.org:8080/13873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h@24
PS6, Line 24: /// PlanRootSink that buffers RowBatches from the 'sender' (fragment) thread. RowBatches
            : /// are buffered in memory until a memory limit is hit.
> This is just for the initial implementation only, right ? We kind need to s
Yeah, I left the docs for this and the new query option SPOOL_QUERY_RESULTS intentionally vague. The plan is to update the docs in future patches as we add more functionality.


http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h
File be/src/exec/plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h@a67
PS6, Line 67: 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
> nit: these are part of the interfaces of DataSink. It seems clearer to just
Done. Plus updated some of the docs appropriately.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 17:35:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13873/3//COMMIT_MSG@24
PS3, Line 24: * Since no new functionality has been added no tests were added
> Can you add a test that exercises the option to confirm that it doesn't cra
Done - added tests/query_test/test_result_spooling.py - doesn't do much yet, just validates that running a simple query with SPOOL_QUERY_RESULTS = true does not crash Impala.


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h
File be/src/exec/blocking-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@18
PS3, Line 18: #ifndef IMPALA_EXEC_BLOCKING_PLAN_ROOT_SINK_H
> #pragma once, since that's what we're doing in new code
Done


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/blocking-plan-root-sink.h@56
PS3, Line 56:   virtual Status Send(RuntimeState* state, RowBatch* batch);
> Can you add override annotations - that's a good practice for new code.
Done


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@19
PS3, Line 19: #define IMPALA_EXEC_BUFFERED_PLAN_ROOT_SINK_H
> #pragma once
Done


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/exec/buffered-plan-root-sink.h@34
PS3, Line 34:   virtual Status Send(RuntimeState* state, RowBatch* batch);
> Can you add override annotations - that's a good practice for new code.
Done


http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/13873/3/be/src/service/query-options.h@175
PS3, Line 175: ADVANCED
> This should be a DEVELOPMENT query option at least until it's ready.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 19:04:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Since no new functionality has been added no tests were added
* Running core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A tests/query_test/test_result_spooling.py
13 files changed, 397 insertions(+), 154 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 6: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h@24
PS6, Line 24: /// PlanRootSink that buffers RowBatches from the 'sender' (fragment) thread. RowBatches
            : /// are buffered in memory until a memory limit is hit.
This is just for the initial implementation only, right ? We kind need to support spilling to disk eventually so we can produce all the possible result rows from the underlying plan tree.

As it's described now, it's not completely solving the problem of a client not fetching all result rows.


http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h
File be/src/exec/plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h@a67
PS6, Line 67: 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
nit: these are part of the interfaces of DataSink. It seems clearer to just leave them here as pure virtual functions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 04:58:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3891/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 02:19:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/blocking-plan-root-sink.h
File be/src/exec/blocking-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/blocking-plan-root-sink.h@51
PS1, Line 51:   BlockingPlanRootSink(TDataSinkId sink_id, const RowDescriptor* row_desc, RuntimeState* state);
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/blocking-plan-root-sink.cc
File be/src/exec/blocking-plan-root-sink.cc:

http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/blocking-plan-root-sink.cc@89
PS1, Line 89:   // All rows have been sent by the producer, so wake up the producer so it can set eos to true.
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/blocking-plan-root-sink.cc@116
PS1, Line 116:   // Set the shared QueryResultSet pointer 'results_' to the given 'results' object and wake
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/plan-root-sink.cc
File be/src/exec/plan-root-sink.cc:

http://gerrit.cloudera.org:8080/#/c/13873/1/be/src/exec/plan-root-sink.cc@43
PS1, Line 43: void PlanRootSink::ValidateCollectionSlots(const RowDescriptor& row_desc, RowBatch* batch) {
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jul 2019 01:39:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Added tests/query_test/test_result_spooling.py; currently only runs a
simple select limit 10 with SPOOL_QUERY_RESULTS = true and validates
that 0 rows are returned
* Ran core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A tests/query_test/test_result_spooling.py
13 files changed, 396 insertions(+), 154 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13873 )

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4614/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 8
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 19:20:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations
......................................................................

IMPALA-8656: Re-factor PlanRootSink into blocking and buffered implementations

Refactors PlanRootSink into a base class with two subclasses:
BlockingPlanRootSink and BufferedPlanRootSink. BlockingPlanRootSink
encapsulates the current implementation of PlanRootSink and
BufferedPlanRootSink encapsulates a new implementation that will buffer
RowBatches in memory until they are read by the client. The
implementation of BlockingPlanRootSink is left to future work.

A new query option called POOL_QUERY_RESULTS controls whether a
BlockingPlanRootSink or a BufferedPlanRootSink is used as the DataSink.
POOL_QUERY_RESULTS is false by default.

Added a few more docs to PlanRootSink and BlockingPlanRootSink to make
the implementation easier to understand.

Testing:
* Since no new functionality has been added no tests were added
* Ran core tests

Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
---
M be/src/exec/CMakeLists.txt
A be/src/exec/blocking-plan-root-sink.cc
A be/src/exec/blocking-plan-root-sink.h
A be/src/exec/buffered-plan-root-sink.cc
A be/src/exec/buffered-plan-root-sink.h
M be/src/exec/data-sink.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
12 files changed, 369 insertions(+), 154 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>