Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 )
Change subject: IMPALA-12543: Detect self-events before finishing DDL ...................................................................... Patch Set 21: (7 comments) http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1101 PS20, Line 1101: /** : * Cancel the new version number registration out of table's in-flight events if > It's possible that registerInflightEvent() fails to add the version due to That make sense. Removed this precondition. Also make registerInflightEvent re-entrant. Forgot about the rename, will do it in next patch set. http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1645 PS20, Line 1645: > Isn't this done at L1623? Good catch, thanks! http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3373 PS20, Line 3373: > Could you add a TODO that we need to revisit this? I think if the table is Done http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3472 PS20, Line 3472: catalog_.getLock().writeLock().unlock(); : modification.addCatalogServiceIdentifiersTo > These could already generate events. Could you add a TODO to revisit this? Done. Add call to registerInflightEvent() here as well. http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3509 PS20, Line 3509: boolean isTableBeingReplicated = false; : Stopwatch sw = Stopwatch.createStarted(); : try { : // if the table is being replicated we issue the HMS API to truncate the table : // si > I think L3516 will always generate HMS events so we should move this to L35 I think you mean 'isTableBeingReplicated' is True? ps21 call registerInflightEvent() in two place: L3518 and L3542. Please let me know if this is correct. http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py@43 PS20, Line 43: TestEventProcessingCust > nit: need another name since there is one with the same name in tests/metad Renamed to TestEventProcessingCustomConfigsBase. http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py@52 PS20, Line 52: the queries are run from Impal > nit: "if the queries are run from Impala" Done -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 21 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 09 Apr 2024 05:43:33 +0000 Gerrit-HasComments: Yes
