You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org> on 2024/03/13 05:00:11 UTC

[Impala-ASF-CR] [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Sai Hemanth Gantasala has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21143


Change subject: [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

[WIP] IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a hidden boolean config is_return_empty_partition_values to
add malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
7 files changed, 60 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/1
-- 
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: newchange
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15612/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:40:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15752/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 00:57:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2024 00:33:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang 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 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/5/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/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2883
PS5, Line 2883:         hmsPartitions = MetaStoreUtil.fetchPartitionsByName(client, partNames, msTable_);
What about doing the check and retry inside MetaStoreUtil.fetchPartitionsByName()? It fetches partitions in batch and just needs to retry the problematic batch.



-- 
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: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2024 08:31:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang 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 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/4/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/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2869
PS4, Line 2869:           continue;
> Fair point. Currently, there is no reliable way of fetching the partition i
The possibility of hitting this issue twice continuely should be really small. But to be simpler, maybe we can just throw a meaningful exception instead of the current IllegalStateException, and rely on IMPALA-12832 to recover.



-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2024 06:52:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 )

Change subject: [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................


Patch Set 1:

(6 comments)

LGTM, just have some minor comments to make this a FE-only change.

http://gerrit.cloudera.org:8080/#/c/21143/1/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/21143/1/be/src/catalog/catalog-server.cc@192
PS1, Line 192: DEFINE_bool_hidden(is_return_empty_partition_values, false, "This configuration is used "
We don't need this for FE tests. We can use the debug_actions flag and add an action appropriately.


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(IMetaStoreClient client,
nit: it seems this is only used by the event-processor. We can pass in the eventId and eventType to improve the logging if that's not too hard.


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2858
PS1, Line 2858: warn
nit: error() seems more suitable. This can also be simplified as

          LOG.error("Received partition with empty values: {}. \nIgnoring" +
                  " reloading the partition.", partition));


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2891
PS1, Line 2891:   public int reloadPartitionsFromEvent(IMetaStoreClient client,
nit: we can pass in the eventId and eventType to improve the logging


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java@461
PS1, Line 461:   public String debugActions() { return backendCfg_.debug_actions; }
We can add setDebugAction() for the new FE test.


http://gerrit.cloudera.org:8080/#/c/21143/1/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/1/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@212
PS1, Line 212:     if (BackendConfig.INSTANCE.getIsReturnEmptyPartitionValues()) {
and use the debug action here like DebugUtils.hasDebugAction(BackendConfig.INSTANCE.debugActions(), DebugUtils.MOCK_EMPTY_PARTITION_VALUES)



-- 
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: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Mar 2024 12:44:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/4/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/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2869
PS4, Line 2869:           continue;
> The only concern is the table metadata would be stale after this, i.e. the 
Fair point. Currently, there is no reliable way of fetching the partition info accurately (especially when there are drop and add partitions repeatedly). So we might end up in an infinite loop theoretically.



-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 23:50:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Kurt Deschler, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#6).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 192 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/6
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang 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 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-12856: Event processor should ignore processing partition
> IMO, we can use this patch to address the empty partition values from HMS a
Thank Venu for digging into the HMS bug! Do we have a HIVE JIRA for this? I think in the Impala side, we need some rework to not trust all responses from external systems, or add some retry if hitting such issues twice is pretty rare.


http://gerrit.cloudera.org:8080/#/c/21143/4/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/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2869
PS4, Line 2869:           continue;
The only concern is the table metadata would be stale after this, i.e. the partition is not updated in the catalog cache. It might miss some files or still point to deleted files.

Without this change, the event-processor is able to recover from the IllegalStateException and invalidate the table to avoid stale metadata. As Venu pointed out that the cause is a race issue in HMS, do you think we can retry fetching the partition to better recover from here?



-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 23:43:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15506/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 01:19:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Kurt Deschler, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#5).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 186 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/5
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler 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 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/4/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/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2869
PS4, Line 2869:           continue;
> The possibility of hitting this issue twice continuely should be really sma
I agree with Quanlong here but the check should be in a separate loop above so reloads are not wasted if the empty partition is late in the list.



-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2024 11:59:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#2).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 128 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/2
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 )

Change subject: [WIP] IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15496/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Mar 2024 05:23:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com 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:

Sorry for the confusion. Ignore the first comment in it.


-- 
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 18:17:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#3).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 139 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/3
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer 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 2:

(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, String eventType,
> Ack
Shouldn't the 'reason' already include the event type?

I think that ideally 'reason' would also contain the eventId, so no additional argument would have to passed just for the sake of logging, but this seems like a larger change.


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 added?


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:     if (DebugUtils.hasDebugAction(BackendConfig.INSTANCE.debugActions(),
Can you add a comment about the Jira to explain why this error injection is added?


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: testEmptyPartitionValues
Can you add a comment about being the regression test for the Jira?


http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3837
PS2, Line 3837:       BackendConfig.INSTANCE.setDebugActions(DebugUtils.MOCK_EMPTY_PARTITION_VALUES);
Is this needed here? Doesn't have an affect only when Impala processes the event or reloads the table?
It also doesn't seem intuitive that this function has the side effect of changing the backed flags.


http://gerrit.cloudera.org:8080/#/c/21143/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3847
PS2, Line 3847:     BackendConfig.INSTANCE.setDebugActions(prevFlag);
It doesn't seem intuitive to me that we reset this here.



-- 
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: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 11:10:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15659/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2024 19:39:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang 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 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG@13
PS5, Line 13: partitions with empty values, EP should ignore such partitions instead
> HMS will only return metadata info like schema, table location but not the 
I think other fields could also be incomplete, like partition params, storage descriptor params, serde params, sort cols, bucket cols, skewed cols etc mentioned in HIVE-28145.


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:     } catch (IllegalStateException ex) {
hmm, how can we make sure all the IllegalStateException are due to HMS errors here? I think we should just replace the check at L2614 with throwing an exception. E.g.

  if (partitionColumns.size() != values.size()) {
    throw new InvalidObjectException(String.format("Unmatched numbers of partition values: expected=%d, actual=%d", partitionColumns.size(), values.size()));
  }

Also add the table name in the error message.


http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2926
PS6, Line 2926:         continue;
If we use "continue" here, how do we throw an exception?


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


http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@215
PS6, Line 215: partitions
nit: "values" ?



-- 
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: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2024 01:15:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG@13
PS5, Line 13: partitions with empty values, EP should ignore such partitions instead
> Can the HMS bug also return non-empty partitions that are incomplete (missi
HMS will only return metadata info like schema, table location but not the actual data.


http://gerrit.cloudera.org:8080/#/c/21143/5/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/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2883
PS5, Line 2883:         }
> What about doing the check and retry inside MetaStoreUtil.fetchPartitionsBy
Addressed this comment in the latest patch.



-- 
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: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2024 22:26:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com 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:

(2 comments)

An input from my side.

http://gerrit.cloudera.org:8080/#/c/21143/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-12856: Event processor should ignore processing partition
Not related to this patch. Currently ReloadEvent for partition doesn't use the field reloadPartition_ obtained from reload message. Instead, we reload the partitions using part names(uses getPartitionsByNames HMS API). getPartitionsByNames API(direct sql) is making multiple queries to db internally to build partition objects(query part ids from partition names, join partitions, sds, serdes tables for those part ids and forms partition objects, then query partition_key_vals table to get the partition values for those part ids and populate in already formed partition objects). May be, partition is deleted just before partition_key_vals table query and that resulted in values being emtpy in partition? I am not certain yet. I am trying to simulate it.

If we had made the use of reloadPartition_ received in ReloadEvent, probably we might have hit this issue? 

https://github.com/apache/impala/blob/691604b1d1f0e5f0dc95fdb4976cf826135e08fb/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L3004C13-L3004C29


http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-12856: Event processor should ignore processing partition
Have checked org.apache.hadoop.hive.metastore.MetaStoreDirectSql#getPartitionsViaPartNames() HMS method. It is invoked when getPartitionsByNames API is called.
It internally executes multiple queries to backend db to make the partition objects. First query part ids from partition names, then a query to join partitions, sds, serdes tables for those part ids and create partition objects, and then query partition_key_vals table to get the partition values for those part ids and populate it in already created partition objects.  
Actually many other queries are executed to populate the other fields in partition objects(like partition params, storage descriptor, storage descriptor params, serde, serde params sort cols, bucket cols, skewed cols etc).

If the partition is deleted in another transaction just before the partition_key_vals table query step in get partitions by names call above, it can lead to return empty values in the partition object. Same issue can happen for other fields too. Like partition params, storage descriptor param, serde params sort cols, bucket cols, skewed cols can be empty too.

I could reproduce it with a hive unittest having 2 threads(2 clients). One thread doing getPartitionsByNames and other thread doing dropPartition.



-- 
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2024 18:13:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 21:13:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-12856: Event processor should ignore processing partition
> Have checked org.apache.hadoop.hive.metastore.MetaStoreDirectSql#getPartiti
IMO, we can use this patch to address the empty partition values from HMS as we have seen this in production env. Other discovered cases should be handled in metastore as handling them in the catalog's cache becomes complex.



-- 
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 00:11:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler 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 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG@13
PS5, Line 13: partitions with empty values, EP should ignore such partitions instead
Can the HMS bug also return non-empty partitions that are incomplete (missing files)?



-- 
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: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2024 12:50:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15682/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2024 22:47:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15522/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 21:36:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/21143/1/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/21143/1/be/src/catalog/catalog-server.cc@192
PS1, Line 192: DECLARE_string(state_store_host);
> We don't need this for FE tests. We can use the debug_actions flag and add 
Ack


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, String eventType,
> nit: it seems this is only used by the event-processor. We can pass in the 
Ack


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2858
PS1, Line 2858: erro
> nit: error() seems more suitable. This can also be simplified as
Ack


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2891
PS1, Line 2891:    */
> nit: we can pass in the eventId and eventType to improve the logging
Only eventId is available, so I'm passing that for now.


http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/21143/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java@461
PS1, Line 461:   public String debugActions() { return backendCfg_.debug_actions; }
> We can add setDebugAction() for the new FE test.
Ack


http://gerrit.cloudera.org:8080/#/c/21143/1/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/1/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@212
PS1, Line 212:     if (DebugUtils.hasDebugAction(BackendConfig.INSTANCE.debugActions(),
> and use the debug action here like DebugUtils.hasDebugAction(BackendConfig.
Ack



-- 
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: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Mar 2024 00:55:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
k.venureddy2103@gmail.com 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 4:

(1 comment)

Have created HIVE-28145 for hive.

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-12856: Event processor should ignore processing partition
> Thank Venu for digging into the HMS bug! Do we have a HIVE JIRA for this? I
Have created a hive jira(HIVE-28145)



-- 
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: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 10:37:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#4).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 149 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/4
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Kurt Deschler, k.venureddy2103@gmail.com, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21143

to look at the new patch set (#7).

Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values
......................................................................

IMPALA-12856: Event processor should ignore processing partition
with empty partition values

While processing partition related events, Event Processor (EP) is
facing IllegalStateException if the partition fetched from HMS has
empty partition values. Even though this is a bug in HMS which returns
partitions with empty values, EP should ignore such partitions instead
of throwing IllegalStateException.

Note: Added a debug option 'mock_empty_partition_values' to add
malformed partition objects.

Testing:
- Manually verified the test provided in jira details in local env.
- Added unit test to return empty partition values and verify EP state.

Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
6 files changed, 198 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/21143/7
-- 
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: newpatchset
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values

Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
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 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21143/4/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/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2869
PS4, Line 2869:         getFullName(), generateDebugStr(partNames, 3), reason));
> I agree with Quanlong here but the check should be in a separate loop above
Ack



-- 
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: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2024 19:16:41 +0000
Gerrit-HasComments: Yes