Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21655 )
Change subject: IMPALA-13293: Fix too long wait for initial catalog update ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21655/3/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java File fe/src/main/java/org/apache/impala/catalog/FeCatalog.java: http://gerrit.cloudera.org:8080/#/c/21655/3/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java@103 PS3, Line 103: void waitForCatalogUpdate(long timeoutMs); > Can we add a comment saying this is only used in startup? If it's used afte It is actually used after startup at a single call site: https://github.com/apache/impala/blob/d8a8412c2b750937a3577b08d81ffd9a16269b83/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java#L240 I tried to not change the behavior in that case - LocalCatalog will return immediately as it is already ready, while ImpaladCatalog will wait for the next catalog update to notify it. If a full catalog update is received after startup, I think the same speedup will apply as during startup. I think that it is confusing at this point, as in ImpaladCatalog this is really meant to wait for an update, while in LocalCatalog it is NOOP unless it is the initial startup (or probably it will also wait after INVALIDATE METADATA or catalog service id change?). I was thinking about refactoring it, but it seemed better to wait for non-local catalog support to be dropped before doing it (probably the original authors thought the same :) ) http://gerrit.cloudera.org:8080/#/c/21655/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/21655/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@425 PS3, Line 425: } > Can we add a Preconditions check to make sure isReady() is true at the end? If the timeout is reached, then it may not be ready at this point. Both places that call waitForCatalogUpdate() do it in a loop and will wait again for 2s for the catalog to be ready. -- To view, visit http://gerrit.cloudera.org:8080/21655 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4da1d0c0d25f80b02c39be863431201d1b913c5d Gerrit-Change-Number: 21655 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 13 Aug 2024 13:40:09 +0000 Gerrit-HasComments: Yes
