[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


IMPALA-5744: Add 'use_krpc' flag and create DataStream interface

This patch introduces a 'use_krpc' flag and creates an abstract
interface for the DataStreamRecvr/Mgr.

The 'use_krpc' flag defaults to 'false'. Cluster startup will abort
with an error if the flag is switched to 'true'.

The DataStreamSender implements the same virtual interface as the
DataSink, so a pure virtual class for the DataStreamSender would
essentially be an empty class. Therefore, it is not implemented.

The new interfaces are pure virtual base classes and are named
DataStream*Base. Using pure virtual base classes may result in code
duplication but the current approach is to optimize for the eventual
removal of the thrift implementations.

Stubs for the Krpc implementations are also introduced and are named
KrpcDataStream*. They currently only abort with a fatal error if they
are used. Their actual implementations will be filled in a later
patch.

When the time comes to standardize on the KRPC implementations of
DataStream*Base, we will get rid of the DataStream*Base classes and
the Thrift versions of the classes and rename KrpcDataStream* to
DataStream*. We will also rename all the references that the clients
have to DataStream*Base to DataStream*.

Also did some spurious includes cleanup.

Change-Id: I5d52245154e910529a68f53049520238eca16241
Reviewed-on: http://gerrit.cloudera.org:8080/7542
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/data-stream-mgr-base.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
A be/src/runtime/data-stream-recvr-base.h
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-mgr.cc
A be/src/runtime/krpc-data-stream-mgr.h
A be/src/runtime/krpc-data-stream-recvr.cc
A be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/service/impala-server.cc
20 files changed, 436 insertions(+), 39 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 13:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-09 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 13: Code-Review+2

More clang-tidy fixes.

Rebase, carry +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-09 Thread Sailesh Mukil (Code Review)
Hello Impala Public Jenkins, Henry Robinson, Michael Ho,

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

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

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

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..

IMPALA-5744: Add 'use_krpc' flag and create DataStream interface

This patch introduces a 'use_krpc' flag and creates an abstract
interface for the DataStreamRecvr/Mgr.

The 'use_krpc' flag defaults to 'false'. Cluster startup will abort
with an error if the flag is switched to 'true'.

The DataStreamSender implements the same virtual interface as the
DataSink, so a pure virtual class for the DataStreamSender would
essentially be an empty class. Therefore, it is not implemented.

The new interfaces are pure virtual base classes and are named
DataStream*Base. Using pure virtual base classes may result in code
duplication but the current approach is to optimize for the eventual
removal of the thrift implementations.

Stubs for the Krpc implementations are also introduced and are named
KrpcDataStream*. They currently only abort with a fatal error if they
are used. Their actual implementations will be filled in a later
patch.

When the time comes to standardize on the KRPC implementations of
DataStream*Base, we will get rid of the DataStream*Base classes and
the Thrift versions of the classes and rename KrpcDataStream* to
DataStream*. We will also rename all the references that the clients
have to DataStream*Base to DataStream*.

Also did some spurious includes cleanup.

Change-Id: I5d52245154e910529a68f53049520238eca16241
---
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/data-stream-mgr-base.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
A be/src/runtime/data-stream-recvr-base.h
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-mgr.cc
A be/src/runtime/krpc-data-stream-mgr.h
A be/src/runtime/krpc-data-stream-recvr.cc
A be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/service/impala-server.cc
20 files changed, 436 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/7542/13
-- 
To view, visit http://gerrit.cloudera.org:8080/7542
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 12: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 12:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 12: Code-Review+2

Rebase conflict. Fixed. Carry +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Sailesh Mukil (Code Review)
Hello Impala Public Jenkins, Henry Robinson, Michael Ho,

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

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

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

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..

IMPALA-5744: Add 'use_krpc' flag and create DataStream interface

This patch introduces a 'use_krpc' flag and creates an abstract
interface for the DataStreamRecvr/Mgr.

The 'use_krpc' flag defaults to 'false'. Cluster startup will abort
with an error if the flag is switched to 'true'.

The DataStreamSender implements the same virtual interface as the
DataSink, so a pure virtual class for the DataStreamSender would
essentially be an empty class. Therefore, it is not implemented.

The new interfaces are pure virtual base classes and are named
DataStream*Base. Using pure virtual base classes may result in code
duplication but the current approach is to optimize for the eventual
removal of the thrift implementations.

Stubs for the Krpc implementations are also introduced and are named
KrpcDataStream*. They currently only abort with a fatal error if they
are used. Their actual implementations will be filled in a later
patch.

When the time comes to standardize on the KRPC implementations of
DataStream*Base, we will get rid of the DataStream*Base classes and
the Thrift versions of the classes and rename KrpcDataStream* to
DataStream*. We will also rename all the references that the clients
have to DataStream*Base to DataStream*.

Also did some spurious includes cleanup.

Change-Id: I5d52245154e910529a68f53049520238eca16241
---
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/data-stream-mgr-base.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
A be/src/runtime/data-stream-recvr-base.h
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-mgr.cc
A be/src/runtime/krpc-data-stream-mgr.h
A be/src/runtime/krpc-data-stream-recvr.cc
A be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/service/impala-server.cc
20 files changed, 441 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/7542/12
-- 
To view, visit http://gerrit.cloudera.org:8080/7542
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 11:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Sailesh Mukil (Code Review)
Hello Impala Public Jenkins, Henry Robinson, Michael Ho,

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

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

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

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..

IMPALA-5744: Add 'use_krpc' flag and create DataStream interface

This patch introduces a 'use_krpc' flag and creates an abstract
interface for the DataStreamRecvr/Mgr.

The 'use_krpc' flag defaults to 'false'. Cluster startup will abort
with an error if the flag is switched to 'true'.

The DataStreamSender implements the same virtual interface as the
DataSink, so a pure virtual class for the DataStreamSender would
essentially be an empty class. Therefore, it is not implemented.

The new interfaces are pure virtual base classes and are named
DataStream*Base. Using pure virtual base classes may result in code
duplication but the current approach is to optimize for the eventual
removal of the thrift implementations.

Stubs for the Krpc implementations are also introduced and are named
KrpcDataStream*. They currently only abort with a fatal error if they
are used. Their actual implementations will be filled in a later
patch.

When the time comes to standardize on the KRPC implementations of
DataStream*Base, we will get rid of the DataStream*Base classes and
the Thrift versions of the classes and rename KrpcDataStream* to
DataStream*. We will also rename all the references that the clients
have to DataStream*Base to DataStream*.

Also did some spurious includes cleanup.

Change-Id: I5d52245154e910529a68f53049520238eca16241
---
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/data-stream-mgr-base.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
A be/src/runtime/data-stream-recvr-base.h
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-mgr.cc
A be/src/runtime/krpc-data-stream-mgr.h
A be/src/runtime/krpc-data-stream-recvr.cc
A be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/service/impala-server.cc
20 files changed, 443 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/7542/11
-- 
To view, visit http://gerrit.cloudera.org:8080/7542
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-08 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 11: Code-Review+2

Fixed a clang-tidy warning.

Rebase, carry +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 10:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 10:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 10: Code-Review+2

Thanks for the review!

Carry +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Henry Robinson (Code Review)
Henry Robinson has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 9: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 8: Code-Review+2

(8 comments)

Thanks for the review. Will also wait for Henry's final comments/confirmation.

Carry +2.

http://gerrit.cloudera.org:8080/#/c/7542/8//COMMIT_MSG
Commit Message:

PS8, Line 7: dummy 'use_krpc' flag
> The flag is not really dummy, right ? We are just using dummy implementatio
Done


Line 20: DataStream*Base.
> Using pure virtual base class may result in code duplication but the curren
Done


PS8, Line 27: Since having both the Thrift and KRPC implementations of the
: DataStream* classes are only expected to be temporary for now, 
this
: was written and optimized with the end goal of having only the 
KRPC
: versions of the DataStreamMgr/Recvr, at which point we will get 
rid
: of the DataStream*Base classes, the Thrift versions of the classes
: and rename KrpcDataStream* to DataStream*. We will also rename all
: the references that the clients have to DataStream*Base to 
DataStream*.
> May remove or shorten this entire paragraph if you take the one line sugges
Done


http://gerrit.cloudera.org:8080/#/c/7542/8/be/src/runtime/data-stream-mgr-base.h
File be/src/runtime/data-stream-mgr-base.h:

PS8, Line 35: enforces a basic interface
> defines the basic interface
Done


PS8, Line 36: , one each for Thrift and KRPC.
> for thrift and KRPC respectively.
There's nothing for them to be respective to, as Henry mentioned in his 
comments in the previous patchset.


http://gerrit.cloudera.org:8080/#/c/7542/8/be/src/runtime/data-stream-recvr-base.h
File be/src/runtime/data-stream-recvr-base.h:

PS8, Line 32: enforces a basic interface
> defines the basic interface
Done


PS8, Line 33: , one each for Thrift and KRPC
> for thrift and KRPC respectively.
There's nothing for them to be respective to, as Henry mentioned in his 
comments in the previous patchset.


http://gerrit.cloudera.org:8080/#/c/7542/8/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:

PS8, Line 92: Clients of the implementations of DataStreamMgrBase should not 
use these functions
> Clients of DataStreamMgrBase should use stream_mgr() unless ...
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

2017-08-07 Thread Sailesh Mukil (Code Review)
Hello Michael Ho,

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

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

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

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..

IMPALA-5744: Add 'use_krpc' flag and create DataStream interface

This patch introduces a 'use_krpc' flag and creates an abstract
interface for the DataStreamRecvr/Mgr.

The 'use_krpc' flag defaults to 'false'. Cluster startup will abort
with an error if the flag is switched to 'true'.

The DataStreamSender implements the same virtual interface as the
DataSink, so a pure virtual class for the DataStreamSender would
essentially be an empty class. Therefore, it is not implemented.

The new interfaces are pure virtual base classes and are named
DataStream*Base. Using pure virtual base classes may result in code
duplication but the current approach is to optimize for the eventual
removal of the thrift implementations.

Stubs for the Krpc implementations are also introduced and are named
KrpcDataStream*. They currently only abort with a fatal error if they
are used. Their actual implementations will be filled in a later
patch.

When the time comes to standardize on the KRPC implementations of
DataStream*Base, we will get rid of the DataStream*Base classes and
the Thrift versions of the classes and rename KrpcDataStream* to
DataStream*. We will also rename all the references that the clients
have to DataStream*Base to DataStream*.

Also did some spurious includes cleanup.

Change-Id: I5d52245154e910529a68f53049520238eca16241
---
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/data-stream-mgr-base.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
A be/src/runtime/data-stream-recvr-base.h
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-mgr.cc
A be/src/runtime/krpc-data-stream-mgr.h
A be/src/runtime/krpc-data-stream-recvr.cc
A be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/service/impala-server.cc
20 files changed, 441 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/7542/9
-- 
To view, visit http://gerrit.cloudera.org:8080/7542
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil