subject:"\[Impala\-ASF\-CR\] IMPALA\-12543\: Detect self\-events before finishing DDL"

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 22:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/20/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/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1101
PS20, Line 1101: /**
   :  * Cancel the new version number registration out of 
table's in-flight events if
> That make sense. Removed this precondition.
Renamed to cancelInflightEventIfExist in ps22.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 22
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 09 Apr 2024 05:48:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS using helper class
InProgressTableModification. If HMS update does not complete (ie., an
exception is thrown), the new newCatalogVersion that was added is then
removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 890 insertions(+), 591 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/22
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 22
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 21:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/21029/20/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/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1101
PS20, Line 1101: /**
   :  * Cancel the new version number registration out of 
table's in-flight events if
> It's possible that registerInflightEvent() fails to add the version due to
That make sense. Removed this precondition.
Also make registerInflightEvent re-entrant.
Forgot about the rename, will do it in next patch set.


http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1645
PS20, Line 1645:
> Isn't this done at L1623?
Good catch, thanks!


http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3373
PS20, Line 3373:
> Could you add a TODO that we need to revisit this? I think if the table is
Done


http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3472
PS20, Line 3472: catalog_.getLock().writeLock().unlock();
   : modification.addCatalogServiceIdentifiersTo
> These could already generate events. Could you add a TODO to revisit this?
Done. Add call to registerInflightEvent() here as well.


http://gerrit.cloudera.org:8080/#/c/21029/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3509
PS20, Line 3509: boolean isTableBeingReplicated = false;
   : Stopwatch sw = Stopwatch.createStarted();
   : try {
   :   // if the table is being replicated we issue the HMS API 
to truncate the table
   :   // si
> I think L3516 will always generate HMS events so we should move this to L35
I think you mean 'isTableBeingReplicated' is True? ps21 call 
registerInflightEvent() in two place: L3518 and L3542.
Please let me know if this is correct.


http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py@43
PS20, Line 43: TestEventProcessingCust
> nit: need another name since there is one with the same name in tests/metad
Renamed to TestEventProcessingCustomConfigsBase.


http://gerrit.cloudera.org:8080/#/c/21029/20/tests/custom_cluster/test_events_custom_configs.py@52
PS20, Line 52: the queries are run from Impal
> nit: "if the queries are run from Impala"
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 21
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 09 Apr 2024 05:43:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS using helper class
InProgressTableModification. If HMS update does not complete (ie., an
exception is thrown), the new newCatalogVersion that was added is then
removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 890 insertions(+), 591 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/21
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 21
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-06 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 20:

> Patch Set 20:
>
> (1 comment)
>
> ps20 is a bigger refactoring where InProgressTableModification is being used 
> in more places. I will rerun exhaustive tests overnight.

ps20 pass exhaustive tests here: 
https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/2253/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 20
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Sat, 06 Apr 2024 19:03:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-05 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 20:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 20
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Fri, 05 Apr 2024 23:17:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 20:

(1 comment)

ps20 is a bigger refactoring where InProgressTableModification is being used in 
more places. I will rerun exhaustive tests overnight.

http://gerrit.cloudera.org:8080/#/c/21029/19/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/21029/19/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2671
PS19, Line 2671: y {
> nit: It might be worth to change this and 2 other overload of this method i
Moved it inside InProgressTableModification class.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 20
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Fri, 05 Apr 2024 22:55:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS using helper class
InProgressTableModification. If HMS update does not complete (ie., an
exception is thrown), the new newCatalogVersion that was added is then
removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 886 insertions(+), 591 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/20
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 20
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-05 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 19:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 19
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Fri, 05 Apr 2024 18:24:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 19:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21029/18//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21029/18//COMMIT_MSG@51
PS18, Line 51: - Pass exhaustive tests.
> Rerun exhaustive tests overnight and it is now failing at some tests. I'll
Done


http://gerrit.cloudera.org:8080/#/c/21029/18/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/21029/18/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3321
PS18, Line 3321: ns.checkState(mod
> Should replace this with modification.newVersionNumber()
Done


http://gerrit.cloudera.org:8080/#/c/21029/19/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/21029/19/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2671
PS19, Line 2671: addCatalogServiceIdentifiers
nit: It might be worth to change this and 2 other overload of this method into 
static and provide way to access it via InProgressTableModification object.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 19
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Fri, 05 Apr 2024 18:05:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 811 insertions(+), 549 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/19
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 19
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/18/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/21029/18/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3321
PS18, Line 3321: newCatalogVersion
Should replace this with modification.newVersionNumber()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 18
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Fri, 05 Apr 2024 17:06:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-03 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/18//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21029/18//COMMIT_MSG@51
PS18, Line 51: - Pass exhaustive tests.
Rerun exhaustive tests overnight and it is now failing at some tests. I'll take 
a closer look whats happening.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 18
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 03 Apr 2024 23:21:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-02 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 18:

ps18 is a rebase.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 18
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 03 Apr 2024 00:07:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-02 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 18:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 18
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 03 Apr 2024 00:08:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-04-02 Thread Riza Suminto (Code Review)

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 781 insertions(+), 515 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/18
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 18
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-19 Thread Quanlong Huang (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 17: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 19 Mar 2024 23:54:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-19 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 17:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 19 Mar 2024 16:04:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-19 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/16/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/21029/16/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3429
PS16, Line 3429:   // if the table is being replicated we issue the HMS API 
to truncate the table
> This will introduce dangling version if !isTableBeingReplicated and !params
Moved this within try and check for those conditions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 19 Mar 2024 15:42:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-19 Thread Riza Suminto (Code Review)

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 764 insertions(+), 512 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/17
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 17
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-14 Thread Quanlong Huang (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 16:

(1 comment)

Overall looks good to me. Just have a minor comment. Thanks for adding the 
tests!

http://gerrit.cloudera.org:8080/#/c/21029/16/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/21029/16/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3429
PS16, Line 3429:   modification.registerInflightEvent();
This will introduce dangling version if !isTableBeingReplicated and 
!params.isDelete_stats(). In such case, no HMS operations will be invoked.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Thu, 14 Mar 2024 07:08:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-12 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 16:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 12 Mar 2024 13:10:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-12 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 16:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/21029/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21029/14//COMMIT_MSG@48
PS14, Line 48: Testing:
> Currently, we just have flaky tests like test_iceberg_self_events that can
Added TestEventProcessingWithImpala.


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1203
PS14, Line 1203:
   :   /**
> Let's fix this as well, i.e. only log this when the version is added for th
Done


http://gerrit.cloudera.org:8080/#/c/21029/14/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/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1054
PS14, Line 1054: CatalogServiceCatalog catalog, Table table, long 
newVersionNumber) {
> 'isInsertEvent' is always used as false. I think we can't track the infligh
Done


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1080
PS14, Line 1080:  * Cancel the new version number registration out of 
table's in-flight events.
> nit: please add an error message.
Done


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1221
PS14, Line 1221:
   :   responseSum
> nit: "Update the table object with the new catalog version"
Done


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1248
PS14, Line 1248:
   : case DROP_PARTITION:
   :   TAlterTableDropPartitionParams dropPartPa
> nit: this comment is stale
Removed.


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1376
PS14, Line 1376:
> nit: maybe 'modification.isInProgress()' sounds better
Done


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1592
PS14, Line 1592: if (!needsToUpdateHms) {
> Can we remove this if the Iceberg operations won't update HMS? It also seem
This is still needed, because ALTER_TABLE operation via Iceberg's HiveCatalog 
can produce event as well.
What needsToUpdateHms means is whether caller of alterIcebergTable() need to 
follow up with separate HMS alter table call that is not through Iceberg 
library.


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2650
PS14, Line 2650:   if (partition.getPartitionStatsCompressed() != null) 
{
> This is unchanged yet.
Changed to use InProgressTableModification.


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3239
PS14, Line 3239:   try {
> This is also unchanged but it's a bit complex - we are not always triggerin
Changed all three truncate* function to return InProgressTableModification.
This truncateTable() table only need to call 
markInflightEventRegistrationComplete().


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5512
PS14, Line 5512: String.format(HMS_RPC_ERROR_FORMAT_STR, 
"alter_table"), e);
> This is also unchanged for RENAME but I think we can't do it before the HMS
Add TODO for future investigation.


http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7242
PS14, Line 7242:
> This is also unchanged for inserting Iceberg tables.
Changed to use InProgressTableModification.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 12 Mar 2024 12:58:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-12 Thread Riza Suminto (Code Review)

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Add TestEventProcessingWithImpala, run it with debug_action
  and sync_ddl dimensions.
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/custom_cluster/test_events_custom_configs.py
9 files changed, 757 insertions(+), 510 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/16
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-11 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 15:

ps15 is just a rebase to pick up IMPALA-12678


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 15
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Tue, 12 Mar 2024 04:03:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-08 Thread Quanlong Huang (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

Patch Set 14:

(12 comments)

Took a second round of review and found there are several usages of
addVersionsForInflightEvents() that are unchanged. I think some need changes
and some can be dealt with in future JIRAs to limit the scope of this patch.

Also found we have some cases that adding versions to the list without firing
HMS RPCs, which leads to dangling items in the list. Filed IMPALA-12884 to add
GC to the list.

http://gerrit.cloudera.org:8080/#/c/21029/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21029/14//COMMIT_MSG@48
PS14, Line 48: Testing:
Currently, we just have flaky tests like test_iceberg_self_events that can
cover this fix. It'd be nice if we can add a test that always fails without
this fix, probably by injecting a sleep before the above step-7.

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1203
PS14, Line 1203: LOG.info("Added catalog version {} in database's {}
in-flight events",
: versionNumber, db.getName());
Let's fix this as well, i.e. only log this when the version is added for the db.

http://gerrit.cloudera.org:8080/#/c/21029/14/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/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1054
PS14, Line 1054: isInsertEvent_ = isInsertEvent;
'isInsertEvent' is always used as false. I think we can't track the inflight
INSERT events before we firing them since we need the HMS RPC to return the
event ids. Probably we can remove this field and replace the usages with
'false'.

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1080
PS14, Line 1080: Preconditions.checkState(inflightEventRegistered_);
nit: please add an error message.

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1221
PS14, Line 1221: Get the new table object with an updated catalog
: // version.
nit: "Update the table object with the new catalog version"

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1248
PS14, Line 1248: Get the table object
: // with an updated catalog version. If the partition
does not exist and
: // "IfExists" is true, null is returned.
nit: this comment is stale

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1376
PS14, Line 1376: modification.hasInProgressModification()
nit: maybe 'modification.isInProgress()' sounds better

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1592
PS14, Line 1592: catalog_.addVersionsForInflightEvents(false, tbl,
newCatalogVersion);
Can we remove this if the Iceberg operations won't update HMS? It also seems
buggy that we don't do this when 'needsToUpdateHms' is true. We can investigate
this in a separate JIRA if you think it's a new area.

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2650
PS14, Line 2650: catalog_.addVersionsForInflightEvents(false, table,
newCatalogVersion);
This is unchanged yet.

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3239
PS14, Line 3239: catalog_.addVersionsForInflightEvents(false, table,
newCatalogVersion);
This is also unchanged but it's a bit complex - we are not always triggering
HMS RPCs. It depends on whether the table is being replicated by Hive and
whether params.isDelete_stats() is true.

It's an exising issue that we might add a catalog version without a
corresponding event. So the version won't be cleared in the inFlight event list
anymore. Filed IMPALA-12884

http://gerrit.cloudera.org:8080/#/c/21029/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5512
PS14, Line 5512: catalog_.addVersionsForInflightEvents(false,
result.second, newCatalogVersion);
This is also unchanged for RENAME but I think we can't do it before the HMS RPC
since the new table is not created in catalog cache at that time.

There is a chance (though rare) that the RENAME event is processed in parallel
when the current thread is

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-07 Thread Jason Fehr (Code Review)

Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 14: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21029/13/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/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1090
PS13, Line 1090:   + " isRemoved=" + isRemoved);
> Precodition is added to accomodate this comment:
Done


http://gerrit.cloudera.org:8080/#/c/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1393
PS13, Line 1393: } finally {
> There should only be 1 thread tracking using 1 InProgressTableModification
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Thu, 07 Mar 2024 22:01:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-07 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 14:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Thu, 07 Mar 2024 16:49:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-07 Thread Riza Suminto (Code Review)

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21029/13/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/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1090
PS13, Line 1090:   + " isRemoved=" + isRemoved);
> Can this function be modified to be called multiple times by removing the p
Precodition is added to accomodate this comment:
https://gerrit.cloudera.org/c/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#1078

>From perspective of call path that track using this 
>InProgressTableModification, I think the Precondition is good to enforce that 
>cancelInflightEvent() should always be preceded by registerInflightEvent().


http://gerrit.cloudera.org:8080/#/c/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1393
PS13, Line 1393: } finally {
> This pattern of checking hasInflightEventRegistration followed by calling c
There should only be 1 thread tracking using 1 InProgressTableModification 
object.
The possible concurrent modification is for 
table_.removeFromVersionsForInflightEvents(), not the cancelInflightEvent() 
itself.

Modified cancelInflightEvent() comment to clarify this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Thu, 07 Mar 2024 16:27:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-07 Thread Riza Suminto (Code Review)

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
5 files changed, 288 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/14
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-07 Thread Jason Fehr (Code Review)

Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 13:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21029/13/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/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1090
PS13, Line 1090:   markInflightEventRegistrationComplete();
Can this function be modified to be called multiple times by removing the 
precondition?


http://gerrit.cloudera.org:8080/#/c/21029/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1393
PS13, Line 1393:   if (modification.hasInflightEventRegistration()) 
modification.cancelInflightEvent();
This pattern of checking hasInflightEventRegistration followed by calling 
cancelInflightEvent is repeated several times.  The comment on 
cancelInflightEvent indicates there could be multiple threads calling 
cancelInflightEvent at the same time.  If that is the case, there is a 
potential with this pattern to fail the precondition check where another thread 
calls cancelInflightEvent before the code here calls it since this pattern is 
not an atomic operation.

Can the cancelInflightEvent function be modified to be called multiple times?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 13
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Thu, 07 Mar 2024 16:01:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-06 Thread Csaba Ringhofer (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 13: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 13
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 20:11:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 13:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 13
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 20:11:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/12/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/21029/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6452
PS12, Line 6452:   
msTbl.putToParameters(StatsSetupConst.DO_NOT_UPDATE_STATS, 
StatsSetupConst.TRUE);
> My bad, this swallowed by line reverts. Will add this back.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 13
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 19:46:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
5 files changed, 287 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/13
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 13
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/12/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/21029/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6452
PS12, Line 6452:
> was registerInflightEvent() removed on purpose?
My bad, this swallowed by line reverts. Will add this back.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 12
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 19:42:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-06 Thread Csaba Ringhofer (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21029/12/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/21029/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6452
PS12, Line 6452:
was registerInflightEvent() removed on purpose?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 12
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 19:36:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

2024-03-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 11:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 11
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 19:13:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21029 )

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..


Patch Set 12:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/21029/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21029/10//COMMIT_MSG@11
PS10, Line 11:
> I think that this is a bit misleading - the issue could probably happen on
I think so. Removed this sentence.


http://gerrit.cloudera.org:8080/#/c/21029/10//COMMIT_MSG@15
PS10, Line 15: part1).
> nit: lock is
Done


http://gerrit.cloudera.org:8080/#/c/21029/7/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/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4493
PS7, Line 4493:   partitionCachingOpMap);
> Probably not, but I prefer matching the old-behavior like in old-code, L114
Added check and log at patch set 11.


http://gerrit.cloudera.org:8080/#/c/21029/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6135
PS7, Line 6135:   if (cachePoolName != null) {
> This is one spot where I'm no sure about. What if some alterPartitions went
I'm guessing that partition-level event is simpler, such that 
registerInflightEvent() is only needed if no exception is thrown at all.
Added comment.


http://gerrit.cloudera.org:8080/#/c/21029/9/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/21029/9/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5317
PS9, Line 5317:* If purge is true, partition data is
> I'll try run core tests with this removed.
Exhaustive tests pass without this catalog version bumped. Removed this line.


http://gerrit.cloudera.org:8080/#/c/21029/10/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/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1078
PS10, Line 1078:  */
> Can we call this when inflightEventRegistered_ is false? If not, then a pre
Done


http://gerrit.cloudera.org:8080/#/c/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1081
PS10, Line 1081:   // Since inflightEventRegistered_ is True, isRemoved 
should also be True.
   :   // Unless, if the in-flight event has been concurrently 
removed by other call site
   :   // such as MetastoreEvent.isSelfE
> Can you add a comment about why isRemoved can be false?
Done


http://gerrit.cloudera.org:8080/#/c/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1390
PS10, Line 1390:   // Make sure all the modifications are done.
> Shouldn't we do something with `modification` in case of an exception? Can
I just add cancelInflightEvent() here. Not sure how to undo 
table_.hasInProgressModification().


http://gerrit.cloudera.org:8080/#/c/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6447
PS10, Line 6447: computing/
> This won't catch exceptions from alterTableWithTransaction as it wraps the
Done


http://gerrit.cloudera.org:8080/#/c/21029/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6452
PS10, Line 6452:
> This could be done outside the try block at line 6424
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 12
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala 
Gerrit-Comment-Date: Wed, 06 Mar 2024 18:59:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL

Hello Quanlong Huang, Jason Fehr, Sai Hemanth Gantasala, Csaba Ringhofer, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12543: Detect self-events before finishing DDL
..

IMPALA-12543: Detect self-events before finishing DDL

test_iceberg_self_events has been flaky for not having
tbls_refreshed_before equal to tbls_refreshed_after in-between query
executions.

Further investigation reveals concurrency bug due to db/table level
lock is not taken during db/table self-events check (IMPALA-12461
part1). The order of ALTER TABLE operation is as follow:

1. alter table starts in CatalogOpExecutor
2. table level lock is taken
3. HMS RPC starts (CatalogOpExecutor.applyAlterTable())
4. HMS generates the event
5. HMS RPC returns
6. table is reloaded
7. catalog version is added to inflight event list
8. table level lock is releases

Meanwhile the event processor thread fetches the new event after 4 and
before 7. Because of IMPALA-12461 (part 1), it can also finish
self-events checking before reaching 7. Before IMPALA-12461, self-events
would have needed to wait for 8. Note that this issue is only relevant
for table level events, as self-events checking for partition level
events still takes table lock.

This patch fix the issue by adding newCatalogVersion to the table's
inflight event list before updating HMS. If HMS update does not
complete (ie., an exception is thrown), the new newCatalogVersion that
was added is then removed.

This patch also fix few smaller issues, including:
- Avoid incrementing EVENTS_SKIPPED_METRIC if numFilteredEvents == 0 in
  MetastoreEventFactory.getFilteredEvents().
- Increment EVENTS_SKIPPED_METRIC in
  MetastoreTableEvent.reloadTableFromCatalog() if table is already in
  the middle of reloading (revealed through flaky
  test_skipping_older_events).
- Rephrase misleading log message in
  MetastoreEventProcessor.getNextMetastoreEvents().

Testing:
- Pass exhaustive tests.

Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.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/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
5 files changed, 286 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/21029/12
--
To view, visit http://gerrit.cloudera.org:8080/21029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 12
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Sai Hemanth Gantasala

[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL