impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vuk Ercegovac (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.
Date Fri, 20 Oct 2017 22:57:03 GMT
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 <vercegovac@cloudera.com>
Gerrit-Reviewer: Balazs Jeszenszky <jeszyb@gmail.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercegovac@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Oct 2017 22:57:03 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message