Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10744 )

Change subject: IMPALA-1760: Implement shutdown command
......................................................................


Patch Set 11: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10744/6/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/10744/6/be/src/service/impala-hs2-server.cc@381
PS6, Line 381: HS2_RETURN_IF_ERROR(return_val, CheckNotShuttingDown(), 
SQLSTATE_GENERAL_ERROR);
> That brings up a good point - I was a bit uncertain about what the best thi
Fair point. Perhaps we can add to the documentation related to shutdown about 
how the HS2 and beeswax API might change in behavior


http://gerrit.cloudera.org:8080/#/c/10744/11/be/src/util/default-path-handlers.cc
File be/src/util/default-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/10744/11/be/src/util/default-path-handlers.cc@228
PS11, Line 228: bool is_quiescing = impala_server->IsShuttingDown();
I think for now we should just stick to is_shutting_down since this statement 
seems confusing as it implies IsShuttingDown being true is equivalent to 
is_quiescing being true which is not true after the quiescence period ends. We 
can generalize this later if we decide to separate the two stages


http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:

http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py@212
PS11, Line 212:     # Test that we can reduce the deadline after setting it to 
a high value.
maybe test that we cannot increase the deadline too



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0
Gerrit-Change-Number: 10744
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Fri, 06 Jul 2018 20:35:18 +0000
Gerrit-HasComments: Yes

Reply via email to