Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16159 )
Change subject: IMPALA-3127: Support incremental metadata updates in partition level ...................................................................... Patch Set 2: (6 comments) Thanks for the review! Addressed the comments. http://gerrit.cloudera.org:8080/#/c/16159/1/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/16159/1/common/thrift/CatalogObjects.thrift@424 PS1, Line 424: > nit, partition Removed this field http://gerrit.cloudera.org:8080/#/c/16159/1/common/thrift/CatalogObjects.thrift@425 PS1, Line 425: // Each TNetworkAddress is a datanode which contains blocks of a file in the table. : // Used so that each THdfsFileBlock can just reference an index in this list rather : // than duplicate the list of network address, w > Is there any value of having a new field? Seems like this list is always de Done. Merged the list into the partition map and introduce some flags to reveal the state. http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/Catalog.java@632 PS1, Line 632: ":" > I feel having space in the catalogObjectKey is bit unconventional and can c Sure. Will change to ":" http://gerrit.cloudera.org:8080/#/c/16159/1/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/16159/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@710 PS1, Line 710: byte[] data = serializer.serialize(minimalObject); : String v2Key = CatalogServiceConstants.CATALOG_TOPIC_V2_PREFIX + key; : > You may want to consider sending the updates for partitions as well since t Yeah, good point! We can send invalidation on the old (replaced) partition id. http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1614 PS1, Line 1614: ptor tableDesc = new TTab > perhaps a better name could be toThriftWithMinimalPartitions since we are a Done http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java: http://gerrit.cloudera.org:8080/#/c/16159/1/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@469 PS1, Line 469: pdates. : if (newTable instanceof HdfsTable > I think it would be more readable if we move this to a method called isFrom Done. Merge the list into the map. -- To view, visit http://gerrit.cloudera.org:8080/16159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0abfb346903d6e7cdc603af91c2b8937d24d870 Gerrit-Change-Number: 16159 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Anurag Mantripragada <anu...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Thu, 16 Jul 2020 03:34:12 +0000 Gerrit-HasComments: Yes
