Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10835 )
Change subject: IMPALA-7212: Deprecate --use_krpc flag and remove old DataStream services ...................................................................... Patch Set 1: (5 comments) LGTM overall. Just some comments around the test code. http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc File be/src/runtime/data-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@23 PS1, Line 23: #include "common/status.h" I think I've found all the dead code in this patch, but if you want a reference, you can have a look at this patch to see if you need to remove anything else: https://github.com/apache/impala/commit/ff86feaa67ff8bf703896e33d9a358e42739ae30#diff-0b7021d2a8bfaf517b243f11a53bcde8 I added that when I was making this test KRPC compatible. http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@128 PS1, Line 128: template <class T> class DataStreamTestBase : public T { : protected: : virtual void SetUp() {} : virtual void TearDown() {} : }; No longer needed. http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@134 PS1, Line 134: enum KrpcSwitch { : USE_THRIFT, : USE_KRPC : }; No longer needed. http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@139 PS1, Line 139: public DataStreamTestBase<testing::TestWithParam<KrpcSwitch>> You can replace this with: public testing::Test http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py File tests/custom_cluster/test_rpc_exception.py: http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py@a73 PS1, Line 73: > We should probably look into removing certain injected exception if it's no Can't this happen without TransmitData() today? Don't we also call it on ReportExecStatus() and CancelQueryFInstances() ? https://github.com/apache/impala/blob/fb8ea6a9ccaedc41a71f6f6dcb367fc1facd73b6/be/src/runtime/backend-client.h#L58-L76 -- To view, visit http://gerrit.cloudera.org:8080/10835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3 Gerrit-Change-Number: 10835 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Tue, 03 Jul 2018 18:36:28 +0000 Gerrit-HasComments: Yes
