Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12299 )

Change subject: IMPALA-2990: timeout unresponsive queries in coordinator
......................................................................


Patch Set 9:

(9 comments)

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

http://gerrit.cloudera.org:8080/#/c/12299/8//COMMIT_MSG@34
PS8, Line 34:  'REPORT_EXEC_STATUS_DELAY:JITTER@1000'
> Stale ?
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/runtime/query-state.cc@400
PS8, Line 400:  we set backend rpc timeouts for kr
> Can you please leave a TODO on rethinking the timeout period for RPCs in ge
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/runtime/query-state.cc@414
PS8, Line 414: repor
> spent
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc@217
PS8, Line 217:     "report is sent.");
> and only the final report is sent.
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc@218
PS8, Line 218: 600
> To avoid regression, let's be more conservative and set it to be at least 3
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc@221
PS8, Line 221: The coordinator will wait "
             :     "--status_report_max_retry_s * (1 + 
--status_report_cancellation_padding / 100) "
             :     "without receiving a status report before deciding that a 
backend is unresponsive
> May be clearer to include the formula you have in the commit message.
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc@421
PS8, Line 421:       if (FLAGS_abort_on_config_error) {
> May wanna check if --status_report_cancellation_padding is set to something
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/be/src/service/impala-server.cc@2248
PS8, Line 2248:     // this thread.
> DCHECK_GT(max_lag_ms, 0);
Done


http://gerrit.cloudera.org:8080/#/c/12299/8/tests/custom_cluster/test_rpc_timeout.py
File tests/custom_cluster/test_rpc_timeout.py:

http://gerrit.cloudera.org:8080/#/c/12299/8/tests/custom_cluster/test_rpc_timeout.py@44
PS8, Line 44:
> flake8: E501 line too long (103 > 90 characters)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I196c8c6a5633b1960e2c3a3884777be9b3824987
Gerrit-Change-Number: 12299
Gerrit-PatchSet: 9
Gerrit-Owner: Thomas Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Wed, 24 Apr 2019 20:20:44 +0000
Gerrit-HasComments: Yes

Reply via email to