[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Reviewed-on: http://gerrit.cloudera.org:8080/8202
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 346 insertions(+), 214 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 25
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:14:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1459/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 17:52:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 17:51:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#24).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 346 insertions(+), 214 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/24
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 22:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1930
PS22, Line 1930: int32_t hs2_port) {
> fix indentation
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1948
PS22, Line 1948: ;
> remove
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1980
PS22, Line 1980:   boost::shared_ptr beeswax_processor(new 
ImpalaServiceProcessor(handler));
> nit: long line
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:

http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impalad-main.cc@88
PS22, Line 88: FLAGS_hs2_port
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/bin/start-impala-cluster.py@316
PS22, Line 316:   """Waits for a catalog copy to be received by the impalad. 
When its received, additionally
> nit: long line
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/tests/common/custom_cluster_test_suite.py@125
PS22, Line 125: cluster_size=CLUSTER_SIZE, num_coordinators=NUM_COORDINATORS,
  : use_exclusive_coordinators=False, 
log_level=1,
  : 
expected_num_executors=CLUSTER_SIZE):
> indentation
Done


http://gerrit.cloudera.org:8080/#/c/8202/22/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/tests/custom_cluster/test_catalog_wait.py@81
PS22, Line 81: # TODO: add more tests
 :   # - impalads that are just coordinators
 :   # - impalads that are just executors
 :   # - process reincarnation, e.g., healthy impalad crashes and 
is restarted
> remove
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 10 Nov 2017 19:20:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#23).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 346 insertions(+), 214 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/23
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 23
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 22: Code-Review+2

(7 comments)

Minor formatting nits. Tests seem reasonable to me.

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

http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1930
PS22, Line 1930: int32_t hs2_port) {
fix indentation


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1948
PS22, Line 1948: ;
remove


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impala-server.cc@1980
PS22, Line 1980:   boost::shared_ptr beeswax_processor(new 
ImpalaServiceProcessor(handler));
nit: long line


http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:

http://gerrit.cloudera.org:8080/#/c/8202/22/be/src/service/impalad-main.cc@88
PS22, Line 88: FLAGS_hs2_port
nit: indentation


http://gerrit.cloudera.org:8080/#/c/8202/22/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/bin/start-impala-cluster.py@316
PS22, Line 316:   """Waits for a catalog copy to be received by the impalad. 
When its received, additionally
nit: long line


http://gerrit.cloudera.org:8080/#/c/8202/22/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/tests/common/custom_cluster_test_suite.py@125
PS22, Line 125: cluster_size=CLUSTER_SIZE, num_coordinators=NUM_COORDINATORS,
  : use_exclusive_coordinators=False, 
log_level=1,
  : 
expected_num_executors=CLUSTER_SIZE):
indentation


http://gerrit.cloudera.org:8080/#/c/8202/22/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:

http://gerrit.cloudera.org:8080/#/c/8202/22/tests/custom_cluster/test_catalog_wait.py@81
PS22, Line 81: # TODO: add more tests
 :   # - impalads that are just coordinators
 :   # - impalads that are just executors
 :   # - process reincarnation, e.g., healthy impalad crashes and 
is restarted
remove



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 10 Nov 2017 07:00:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 22:

thanks for the reviews, who can +2 this?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 09 Nov 2017 01:15:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 22: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 08 Nov 2017 00:55:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#22).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 353 insertions(+), 216 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/22
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#21).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 355 insertions(+), 216 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/21
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 20:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@89
PS19, Line 89: must call the following
 : /// methods:
> That DCHECK:
combined them.


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

http://gerrit.cloudera.org:8080/#/c/8202/20/be/src/service/impala-server.cc@1936
PS20, Line 1936:
> I think a quick comment is worth it to highlight the dependency between the
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 20
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 07 Nov 2017 23:08:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 20:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@89
PS19, Line 89: must call the following
 : /// methods:
> good question-- I've kept it the same as before so not sure what the reason
That DCHECK:

  DCHECK(exec_env.process_mem_tracker() != nullptr)
  << "ExecEnv::StartServices() must be called before starting RPC services";

looks stale since the process_mem_tracker() is set in exec_env_.Init() which is 
called earlier in main (not to mention StartServices() doesn't exist anymore). 
I also don't see what the specific dependency is that this is trying to point 
out (this DCHECK should have been closer to the code that requires this to be 
true).

If you want to keep the dcheck, I see no reason it couldn't be moved before 
both Init and Start. Then we could combine them, since there doesn't appear to 
be any logic behind the separation of those two.  But you can defer cleaning 
this up if you prefer (though the dcheck wording should be updated at least).


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

http://gerrit.cloudera.org:8080/#/c/8202/20/be/src/service/impala-server.cc@1936
PS20, Line 1936:
I think a quick comment is worth it to highlight the dependency between these 
two lines:
// Subscribe to the catalog topic and then wait for the initial catalog update.
(or whatever is accurate)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 20
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 07 Nov 2017 18:04:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 20: Code-Review+1

carrying +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 20
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 07 Nov 2017 06:16:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#20).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 302 insertions(+), 153 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/20
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 20
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 19:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/frontend.h
File be/src/service/frontend.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/frontend.h@162
PS19, Line 162: Sets the FE catalog to be initialized.
> I don't understand what that's trying to say.
updated the comment to better reflect the method renaming.


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@79
PS19, Line 79: The executor role starts ImpalaInternalService API's
> technically the coordinator also has to start the ImpalaInternalService API
Done


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@89
PS19, Line 89: must call the following
 : /// methods:
> why is this split into two steps? Is there something the caller would want
good question-- I've kept it the same as before so not sure what the reason was 
to keep them separated. currently, there is a check in impalad-main in between 
init and start, but perhaps we can do without that.


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

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.cc@1518
PS19, Line 1518:
> nit: indentation glitch
Done


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.cc@1937
PS19, Line 1937:   RETURN_IF_ERROR(exec_env_->StartServices());
> why does this have to be moved so early? i'm not sure if this will cause pr
I need it for the statestore subscriber to receive the catalog. L1939 will not 
work otherwise. Not sure why tests did not run into any issues. For now, I've 
separated them so that krpc, when enabled, starts at the same time as before 
this change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 19
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 07 Nov 2017 06:14:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 19:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/frontend.h
File be/src/service/frontend.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/frontend.h@162
PS19, Line 162: Sets the FE catalog to be initialized.
I don't understand what that's trying to say.


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@79
PS19, Line 79: The executor role starts ImpalaInternalService API's
technically the coordinator also has to start the ImpalaInternalService API in 
order to run the coordinator fragment instance.


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.h@89
PS19, Line 89: must call the following
 : /// methods:
why is this split into two steps? Is there something the caller would want to 
do between opening the ports and starting the services?


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

http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.cc@1518
PS19, Line 1518:
nit: indentation glitch


http://gerrit.cloudera.org:8080/#/c/8202/19/be/src/service/impala-server.cc@1937
PS19, Line 1937:   RETURN_IF_ERROR(exec_env_->StartServices());
why does this have to be moved so early? i'm not sure if this will cause 
problems when krpc is enabled.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 19
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 06 Nov 2017 23:22:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 19: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 19
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 19:06:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   /// Flag that records if backend and/or client services have 
been started.
> they're on back-to-back lines now in impala_server.cc so I think we're fine
Ahh right, I wrote this comment before seeing the other file :). lgtm



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 19:06:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Juan Yu, Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris 
Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#19).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
18 files changed, 282 insertions(+), 144 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/19
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 19
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 18:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   /// an accurate expiration time, and this structure 
guarantees that we will always
> Works for me. To make the relationship between 'services_started_' and 'is_
they're on back-to-back lines now in impala_server.cc so I think we're fine. if 
they drift to different locations, makes sense to add the dcheck.


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

http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc@2050
PS18, Line 2050: I
> just set to true?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 18
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 19:03:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 18: Code-Review+1

(3 comments)

I'm happy with this change, but would be great to get another pair of eyes on 
it. Dan, can you take a look?

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98
PS17, Line 98: ///- Start ImpalaInternalService API
> currently, the main flips this bit-- its the one orchestrating the sequence
sounds good


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   /// an accurate expiration time, and this structure 
guarantees that we will always
> I think the thing I'm objecting to here is that the internal state will dep
Works for me. To make the relationship between 'services_started_' and 
'is_ready' clear, I think we should add a DCHECK right before setting 
'is_ready' that asserts 'services_started_' must be true.


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

http://gerrit.cloudera.org:8080/#/c/8202/18/be/src/service/impala-server.cc@2050
PS18, Line 2050: I
just set to true?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 18
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 18:51:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java@859
PS18, Line 859:  Waits indefinitely
> Will this impact shutting down?
at least with the start-impala-cluster script, it had no issue. that script 
relies on killing the process.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 18
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 17:52:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Juan Yu (Code Review)]

Juan Yu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/18/fe/src/main/java/org/apache/impala/service/Frontend.java@859
PS18, Line 859:  Waits indefinitely
Will this impact shutting down?
If after wait for 30 seconds I decide to restart the whole service, could the 
Impalad be shutdown or restart quickly?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 18
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 17:43:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#18).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
18 files changed, 282 insertions(+), 144 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/18
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 18
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 17:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@81
PS17, Line 81: ///
> whitespace
Done


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@85
PS17, Line 85: /// by clients at the same time.
> Might want to weave in something like this to motivate the specific startup
Done


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@87
PS17, Line 87: /// 1. Init
> Init()
Done


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@89
PS17, Line 89: ///- Wait (indefinitelt) for local catalog to be initialized 
from statestore (if coordinator)
> typo: indefinitely
Done


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@93
PS17, Line 93: /// 2. Start
> Start()
Done


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98
PS17, Line 98: /// Membership callback thread:
> When is the 'is_ready' metric set?
currently, the main flips this bit-- its the one orchestrating the sequence so 
it knows when its ready. since we've now equated services_started_ and the 
ready flag, I'll consolidate them and move them to the server. perhaps there 
were other things the main wanted to do before announcing itself as ready, but 
for now, they're 1:1.


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   ExpirationQueue queries_by_timestamp_;
> Fair point.
I think the thing I'm objecting to here is that the internal state will depend 
on an external view. I'd prefer the arrows to be oriented only from internal to 
external (external depends on internal), even at the expense of storing this 
value twice. By using the metric, its the same as accessing a global. I don't 
see any example where we use metric values for internal state/logic-- afaict, 
they're used for sanity checks or logging (in addition to their intended 
purpose). I think its preferable to keep it that way.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 17
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 17:30:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 17:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@81
PS17, Line 81: ///
whitespace


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@85
PS17, Line 85: /// by clients at the same time.
Might want to weave in something like this to motivate the specific startup 
sequence:

The Impala server is considered 'ready' iff it can successfully process 
requests in all of its roles. The goal is make the state of the service easy to 
reason about.


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@87
PS17, Line 87: /// 1. Init
Init()


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@89
PS17, Line 89: ///- Wait (indefinitelt) for local catalog to be initialized 
from statestore (if coordinator)
typo: indefinitely


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@93
PS17, Line 93: /// 2. Start
Start()


http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98
PS17, Line 98: /// Membership callback thread:
When is the 'is_ready' metric set?


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   ExpirationQueue queries_by_timestamp_;
> clarified the comment.
Fair point.
My preference would be to use 'is_ready' to avoid redundant state. If metrics 
are seen as a view on the internal state (which I agree with!), then 'is_ready' 
should report the value of this 'services_started_' flag and not store a 
separate value.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 17
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 16:21:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#17).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
17 files changed, 275 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/17
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 17
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 16:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@105
PS16, Line 105: /// TODO: The state of a running query is currently not cleaned 
up if the
> Let's document the startup procedure and the reasoning behind it here
Done


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   /// Flag that records if backend and/or client services have 
been started.
> Clarify the meaning of "and/or" here. The distinction seems important. Will
clarified the comment.

we can use "is_ready", but I think of metrics as a view on internal state, so 
I'd prefer to not have internal state depend on them. any preferences on the 
topic?


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1017
PS16, Line 1017:   boost::mutex services_started_lock_;
> std::atomic_bool instead of this lock?
Done


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

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1626
PS16, Line 1626:   // Register this backend only if services have been started.
> Feels clearer to me to check this at the caller. Can be hard to reason abou
had it at the caller before-- agreed that its clearer there.


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1933
PS16, Line 1933: Status ImpalaServer::Init(int32_t thrift_be_port, int32_t 
beeswax_port,
> Why reformat fn args? Seemed ok the way it was.
Done


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:

http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@24
PS16, Line 24:   """Impalad must wait for the catalog prior to opening up 
client ports.
> Specify the waiting condition more precisely. An impalad must wait for the
Done


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@54
PS16, Line 54: self.expect_connection(self.cluster.impalads[0])
> Ideally we'd also check the internal service
added a comment when issuing a query. a query will catch two cases: (1) impalad 
registered prematurely is caught by query fragment metrics and (2) query 
failure if the impalad registered as a backend but could not run a fragment.


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@59
PS16, Line 59:   def test_query_subset(self):
> Can we combine this test wit the one above? They seem very similar
Done


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@71
PS16, Line 71: 
self.cluster.impalads[0].service.get_metric_value('impala-server.ready', 1);
> I'm wondering whether this can be flaky. We often use self.wait_for_metric_
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 07:19:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 16:

(9 comments)

Nice, this looks much better

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@105
PS16, Line 105: /// TODO: The state of a running query is currently not cleaned 
up if the
Let's document the startup procedure and the reasoning behind it here


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014
PS16, Line 1014:   /// Flag that records if backend and/or client services have 
been started.
Clarify the meaning of "and/or" here. The distinction seems important. Will 
this flag be set if only one service has been started or not?


I know I suggested this new flag :), but now I'm wondering whether we can use 
the existing "is_ready" metric for this purpose.


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1017
PS16, Line 1017:   boost::mutex services_started_lock_;
std::atomic_bool instead of this lock?


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

http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1626
PS16, Line 1626:   // Register this backend only if services have been started.
Feels clearer to me to check this at the caller. Can be hard to reason about 
functions that are no-ops depending on internal state.
Any reason to have the check in here?

In any case we should add a function comment for the new behavior (ideally in 
the membership callback assuming you agree to move this check)


http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.cc@1933
PS16, Line 1933: Status ImpalaServer::Init(int32_t thrift_be_port, int32_t 
beeswax_port,
Why reformat fn args? Seemed ok the way it was.


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:

http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@24
PS16, Line 24:   """Impalad must wait for the catalog prior to opening up 
client ports.
Specify the waiting condition more precisely. An impalad must wait for the 
initial catalog update to arrive via the statestore.


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@54
PS16, Line 54: self.expect_connection(self.cluster.impalads[0])
Ideally we'd also check the internal service


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@59
PS16, Line 59:   def test_query_subset(self):
Can we combine this test wit the one above? They seem very similar


http://gerrit.cloudera.org:8080/#/c/8202/16/tests/custom_cluster/test_catalog_wait.py@71
PS16, Line 71: 
self.cluster.impalads[0].service.get_metric_value('impala-server.ready', 1);
I'm wondering whether this can be flaky. We often use 
self.wait_for_metric_value() in case we expect delays. These impalads might 
still be waiting for the initial catalog update.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 05:11:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#16).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
17 files changed, 269 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/16
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 15:

(8 comments)

grouped port opening to reflect the all-or-nothing behavior for services on 
startup.

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc@31
PS15, Line 31: #ifndef NDEBUG
> What's the harm in always compiling it in?
its for testing purposes and I saw other flags like this that were controlled 
this way. so one reason is convention, and the other is to disallow this being 
set for release binaries.


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc@256
PS15, Line 256:   if (result) return true;
> return result?
Done


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.h@125
PS15, Line 125:   /// Initializes client RPC services. Must be called after 
InitInternalServices. Is a
> we typically say InitInternalServices() to make it clear it's a function
Done


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

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.cc@1541
PS15, Line 1541: // Additionally wait for the local catalog to be 
initialized if the server is a
> Suggest rephrasing "wait" to clarify. "wait" could be misinterpreted as blo
Done


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impalad-main.cc@91
PS15, Line 91:   Status status = impala_server->StartInternalServices();
> As discussed, ordering is not quite right yet.
rearranged it. I'm keeping one flag for all these ports, since we're grouping 
their behavior now.


http://gerrit.cloudera.org:8080/#/c/8202/15/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/15/bin/start-impala-cluster.py@81
PS15, Line 81: # For testing: list of comma-separated delays, in milliseconds, 
that delay catalog
> clarify which catalog exactly
Done


http://gerrit.cloudera.org:8080/#/c/8202/15/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/15/fe/src/main/java/org/apache/impala/service/Frontend.java@877
PS15, Line 877: }
> add a final LOG.info() here declaring success (and remove the one in impala
this happens on L870. got rid of the one from impala-server.cc


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java@a69
PS12, Line 69:
> Sentry can configured in a service mode or with a file-based auth policy. I
gone!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 15
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 31 Oct 2017 02:08:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 15:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc@31
PS15, Line 31: #ifndef NDEBUG
What's the harm in always compiling it in?


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/frontend.cc@256
PS15, Line 256:   if (result) return true;
return result?


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.h@125
PS15, Line 125:   /// Initializes client RPC services. Must be called after 
InitInternalServices. Is a
we typically say InitInternalServices() to make it clear it's a function


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

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impala-server.cc@1541
PS15, Line 1541: // Additionally wait for the local catalog to be 
initialized if the server is a
Suggest rephrasing "wait" to clarify. "wait" could be misinterpreted as 
blocking. Maybe say something like:

Announce the availability of this impalad coordinator once the local catalog 
has been initialized.


http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:

http://gerrit.cloudera.org:8080/#/c/8202/15/be/src/service/impalad-main.cc@91
PS15, Line 91:   Status status = impala_server->StartInternalServices();
As discussed, ordering is not quite right yet.

It might make sense to add flags for client_services_started and 
internal_services_started to address the race between opening internal/client 
ports and announcing the availability of this daemon through the membership 
callback.


http://gerrit.cloudera.org:8080/#/c/8202/15/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/15/bin/start-impala-cluster.py@81
PS15, Line 81: # For testing: list of comma-separated delays, in milliseconds, 
that delay catalog
clarify which catalog exactly


http://gerrit.cloudera.org:8080/#/c/8202/15/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/15/fe/src/main/java/org/apache/impala/service/Frontend.java@877
PS15, Line 877: }
add a final LOG.info() here declaring success (and remove the one in 
impala-server)


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java@a69
PS12, Line 69:
> I was a bit surprised to see authorization configs here-- what was the inte
Sentry can configured in a service mode or with a file-based auth policy. I 
suppose this test was checking that the readiness check works regardless of 
auth policy. Not sure how much sense that makes.

For auth we use AuthorizationTest.

I'm thinking this test doesn't make sense anymore.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 15
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 30 Oct 2017 21:36:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#15).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
18 files changed, 306 insertions(+), 110 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/15
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 15
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 14:

Current patch has a potential maintainability issue: while a catalog is being 
initialized, if the daemon is an exector, it can evaluate fragments.

The follow-up patch simplifies start-up so that if a daemon is specified as 
both an executor and coordinator, all services are opened at the same time. 
That means that a daemon *will not* evaluate fragments if its also a 
coordinator and its catalog has not yet been initialized.

The patch also backs out the testing flag that stops a catalog process from 
starting-- its a bit hacky and I've replaced it with a configurable, per daemon 
delay that controls the minimum time needed for catalog initialization.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 14
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 30 Oct 2017 19:40:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#14).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
17 files changed, 306 insertions(+), 121 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/14
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 14
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 13:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045: .max_concurrent_connections(FLAGS_fe_service_threads)
> Have you tried a custom_cluster test?
yes. there's a test there in this patch. however, I ran into issues with 
killing/starting processes. the way I'm doing it leaves zombies which then 
block followup kill/start cluster commands that the e2e tests use.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 13
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 25 Oct 2017 17:29:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 12:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045:   if (thrift_be_server_.get()) {
> working on them... starting/stopping cluster processes does not interact we
Have you tried a custom_cluster test?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 25 Oct 2017 17:27:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#13).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
M tests/custom_cluster/test_breakpad.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
18 files changed, 308 insertions(+), 123 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/13
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 13
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 12:

(11 comments)

main change with the patch is to separate internal vs. client init/start.

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.h
File be/src/service/frontend.h:

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.h@171
PS12, Line 171:   Status WaitForCatalog();
> Why even return anything? Success is implied by this function returning.
yup, I had already updated the front end to not return anything.
I simplified the SetCatalogInitialized method in the same manner.


http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.cc@253
PS12, Line 253:   JniLocalFrame jni_frame;
> This JNI frame stuff is not needed here because we are not creating any loc
removed and simplified the setcataloginitialized method in the same manner.


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

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045:   if (thrift_be_server_.get()) {
> I wonder if this can cause problems. A coordinator+executor impalad might h
yes, was able to repro a case where I restart an impalad (but disallow its 
catalog to be initialized), I submit a query to another impalad whose catalog 
is initialized, a query fragment gets sent to the restarted impalad, its 
connection is refused, so the query dies.

I've changed things so that internal vs. client init and start are separated. 
Now, an impalad can process work even though if its supposed to come up as a 
coordinator (and is still waiting on the catalog).


http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045:   if (thrift_be_server_.get()) {
> Yeah, we should make sure we have test cases for this and the various coord
working on them... starting/stopping cluster processes does not interact well 
with the e2e tests. I was able to manually start up a cluster that works for 
the repro above.


http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py@81
PS12, Line 81: parser.add_option("--disable_catalog", dest="disable_catalog", 
default=False,
> disable_catalogd? We seem to be consistently referring to processes with th
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py@255
PS12, Line 255: def is_catalog_ready(impala_cluster):
> is_catalogd_up()?
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@865
PS12, Line 865: LOG.info("Waiting for local catalog to be initialized.");
> Users may not know what "initialized" means, better to state explicitly wha
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@866
PS12, Line 866: int num_tries = 0;
> numTries
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@905
PS12, Line 905: "Analyzing a query is not support when the local 
catalog is not initialized.");
> I'd chose a different phrasing because "not supported" could be misinterpre
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/JniFrontend.java@583
PS12, Line 583:   public void setCatalogInitialized() {
> setCatalogIsReady() for consistency
Done


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java@a69
PS12, Line 69:
> Can you think of a reason for keeping this test around? The interesting cas
I was a bit surprised to see authorization configs here-- what was the intent 
with regard to catalog readiness in the first place? If the authorization tests 
do not matter, then I'll remove the test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 12:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045:   if (thrift_be_server_.get()) {
> I wonder if this can cause problems. A coordinator+executor impalad might h
Yeah, we should make sure we have test cases for this and the various 
coord/exec combinations.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 24 Oct 2017 20:26:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 12:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.h
File be/src/service/frontend.h:

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.h@171
PS12, Line 171:   Status WaitForCatalog();
Why even return anything? Success is implied by this function returning.


http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/frontend.cc@253
PS12, Line 253:   JniLocalFrame jni_frame;
This JNI frame stuff is not needed here because we are not creating any local 
references.

The same is true for SetCatalogInitialized() - frame stuff not needed.


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

http://gerrit.cloudera.org:8080/#/c/8202/12/be/src/service/impala-server.cc@2045
PS12, Line 2045:   if (thrift_be_server_.get()) {
I wonder if this can cause problems. A coordinator+executor impalad might have 
registered with the statestore but not received the initial catalog update yet. 
This will not stop other coordinators from scheduling work on this impalad - 
but the InternalService port has not yet been opened, meaning those queries 
will fail.

I think we need to strictly distinguish between the internal and client facing 
services.


http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py@81
PS12, Line 81: parser.add_option("--disable_catalog", dest="disable_catalog", 
default=False,
disable_catalogd? We seem to be consistently referring to processes with their 
"d" suffix


http://gerrit.cloudera.org:8080/#/c/8202/12/bin/start-impala-cluster.py@255
PS12, Line 255: def is_catalog_ready(impala_cluster):
is_catalogd_up()?

To distinguish the other "catalog ready" which refers to the local catalog 
cache of an impalad.


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@865
PS12, Line 865: LOG.info("Waiting for local catalog to be initialized.");
Users may not know what "initialized" means, better to state explicitly what we 
are waiting for, e.g.:

Waiting for the first catalog update from the statestore.


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@866
PS12, Line 866: int num_tries = 0;
numTries


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/Frontend.java@905
PS12, Line 905: "Analyzing a query is not support when the local 
catalog is not initialized.");
I'd chose a different phrasing because "not supported" could be misinterpreted 
as a missing feature or might make the user think he/she did something wrong - 
but hitting this is not expected and probably a bug on our side. How about:

"Local catalog has not been initialized. Aborting query analysis."


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/main/java/org/apache/impala/service/JniFrontend.java@583
PS12, Line 583:   public void setCatalogInitialized() {
setCatalogIsReady() for consistency


http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/8202/12/fe/src/test/java/org/apache/impala/service/FrontendTest.java@a69
PS12, Line 69:
Can you think of a reason for keeping this test around? The interesting case 
was the "not ready" case which is now gone, so I'm not sure this test makes 
sense anymore.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 12: Code-Review+1

(1 comment)

for the test infra

http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@317
PS11, Line 317:
> Done. Is there a python linter that folks use to catch cases like this?
I like to use flake8 (pip install flake8). I have this in setup.cfg:

  [flake8]
  ignore = E111,E114
  max-line-length = 90



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 23 Oct 2017 22:15:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Michael Brown, Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, 
Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#12).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 168 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/12
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 11:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/frontend.cc@242
PS11, Line 242: Status Frontend::SetCatalogInitialized() {
  :   JNIEnv* jni_env = getJNIEnv();
  :   JniLocalFrame jni_frame;
  :   RETURN_IF_ERROR(jni_frame.push(jni_env));
  :   jni_env->CallObjectMethod(fe_, set_catalog_initialized_id_);
  :   RETURN_ERROR_IF_EXC(jni_env);
  :   return Status::OK();
  : }
> is that needed? could it be handled entirely within InProcessImpalaServer?
its also used by expr-benchmark as well, which instantiates its own frontend 
class (case of neither InProcess nor daemon). there's probably some 
simplification here, but yes, I agree that its orthogonal.


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

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/impala-server.cc@1979
PS11, Line 1979:   // Wait for the frontend catalog to be ready prior to 
opening client ports.
> nit: Maybe expand this comment to mention that this call will block indefin
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@82
PS11, Line 82: action="store_true",
 :   help=SUPPRESS_HELP)
> nit: merge these two and save a line :)
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@258
PS11, Line 258: return not impala_cluster.catalogd and not 
options.disable_catalog
> Why not:
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@304
PS11, Line 304: wait_for_client
> I think this should be named wait_for_catalog since that is the high level
this method additionally waits for the client ports to be opened. changed the 
name and the comment to clarify.


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@317
PS11, Line 317: ;
> remove stray semicolon
Done. Is there a python linter that folks use to catch cases like this?


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@323
PS11, Line 323: client_beeswax.close()
> What about closing the HS2 client?
it does not have a close afaict.


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@327
PS11, Line 327: Client ports within %s seconds.' % timeout_in_seconds
> Can we make this error message clearer?  "Client ports not ready within N s
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@407
PS11, Line 407: # Check for the cluster to be ready only when the catalog 
was started.
  : if not options.disable_catalog:
  :   wait_for_cluster()
> Is this too lax? Can we not call wait_for_impala_process_count()?
simplified... wait for cluster should do the right thing when the cluster is 
started with out the catalog.


http://gerrit.cloudera.org:8080/#/c/8202/11/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/tests/common/custom_cluster_test_suite.py@127
PS11, Line 127: if disable_catalog:
  :   cmd.append("--disable_catalog")
> nit: single line
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py
File tests/custom_cluster/test_catalog_wait.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py@26
PS11, Line 26: cls
> Nit: This is a bound object method, so the first argument here is going to
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py@28
PS11, Line 28: cls
> self
Done


http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py@30
PS11, Line 30: cls
> self here and below as well
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 11: Code-Review+1

(1 comment)

BE looks fine.

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/frontend.cc@242
PS11, Line 242: Status Frontend::SetCatalogInitialized() {
  :   JNIEnv* jni_env = getJNIEnv();
  :   JniLocalFrame jni_frame;
  :   RETURN_IF_ERROR(jni_frame.push(jni_env));
  :   jni_env->CallObjectMethod(fe_, set_catalog_initialized_id_);
  :   RETURN_ERROR_IF_EXC(jni_env);
  :   return Status::OK();
  : }
is that needed? could it be handled entirely within InProcessImpalaServer? 
Maybe orthogonal to this change to so okay to defer.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 23 Oct 2017 19:01:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 11: Code-Review+1

(4 comments)

You may want to ask someone from BE and QE to sing off.

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

http://gerrit.cloudera.org:8080/#/c/8202/11/be/src/service/impala-server.cc@1979
PS11, Line 1979:   // Wait for the frontend catalog to be ready prior to 
opening client ports.
nit: Maybe expand this comment to mention that this call will block 
indefinitely in the case when the first catalog update never arrives for some 
reason.


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@82
PS11, Line 82: action="store_true",
 :   help=SUPPRESS_HELP)
nit: merge these two and save a line :)


http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@304
PS11, Line 304: wait_for_client
I think this should be named wait_for_catalog since that is the high level 
operation we're interested in.


http://gerrit.cloudera.org:8080/#/c/8202/11/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/8202/11/tests/common/custom_cluster_test_suite.py@127
PS11, Line 127: if disable_catalog:
  :   cmd.append("--disable_catalog")
nit: single line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 23 Oct 2017 18:43:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#11).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 168 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/11
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8202/9/be/src/testutil/in-process-servers.cc
File be/src/testutil/in-process-servers.cc:

http://gerrit.cloudera.org:8080/#/c/8202/9/be/src/testutil/in-process-servers.cc@75
PS9, Line 75: if (started.ok()) return impala;
: LOG(WARNING) << started.GetDetail();
:
: delete impala;
> The changes in this file seem to alter the behavior a bit. In particular, p
yes, there is a change here. L107 inits the server which waits on the catalog 
indefinitely with this patch. if the catalog is not set, then we'll just loop 
there which is not intended. so the updated behavior is stricter-- it requires 
the catalog to be ready prior to init. unclear what the prior behavior was if 
the catalog was not ready?

fwict, this class is used in tests and all tests passed with this change.


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@80
PS9, Line 80: parser.add_option("--disable_catalog", dest="disable_catalog", 
default=False,
:   action="store_true",
:   help="Starts all processes except catalogd.")
> This is used only for testing and using this doesn't result in a valid clus
suppress works and I think is fine here. are there uses for 
start-impala-cluster besides for testing/experimenting?


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@289
PS9, Line 289: impalad's catalog
> "catalog cache"
Done


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@304
PS9, Line 304: def wait_for_catalog(impalad, timeout_in_seconds):
 :   """Waits for the impalad catalog to become ready"""
 :   start_time = time()
 :   catalog_ready = False
 :   attempt = 0
 :   while (time() - start_time < timeout_in_seconds and not 
catalog_ready):
 : try:
 :   num_dbs = 
impalad.service.get_metric_value('catalog.num-databases')
 :   num_tbls = 
impalad.service.get_metric_value('catalog.num-tables')
 :   catalog_ready = 
impalad.service.get_metric_value('catalog.ready')
 :   if catalog_ready or attempt % 4 == 0:
 :   print 'Waiting for Catalog... Status: %s DBs / %s 
tables (ready=%s)' %\
 :   (num_dbs, num_tbls, catalog_ready)
 :   attempt += 1
 : except Exception, e:
 :   print e
 : sleep(0.5)
 :   if not catalog_ready:
 : raise RuntimeError('Catalog was not initialized in expected 
time period.')
 :
 : def wait_for_client(impalad, 
timeout_in_seconds=CLUSTER_WAIT_TIMEOUT_IN_SECONDS):
 :   """Waits for the client ports to become ready."""
 :   start_time = time()
 :   client_beeswax = None
 :   client_hs2 = None
 :   while (time() - start_time < timeout_in_seconds):
 : try:
 :   client_beeswax = impalad.service.create_beeswax_client()
 :   client_hs2 = impalad.service.create_hs2_client()
 :   break;
 : except Exception as e:
 :   print 'Client services not ready: %s. Trying again ...'
 : finally:
 :   if client_beeswax is not None: client_beeswax.close()
 : sleep(0.5)
 :
 :   if client_beeswax is None or client_hs2 is None:
 : raise RuntimeError('Client port not openned in expected time 
period.')
> Does it make sense to merge these two functions into a single wait_for_cata
good idea, changed.


http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java@860
PS9, Line 860: state
> update
Done


http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java@904
PS9, Line 904: impaladCatalog_.get()
> getCatalog()
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 9
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#10).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 169 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/10
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 10
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8202/9/be/src/testutil/in-process-servers.cc
File be/src/testutil/in-process-servers.cc:

http://gerrit.cloudera.org:8080/#/c/8202/9/be/src/testutil/in-process-servers.cc@75
PS9, Line 75: if (started.ok()) return impala;
: LOG(WARNING) << started.GetDetail();
:
: delete impala;
The changes in this file seem to alter the behavior a bit. In particular, 
previously even if SetCatalogInitialized would return an error, impala_server_ 
would still be initialized (see L107-108). With your change, if 
SetCatalogInitialized() throws an error, StartWithClientServers () will return 
with an error causing impala to be deleted. I don't recall how we use this 
in-process-server thing and your changes are most likely doing the right thing, 
but just wanted to point out the change in behavior.


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@80
PS9, Line 80: parser.add_option("--disable_catalog", dest="disable_catalog", 
default=False,
:   action="store_true",
:   help="Starts all processes except catalogd.")
This is used only for testing and using this doesn't result in a valid cluster 
configuration. So, I think it's best if we hide/remove this option. One option 
is to hide it using something like SUPPRESS_HELP/USAGE. Other option is to 
control this behavior using an env variable and not a startup option. Maybe the 
first one is not too bad. Thoughts?


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@289
PS9, Line 289: impalad's catalog
"catalog cache"


http://gerrit.cloudera.org:8080/#/c/8202/9/bin/start-impala-cluster.py@304
PS9, Line 304: def wait_for_catalog(impalad, timeout_in_seconds):
 :   """Waits for the impalad catalog to become ready"""
 :   start_time = time()
 :   catalog_ready = False
 :   attempt = 0
 :   while (time() - start_time < timeout_in_seconds and not 
catalog_ready):
 : try:
 :   num_dbs = 
impalad.service.get_metric_value('catalog.num-databases')
 :   num_tbls = 
impalad.service.get_metric_value('catalog.num-tables')
 :   catalog_ready = 
impalad.service.get_metric_value('catalog.ready')
 :   if catalog_ready or attempt % 4 == 0:
 :   print 'Waiting for Catalog... Status: %s DBs / %s 
tables (ready=%s)' %\
 :   (num_dbs, num_tbls, catalog_ready)
 :   attempt += 1
 : except Exception, e:
 :   print e
 : sleep(0.5)
 :   if not catalog_ready:
 : raise RuntimeError('Catalog was not initialized in expected 
time period.')
 :
 : def wait_for_client(impalad, 
timeout_in_seconds=CLUSTER_WAIT_TIMEOUT_IN_SECONDS):
 :   """Waits for the client ports to become ready."""
 :   start_time = time()
 :   client_beeswax = None
 :   client_hs2 = None
 :   while (time() - start_time < timeout_in_seconds):
 : try:
 :   client_beeswax = impalad.service.create_beeswax_client()
 :   client_hs2 = impalad.service.create_hs2_client()
 :   break;
 : except Exception as e:
 :   print 'Client services not ready: %s. Trying again ...'
 : finally:
 :   if client_beeswax is not None: client_beeswax.close()
 : sleep(0.5)
 :
 :   if client_beeswax is None or client_hs2 is None:
 : raise RuntimeError('Client port not openned in expected time 
period.')
Does it make sense to merge these two functions into a single wait_for_catalog 
function? wait_for_client() is only used for checking if the catalog is ready 
because we can't rely on the 'catalog.ready' metric, no? So, if this thing is 
not useful why not remove it and check for the client connections instead? And 
then we can retrieve the num_dbs and num_tbls from the metrics.


http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java@860
PS9, Line 860: state
update


http://gerrit.cloudera.org:8080/#/c/8202/9/fe/src/main/java/org/apache/impala/service/Frontend.java@904

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#9).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 166 insertions(+), 55 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/9
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 9
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Hello Philip Zeyliger, Balazs Jeszenszky, Dimitris Tsirogiannis, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8202

to look at the new patch set (#8).

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 160 insertions(+), 55 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8202/8
--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has abandoned this change. ( http://gerrit.cloudera.org:8080/8275 
)

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I19da0c751348accab271aea99b2162218e5ed1fb
Gerrit-Change-Number: 8275
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/8275 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6

ok

Change-Id: I19da0c751348accab271aea99b2162218e5ed1fb
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/testutil/in-process-servers.cc
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
13 files changed, 160 insertions(+), 55 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/8275/4
--
To view, visit http://gerrit.cloudera.org:8080/8275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I19da0c751348accab271aea99b2162218e5ed1fb
Gerrit-Change-Number: 8275
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac