Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23804 )
Change subject: IMPALA-13863: Add metric to track number of loaded tables in catalogd ...................................................................... Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/23804/6/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/23804/6/tests/custom_cluster/test_automatic_invalidation.py@116 PS6, Line 116: e_query("invalidat > Done, but you taught this in the slide: https://docs.google.com/presentatio Sorry that if it misleads you. The slide is talking about metadata loading. INVALIDATE evicts the loaded table immediately (synchronously) but doesn't reload the metadata of the table. The reloading happens at the next time when the table is accessed. So comparing to REFRESH which reloads the metadata immediately and keeps the table loaded, INVALIDATE is async in the aspect of metadata loading. Here we are checking whether the table is evicted, i.e. replaced with IncompleteTable. So the effect is synchronous. BTW, please don't use internal links in public Gerrit/JIRA. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@117 PS7, Line 117: timeout=10 nit: Don't need this since the default of timeout is 10. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@132 PS7, Line 132: count_after_refresh = catalogd.get_metric_value(metric_name) Here we get the counter without waiting. If the counter changes after 1s (REFRESH_METRICS_INTERVAL_MS), the test still passes. It'd make more sense to sleep 1s here before getting the counter. Please also add a comment that sleep for 1s (REFRESH_METRICS_INTERVAL_MS) is to make sure the metrics is updated. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@146 PS7, Line 146: count_before_rename - 1 nit: To be consistent with other code, let's use constant value (e.g. "1" here) and other similar places. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@147 PS7, Line 147: count_after_rename = catalogd.get_metric_value(metric_name) : # RENAME removes the old loaded table (decrement) and adds : # new IncompleteTable (no change) : assert count_after_rename == count_before_rename - 1, ( : "Count should decrease by 1 after RENAME (was %d, now %d)" : % (count_before_rename, count_after_rename)) nit: Don't need these since the above wait_for_metric_value() already checks it. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@167 PS7, Line 167: count_after_create = catalogd.get_metric_value(metric_name) Also need a sleep before this. http://gerrit.cloudera.org:8080/#/c/23804/7/tests/custom_cluster/test_automatic_invalidation.py@189 PS7, Line 189: test_db = ImpalaTestSuite.get_random_name("test_db_") : self.execute_query("create database if not exists %s" % test_db) : self.execute_query("create table %s.t1 (id int, name string)" % test_db) : self.execute_query("insert into %s.t1 values (1, 'test')" % test_db) : self.execute_query("select * from %s.t1" % test_db) : catalogd.wait_for_metric_value(metric_name, 1, timeout=10) : : self.execute_query("drop database %s cascade" % test_db) : catalogd.wait_for_metric_value(metric_name, 0, timeout=10) Wrap the DROP DATABASE statement in the finally clause so it will be dropped when the assertion fails. test_db = ImpalaTestSuite.get_random_name("test_db_") try: self.execute_query("create database if not exists %s" % test_db) self.execute_query("create table %s.t1 (id int, name string)" % test_db) self.execute_query("insert into %s.t1 values (1, 'test')" % test_db) self.execute_query("select * from %s.t1" % test_db) catalogd.wait_for_metric_value(metric_name, 1, timeout=10) finally: self.execute_query("drop database %s cascade" % test_db) catalogd.wait_for_metric_value(metric_name, 0, timeout=10) -- To view, visit http://gerrit.cloudera.org:8080/23804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5aa54f9f7507709b654df22e24592799811e8b6c Gerrit-Change-Number: 23804 Gerrit-PatchSet: 7 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 30 Dec 2025 06:48:06 +0000 Gerrit-HasComments: Yes
