Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21402 )

Change subject: IMPALA-12712: Invalidate metadata on table should set better 
createEventId
......................................................................


Patch Set 1: Code-Review+1

(5 comments)

looks like a major improvement to avoid these kind of issue, some minor comments

http://gerrit.cloudera.org:8080/#/c/21402/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21402/1//COMMIT_MSG@13
PS1, Line 13: always removing the table. Sequence of drop table + create table +
when is the table removed? in case of drop table event?


http://gerrit.cloudera.org:8080/#/c/21402/1/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/21402/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6999
PS1, Line 6999:       try (MetaStoreClient msClient = 
catalog_.getMetaStoreClient(catalogTimeline)) {
This is a huge improvement compared to the original state, but there is still a 
gap between getting the last event id and fetching the the table, so 
theoretically it could be dropped+recreated between the two.
I don't mean to change anything now, but is my understanding correct that 
ideally HMS would return the creation event id in get_table_meta and similar 
functions to be able to identify which incarnation of the table is returned?


http://gerrit.cloudera.org:8080/#/c/21402/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7044
PS1, Line 7044:               } catch (IcebergTableLoadingException e) {
              :                 updatedThriftTable = 
catalog_.invalidateTable(req.getTable_name(),
              :                     tblWasRemoved, dbWasAdded, catalogTimeline, 
eventId);
This is not really related to the current change, but this Iceberg specific 
handling looks strange to me


http://gerrit.cloudera.org:8080/#/c/21402/1/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/21402/1/tests/custom_cluster/test_events_custom_configs.py@1271
PS1, Line 1271:     self.client.execute("drop table 
{}.{}".format(unique_database, test_tbl))
              :     self.run_stmt_in_hive(
About doing these in Hive vs Impala: the issue would also come if the drop 
table would be done, in Hive, right? Could it also occur if create table was 
done in Impala?


http://gerrit.cloudera.org:8080/#/c/21402/1/tests/custom_cluster/test_events_custom_configs.py@1273
PS1, Line 1273:       "create table {}.{} (i int, j 
int)".format(unique_database, test_tbl))
nit: +2 indentation



--
To view, visit http://gerrit.cloudera.org:8080/21402
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff6ac18fe8d9e7b25cc41c7e41eecde251fbccdd
Gerrit-Change-Number: 21402
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 22 May 2024 14:30:46 +0000
Gerrit-HasComments: Yes

Reply via email to