Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8950 )
Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc File be/src/runtime/data-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc@689 PS3, Line 689: class DataStreamTestForImpala2931 : public DataStreamTest { : protected: : virtual void SetUp() { : DataStreamTest::SetUp(); : } : : virtual void TearDown() { : DataStreamTest::TearDown(); : } : }; > We can do that, but then the test output shows that the test has run succes I find this class specialization a bit misleading given it's essentialy the same as the base DataStreamTest. We can explicitly log in the test that it's skipped for KRPC if you find the output a bit unclear. If you still prefer to do it with the current approach, please rename the class to DataStreamTestThriftOnly and adds a comment on what this class is for. http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc@703 PS3, Line 703: FLAGS_datastream_service_num_deserialization_threads = 1; : FLAGS_datastream_service_deserialization_queue_size = 1; > We could iterate through different config values like the link above and ru It seems rather hard to reuse the code for exercising other cases in the KRPC code path given this hard coding of the flag here. Is there a better way to write this ? Ideally, we can call this DataStreamTestForKRPCOnly for subset of tests which should be run with KRPC only. -- To view, visit http://gerrit.cloudera.org:8080/8950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7 Gerrit-Change-Number: 8950 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Fri, 19 Jan 2018 09:09:00 +0000 Gerrit-HasComments: Yes
