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 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/21143/6/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/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2625 PS6, Line 2625: values.get(i)); > hmm, how can we make sure all the IllegalStateException are due to HMS erro getPartitionExprFromValue() will not throw any exception currently. But I'll go with your suggestion for now since it makes more sense to me. http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2926 PS6, Line 2926: // refetched > If we use "continue" here, how do we throw an exception? Ideally, we don't want to throw an exception here. Consider the scenario where we get an empty partitions list from the event, we have two options 1) we just want to ignore the event without putting the event processor into an error state. Also, I had to wrap the getTypeCompatiblePartValues() with try/catch to catch the IllegalStateException Since I introduced that getTypeCompatiblePartValues() can throw IllegalStateException. 2) Or the other option at our disposal is to remove this "if" condition block and let getTypeCompatiblePartValues() throw an exception and let IMPALA-12832 take care of invalidating the table. But the (1) options doesn't invalidate the table, it just ignores the event. But (2) invalidates the table (which can be costly to load the table especially if it wide table or have hug partition count). I would prefer option (1). That's why I'm keeping if block and try/catch condition. Let me know your thoughts on this one. http://gerrit.cloudera.org:8080/#/c/21143/6/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/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@211 PS6, Line 211: foundEmptyPartitionVals = true; > we can add "break" here Done http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@215 PS6, Line 215: > nit: "values" ? Done -- 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: 7 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: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 02 Apr 2024 00:33:52 +0000 Gerrit-HasComments: Yes
