Henry Robinson has posted comments on this change. Change subject: IMPALA-4278: Don't abort Catalog startup quickly if HMS is not present ......................................................................
Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/4842/3/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS3, Line 41: spend on trying to establish connection to HMS the " : "first time on startup "wait to establish an initial connection to the HMS" PS3, Line 75: init_first init_first seems redundant, here and everywhere else. How about "initial_hms_cnxn_timeout_s" http://gerrit.cloudera.org:8080/#/c/4842/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: Line 67: protected final MetaStoreClientPool metaStoreClientPool_ = new MetaStoreClientPool(0, 0); long line PS3, Line 103: public void initMetaStoreClientPool(long initFirstClientTimeoutSeconds) { : metaStoreClientPool_.addClients(META_STORE_CLIENT_POOL_SIZE, : initFirstClientTimeoutSeconds); : } Why not call this in the c'tor any more? (if timeout is < 0, add no clients. Or maybe pass META_STORE_CLIENT_POOL_SIZE as a parameter to the c'tor). http://gerrit.cloudera.org:8080/#/c/4842/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: PS3, Line 163: super don't think you need super., do you? http://gerrit.cloudera.org:8080/#/c/4842/3/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java: PS3, Line 156: Add 'numClients' to the client pool. : * 'initFirstClientTimeoutSeconds' specifies the time (in seconds) spent on trying to : * initialize the first MetaStore client before giving up. : */ it might be clearer to call this, initially, as addClients(1, 120), then call addClients(9, 10) (or whatever the second timeout should be). The current API is a little confusing. http://gerrit.cloudera.org:8080/#/c/4842/3/tests/experiments/test_catalog_hms_failures.py File tests/experiments/test_catalog_hms_failures.py: PS3, Line 61: metadatra metadata PS3, Line 109: statestored.service.wait_for_live_subscriber if this fails, I think the test can leave the HMS not running. That will affect other tests. -- To view, visit http://gerrit.cloudera.org:8080/4842 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83c70f939429e1d0d20284a1307f3ee1278ae047 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-HasComments: Yes
