Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22816 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/22816/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22816/2//COMMIT_MSG@27 PS2, Line 27: postponding nit: postponing http://gerrit.cloudera.org:8080/#/c/22816/2//COMMIT_MSG@33 PS2, Line 33: A new flag, catalog_delete_log_gc_frequency, is added for this. The : deleteLog GC happens in every N+1 (N=catalog_delete_log_gc_frequency) : topic updates. How many seconds between GC does it translate to in default config? How does it impact sync_hms_events_wait_time_s selection (how long is too long/short)? http://gerrit.cloudera.org:8080/#/c/22816/2/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/22816/2/be/src/catalog/catalog-server.cc@177 PS2, Line 177: catalog_delete_log_gc_frequency Please add validator that this flag is always a positive number. Please do the same for topic_update_log_gc_frequency if there is none yet. http://gerrit.cloudera.org:8080/#/c/22816/2/fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java File fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java: http://gerrit.cloudera.org:8080/#/c/22816/2/fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java@137 PS2, Line 137: == nit: <= for safety. http://gerrit.cloudera.org:8080/#/c/22816/2/tests/metadata/test_event_processing.py File tests/metadata/test_event_processing.py: http://gerrit.cloudera.org:8080/#/c/22816/2/tests/metadata/test_event_processing.py@626 PS2, Line 626: client.execute("create database " + db) Assert that this is success? http://gerrit.cloudera.org:8080/#/c/22816/2/tests/metadata/test_event_processing.py@639 PS2, Line 639: client.execute("create table {}(i int)".format(tbl)) Assert that this is success? http://gerrit.cloudera.org:8080/#/c/22816/2/tests/metadata/test_event_processing.py@640 PS2, Line 640: self.execute_query("drop database if exists {} cascade".format(db)) : self.execute_query("drop database if exists {}_2 cascade".format(db)) Wrap L671 to L639 in try block. Then, put this in finally block to ensure test databases are always dropped in any case. -- To view, visit http://gerrit.cloudera.org:8080/22816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2441440bca2b928205dd514047ba742a5e8bf05e Gerrit-Change-Number: 22816 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 25 Apr 2025 03:37:26 +0000 Gerrit-HasComments: Yes
