Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/24166 )
Change subject: IMPALA-14684: Add metrics for catalogd table invalidations ...................................................................... Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/24166/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24166/7//COMMIT_MSG@17 PS7, Line 17: 10s > Double checked this, I realized metrics are captured by catalog-metrics-ref Great catch, increased the interval to 10s. http://gerrit.cloudera.org:8080/#/c/24166/5/common/thrift/metrics.json File common/thrift/metrics.json: http://gerrit.cloudera.org:8080/#/c/24166/5/common/thrift/metrics.json@670 PS5, Line 670: NONE > Sorry to be back and forth here. I found the value is rendered as something Yup, no worries, reverted it back to NONE and simplified the descriptions too. http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java File fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java: http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java@328 PS7, Line 328: batches.removeFirst(); > I don't think we need this additional lock since invalidateSome() and inval Done http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java@338 PS7, Line 338: long cutoffTime = TIME_SOURCE.read() - windowNanos; > This method has only one usage in recordInvalidationBatch(). It'd be better Done http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java@351 PS7, Line 351: */ > We can declare the method with "synchronized" to use the same object lock a Done http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/catalog/CatalogdTableInvalidator.java@354 PS7, Line 354: public final long ttlInvalidations1Min_; : public final long ttlInvalidations5Min_; : public final long ttlInvalidations30Min_; : public final long memoryPressureInvalidations10Sec_; : pub > Small optimization: start from the last batch and count forward till the on Done http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: http://gerrit.cloudera.org:8080/#/c/24166/7/fe/src/main/java/org/apache/impala/service/JniCatalog.java@661 PS7, Line 661: CatalogdTableInvalidator.InvalidationMetrics metrics = : invalidator.getMetrics(); : response.setCatalog_ttl_invalidations_10s(metrics.ttlInvalidations10Sec_); : response.setCatalog_ttl_invalidations_1m(metrics.ttlInvalidations1Min_); : response.setCatalog_ttl_invalidations_5m(metrics.ttlInvalidations5Min_); : response.setCatalog_ttl_invalidations_30m(metrics.ttlInvalidations30Min_); : response.setCatalog_memory_pressure_invalidations_10s( : metrics.memoryPressureInvalidations10Sec_); : response.setCatalog_memory_pressure_invalidations_1m( : metrics.memoryPressureInvalidations1Min_); : response.setCatalog_memory_pressure_invalidations_5m( : metrics.memoryPressureInvalidations5Min_); : response.setCatalog_memory_pressure_invalidations_30m( : metrics.memoryPressureInvalidations30Min_); : response.setCatalog_last_ttl_invalidation_ms( : metrics.lastTtlInvalidationMillis_); : response.setCatalog_last_ttl_invalidated_tables( : metrics.lastTtlInvalidatedTables_); : response.setCatalog_last_memory_pressure_invalidation_ms( : metrics.lastMemoryPressureInvalidationMillis_); : response.setCatalog_last_memory_pressure_invalidated_tables( : metrics.lastMemoryPressureInvalidatedTables_); : } : > These all acquires the lock individually, which is a waste and could see in Done http://gerrit.cloudera.org:8080/#/c/24166/7/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/24166/7/tests/custom_cluster/test_automatic_invalidation.py@47 PS7, Line 47: metrics_test_timeout_flag = ("--invalidate_tables_timeout_s=" + str(metrics_test_ttl_s)) > The test takes 150s. Can we use a shorter TTL, e.g. 1s? Done. Its now taking ~14s. -- To view, visit http://gerrit.cloudera.org:8080/24166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If78e6c8429801e3c1829860666f04ad287b826ab Gerrit-Change-Number: 24166 Gerrit-PatchSet: 8 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Wed, 22 Jul 2026 09:28:37 +0000 Gerrit-HasComments: Yes
