[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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 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 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 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.
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 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.
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 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 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 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.
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.
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 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 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 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.
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 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 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 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 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.
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 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 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 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 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.
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 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 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.
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 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 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.
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 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 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.
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 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.
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 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 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.
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 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 Gerrit-Change-Nu
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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 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 Gerrit-Comment-Date: Tue, 24 Oct 2017 00:03:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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.
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 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 Gerrit-Comment-Date: Mon, 23 Oct 2017 21:21:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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 11: (9 comments) 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@258 PS11, Line 258: return not impala_cluster.catalogd and not options.disable_catalog Why not: return impala_cluster.catalogd or options.disable_catalog http://gerrit.cloudera.org:8080/#/c/8202/11/bin/start-impala-cluster.py@317 PS11, Line 317: ; remove stray semicolon 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? 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 seconds", for instance. 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()? 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 be the object, not the class. Use the name "self" here. http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py@28 PS11, Line 28: cls self 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 http://gerrit.cloudera.org:8080/#/c/8202/11/tests/custom_cluster/test_catalog_wait.py@45 PS11, Line 45: with pytest.raises(Exception) as e nice! -- 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 Gerrit-Comment-Date: Mon, 23 Oct 2017 19:10:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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 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.
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 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 Jeszenszky Gerrit-Reviewer: Dan H
[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
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 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.
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.
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 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 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