Attila Jeges has posted comments on this change. Change subject: IMPALA-4278: Don't abort Catalog startup quickly if HMS is not present ......................................................................
Patch Set 4: (9 comments) http://gerrit.cloudera.org:8080/#/c/4842/4/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: PS4, Line 105: if (numClients > 0) { : metaStoreClientPool_.addClients(1, initialCnxnTimeoutSec); : metaStoreClientPool_.addClients(numClients - 1, 0); : } > I think you can make this logic a method of MetaStoreClientPool - it's alre Done http://gerrit.cloudera.org:8080/#/c/4842/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: Line 110: private static final int META_STORE_CLIENT_POOL_SIZE = 10; > is this the max size, or the initial size, or both? Initial pool size. Changed the name of the field to reflect this. http://gerrit.cloudera.org:8080/#/c/4842/4/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java: PS4, Line 76: private > Add a comment to this method describing the behaviour. Done PS4, Line 83: IMetaStoreClient hiveClient = null; > What's the point of this variable - why not assign to hiveClient_ directly? hiveClient_ is final, the compiler complains if I try to set it in a loop. PS4, Line 99: catch (InterruptedException ignore) {} > on this path you might end up not sleeping for long enough if you get inter Done PS4, Line 161: initialCnxnTimeoutSec > this is not the 'initial' timeout in the same sense that it is elsewhere, I Done http://gerrit.cloudera.org:8080/#/c/4842/4/tests/experiments/test_catalog_hms_failures.py File tests/experiments/test_catalog_hms_failures.py: PS4, Line 81: 10 > just from experience, suggest you make this 30s. Timeouts are surprisingly Done Line 114: > Perhaps wait 5s or so here to be sure that the catalog is in the 'trying to Done Line 122 > How about a test that confirms that if the HMS does not start within initia Done -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-HasComments: Yes
