Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 )
Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/21143/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/21143/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2846 PS1, Line 2846: public int reloadPartitionsFromNames(long eventId, IMetaStoreClient client, > Shouldn't the 'reason' already include the event type? Reason: RELOAD event, Received partition with empty values: Partition(values:[]...). Ignoring reloading the partition. Reason only has eventType, so I'll have to introduce eventId as an argument. http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/main/java/org/apache/impala/util/DebugUtils.java File fe/src/main/java/org/apache/impala/util/DebugUtils.java: http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/main/java/org/apache/impala/util/DebugUtils.java@77 PS2, Line 77: // debug action label for mock that metastore returns partitions with empty values > Can you add a comment about the Jira to explain why this error injection is Done http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java: http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@212 PS2, Line 212: // This action is required for repro in the unit test (MetastoreEventsProcessorTest) > Can you add a comment about the Jira to explain why this error injection is Done http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3757 PS2, Line 3757: PartitionValues() is a r > Can you add a comment about being the regression test for the Jira? Done http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3837 PS2, Line 3837: long txnId = MetastoreShim.openTransaction(client.getHiveClient()); > Is this needed here? Doesn't have an affect only when Impala processes the Not really. To avoid the repetition of setting this flag before processEvents(), I have set it here. I'm moving this line out of this method in the next patch set. http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3847 PS2, Line 3847: } > It doesn't seem intuitive to me that we reset this here. I just want to set this config back to normal, I don't want other events to be affected while I'm testing a particular event. The test passes without setting to the previous value. -- To view, visit http://gerrit.cloudera.org:8080/21143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 Gerrit-Change-Number: 21143 Gerrit-PatchSet: 3 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Anonymous Coward <[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: Thu, 14 Mar 2024 21:13:01 +0000 Gerrit-HasComments: Yes
