Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG@17 PS3, Line 17: If any of these are changed, the file metadata reloading : can be skipped. > What happens with the table property that was changed? While the table meta We will always reload table metadata along with the updated table properties. With this patch, we only intend to skip file metadata of the given table. http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136 PS2, Line 136: "to whitelist the table properties that are supposed t > Shouldn't we keep transactional and add transactional_properties? HiveMetaStore doesn't allow transactional and transactional_properties to be changed. So these properties are not really needed. The thing is we don't know what properties that change often. I think it is upto the admin to configure these values. Insert statement generates insert event which is already handled by the event processor. http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136 PS2, Line 136: "to whitelist the table properties that are supposed to refresh file metadata" : "when these properties are changed. To skip this optimization, set the > I didn't mean to use a query option here. Ack http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift File common/thrift/BackendGflags.thrift: http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift@257 PS3, Line 257: } > nit: extra line Ack http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3900 PS2, Line 3900: if (whitelisted > I see, agree with handling it like this. Ack http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3904 PS2, Line 3904: // but these are the most common types for alter statements. : if (isFieldSchemaChanged(beforeTable, afterTable) || : !beforeTable.getOwner().equals(afterTable.getOwner()) || : !isCustomTblPropsChanged(beforeTable, afterTable)) { : > So if one alter table event changes the field schema or the owner, then it So in one alter table query, either a table schema can be changed, or a table owner can be changed, or any table property (other whitelisted ones) can be changed, so if anyone of these changes, we can skip file metadata reloading. -- To view, visit http://gerrit.cloudera.org:8080/19838 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6 Gerrit-Change-Number: 19838 Gerrit-PatchSet: 4 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 23 May 2023 20:12:20 +0000 Gerrit-HasComments: Yes
