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 <[email protected]> Gerrit-Reviewer: Balazs Jeszenszky <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 20 Oct 2017 22:57:03 +0000 Gerrit-HasComments: Yes
