You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2024/03/01 07:58:08 UTC

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21096

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................

IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

When --enable_reload_events is set to true, catalogd will fire RELOAD
events for INVALIDATE/REFRESH statements. When the RELOAD event is fired
successfully for a REFRESH statement, we also update lastRefreshEventId
of the table/partition. This part could hit NullPointerException when
the partition is dropped by concurrent DDLs.

This patch ignores updating lastRefreshEventId if the partition doesn't
exists. Note that ideally we should hold the table lock of REFRESH until
finish firing the RELOAD events and updating lastRefreshEventId. So no
concurrent operations can drop the partition. However, when the table is
loaded from scratch, we don't actually hold the table write lock. We
just load the table and take a read lock to get the thrift object. The
partition could still be dropped concurrently after the load and before
taking the read lock. So ignoring missing partitions is a simpler
solution.

Refactors some codes of fireReloadEventAndUpdateRefreshEventId to save
some idention and avoid acquiring table lock if no events are fired.
Adds error messages in some Precondition checks in methods used by this
feature. Also refactors Table.getFullName() to not always constructing
the result.

Tests:
- Add e2e test

Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
---
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
5 files changed, 64 insertions(+), 26 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/21096/1
--
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10356/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 06 Mar 2024 12:01:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799
PS1, Line 6799: List<Long> eventId
> There are no events generated when we are reloading the table - we just fet
>There are no events generated when we are reloading the table
I understand that the events are generated only at this line.

My concern is about events generated during the refreshing of the table, e.g. adding a new partition while catalogd reloads file metadata (after getting the partition list from HMS). If these events are processed only after fireReloadEventAndUpdateRefreshEventId() (e.g. because the event processor is lagging), then they will be skipped as their event id is smaller then the reload event's.

There is no perfect way to handle modifications that happen in parallel in HMS (at least for external tables), but it seems safer to process events unnecessarily than to skip them incorrectly.



-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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: Sun, 03 Mar 2024 10:56:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15419/ : 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/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 06 Mar 2024 09:59:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799
PS1, Line 6799: List<Long> eventId
> Ah, sorry to misunderstood this. That's really a problem! So we shouldn't u
A good news is that this only happens when --enable_skipping_older_events is true and it's false by default:
https://github.com/apache/impala/blob/784971c018c2dc44c53d7c0f366ad49cd8681ac6/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L1199

Filed IMPALA-12865 to track this issue.



-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 04 Mar 2024 08:23:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15373/ : 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/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 08:24:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Hello k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................

IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Refactors some codes of fireReloadEventAndUpdateRefreshEventId to save
some indention and avoid acquiring table lock if no events are fired.
Adds error messages in some Precondition checks in methods used by this
feature. Also refactors Table.getFullName() to not always constructing
the result. Improves logs of not reloading a partition for an event.

Tests:
- Add e2e test

Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
---
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
6 files changed, 86 insertions(+), 46 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/21096/3
--
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.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-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21096/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21096/1//COMMIT_MSG@26
PS1, Line 26: indentio
> typo
Done


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

http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799
PS1, Line 6799: List<Long> eventId
> Not related to the current bug, but using the events ids generated here loo
There are no events generated when we are reloading the table - we just fetch metadata from HMS and the storage (e.g. HDFS). The RELOAD events are generated at this line by MetastoreShim.fireReloadEventHelper(). I think it's not too late to use them. However, it does have a chance that they arrive before we acquire the table lock at L6805. Ideally the operation that holding the table lock could also update the lastRefreshEventId so don't need to update it here. But we do have a risk to process the self RELOAD events. I'm not sure if we have better choice. We can have a follow-up JIRA to improve this.

For using the latest event id from HMS as lastRefreshEventId, we already do this when enable_sync_to_latest_event_on_ddls is true:
https://github.com/apache/impala/blob/7c53e87aa166bb77cd2e31646ff913302912d3fd/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L2665-L2668
https://github.com/apache/impala/blob/7c53e87aa166bb77cd2e31646ff913302912d3fd/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L2707
But the latest event id we fetched during reload is still smaller than the RELOAD event ids here since they are fired after the reload.



-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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: Sun, 03 Mar 2024 02:29:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15383/ : 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/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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: Sun, 03 Mar 2024 02:55:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Hello k.venureddy2103@gmail.com, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................

IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Tests:
- Add e2e test

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/21096/2
--
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 3: Code-Review+2

Upgrading to +2 as no one else commented.


-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 06 Mar 2024 11:58:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 06 Mar 2024 16:41:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21096/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21096/1//COMMIT_MSG@26
PS1, Line 26: idention
typo


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

http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799
PS1, Line 6799: List<Long> eventId
Not related to the current bug, but using the events ids generated here looks strange to me.

At this point we are after reloading the table - what will happen to events that arrived during the reload? Wouldn't it be better to get the last event id from HMS before reloading the table and use that one to skip events? + this mechanism would also work if enable_reload_events=false

For self event detection using these event ids is correct but they are also used to skip all events with lower event id:
https://github.com/apache/impala/blob/30fbcc94ea08bb0a5eb58d68bcce9c2a0eb9750e/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L1219



-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 01 Mar 2024 15:05:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21096/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6799
PS1, Line 6799: List<Long> eventId
> >There are no events generated when we are reloading the table
Ah, sorry to misunderstood this. That's really a problem! So we shouldn't update lastRefreshEventId using these RELOAD events. The motivation here is to ignore self RELOAD events. I think what we should do is using the same way as detecting self INSERT events - adding the RELOAD event ids to the inflight-events list.



-- 
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.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, 04 Mar 2024 02:35:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21096 )

Change subject: IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist
......................................................................

IMPALA-12855: Fix NPE in firing RELOAD events when the partition doesn't exist

Tests:
- Add e2e test

Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Reviewed-on: http://gerrit.cloudera.org:8080/21096
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
6 files changed, 86 insertions(+), 46 deletions(-)

Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/21096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I01af3624bf7cf5cd69935cffa28d54f6a6807504
Gerrit-Change-Number: 21096
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.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>