Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12579 )
Change subject: IMPALA-7800: Time out new connections after --fe_service_threads ...................................................................... Patch Set 11: (2 comments) Looking good. Please address the last two items and I can +2 it. http://gerrit.cloudera.org:8080/#/c/12579/11/be/src/rpc/TAcceptQueueServer.cpp File be/src/rpc/TAcceptQueueServer.cpp: http://gerrit.cloudera.org:8080/#/c/12579/11/be/src/rpc/TAcceptQueueServer.cpp@130 PS11, Line 130: timeout nit: timeout_ms http://gerrit.cloudera.org:8080/#/c/12579/10/tests/custom_cluster/test_frontend_connection_limit.py File tests/custom_cluster/test_frontend_connection_limit.py: http://gerrit.cloudera.org:8080/#/c/12579/10/tests/custom_cluster/test_frontend_connection_limit.py@90 PS10, Line 90: except Exception as e: > The test will be marked as failed in that case (XFAIL). Added an else block My understanding is that xfail will just mark it as "unexpected pass" without flagging any other problem if the test happens to pass. Would it be more robust to set a flag in the exception handling block and assert that the flag is set at the end of the test ? In this way, there is no need to mark the test as xfail. -- To view, visit http://gerrit.cloudera.org:8080/12579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb345c1d84cc2f691f54ded467f253e758f87e64 Gerrit-Change-Number: 12579 Gerrit-PatchSet: 11 Gerrit-Owner: Zoram Thanga <zo...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Wed, 27 Mar 2019 22:35:07 +0000 Gerrit-HasComments: Yes
