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 2025/04/18 03:40:30 UTC
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22793
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
IMPALA-13829: Fix waitForHmsEvent response might miss deletions
When a db/table is removed in the catalog cache, catalogd assigns it a
new catalog version and put it into the deleteLog which is used for the
catalog update thread to collect deletion updates. Once the catalog
update thread collects a range of updates, it triggers GC in the
deleteLog to clear items older than the last sent catalog version. The
deletions will be broadcasted by statestore to all the coordinators
eventually.
However, waitForHmsEvent requests is also a consumer of the deleteLog
and could be impacted by these GCs. waitForHmsEvent is a catalogd RPC
used by coordinators when a query wants to wait until the related
metadata is in synced with HMS. The response of waitForHmsEvent returns
the latest metadata including the deletions on related dbs/tables.
If the related deletions in deleteLog is GCed just before the
waitForHmsEvent request collects the results, they will be missing in
the response. Coordinator might keep using stale metadata of
non-existing dbs/tables.
Though the deletions are GCed and can't be collected by the response,
they have been sent to statestore and will arrive in the coordinator
side eventually. This patch adds one more wait step in coordinator side
to wait for their arrival.
Implementation
In catalogd side, after collecting results for the waitForHmsEvent
request, set the last sent catalog version in the response. This is a
catalog version before which all the updates have been sent to
statestore.
In coordinator side, after applying updates of the response, waits until
the local catalog version is updated till that version. So all the
possibly missing deletions must have arrived.
Also removed some unused imports.
Tests:
- Added e2e test. Without the fix, it consistently fails when runs for
2-3 times.
Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
---
M be/src/service/fe-support.cc
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/metadata/test_event_processing.py
9 files changed, 80 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/22793/1
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1: Code-Review-2
This can be abandoned, right?
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 03 May 2025 05:40:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has abandoned this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Abandoned
Fixed by another patch https://gerrit.cloudera.org/c/22816/
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11755/
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Apr 2025 13:17:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
> Patch Set 1:
>
> Let me try rebase
Sorry, I forgot to delete this. Will not rebase.
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Apr 2025 16:07:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22793/1/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/22793/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4422
PS1, Line 4422: // Send back the last catalog version that has been propagated to statestore.
: // Coordinators can wait for that in case some deletions are missing due to deleteLog
: // being GCed.
: res.result.setVersion(lastSentTopicUpdate_.get());
> This is the critical code block in getCatalogDelta() that ideally should no
Good catch!
For hitting ConcurrentModificationException, deleteLog_.retrieveDbObjects() returns a ImmutableList of copied values. So we are safe.
However, we do need to protect the read/write on the conbination of deleteLog_ and lastSentTopicUpdate_. However, adding a lock for this might block the topic update thread.
I'm thinking a simpler solution that postpond the GC in deleteLog for several topic udpates, just like what we did for topicUpdateLog_. An entry may survive for (2 * topicUpdateLogGcFrequency_) - 1 topic updates. topicUpdateLogGcFrequency_ is 1000 by default so should be safe enough, i.e. the deletions must have arrived in the impalad sides, otherwise that's an abnormal impalad and already has other issues, e.g. lots of stale tables.
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Apr 2025 08:42:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
(1 comment)
> Patch Set 1: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11755/
Let me try rebase
http://gerrit.cloudera.org:8080/#/c/22793/1/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/22793/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4422
PS1, Line 4422: // Send back the last catalog version that has been propagated to statestore.
: // Coordinators can wait for that in case some deletions are missing due to deleteLog
: // being GCed.
: res.result.setVersion(lastSentTopicUpdate_.get());
This is the critical code block in getCatalogDelta() that ideally should not happen concurrently with waitForHmsEvent() L4411-L4425.
// Garbage collect the delete and topic update log.
deleteLog_.garbageCollect(ctx.toVersion);
topicUpdateLog_.garbageCollectUpdateLogEntries(ctx.toVersion);
lastSentTopicUpdate_.set(ctx.toVersion);
That code and this setVersion() is not protected by common lock.
deleteLog_ iteration in L4411 can also run concurrently with deleteLog_.garbageCollect().
Will that be an issue?
Like waitForSyncDdlVersion(), should this just wait for notification from getCatalogDelta()?
// Wait for the next topic update.
synchronized(topicUpdateLog_) {
try {
topicUpdateLog_.wait(TOPIC_UPDATE_WAIT_TIMEOUT_MS);
} catch (InterruptedException e) {
// Ignore
}
}
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Apr 2025 16:06:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18957/ : 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/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Apr 2025 04:06:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11755/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Apr 2025 05:24:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13829: Fix waitForHmsEvent response might miss deletions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22793 )
Change subject: IMPALA-13829: Fix waitForHmsEvent response might miss deletions
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22793/1/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/22793/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4422
PS1, Line 4422: // Send back the last catalog version that has been propagated to statestore.
: // Coordinators can wait for that in case some deletions are missing due to deleteLog
: // being GCed.
: res.result.setVersion(lastSentTopicUpdate_.get());
> Good catch!
Proposed another patch as a quick fix by postponding the GC: https://gerrit.cloudera.org/c/22816/
--
To view, visit http://gerrit.cloudera.org:8080/22793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff7bc4d8b47d2aabec0f1fe73a3ab39e8e407cb
Gerrit-Change-Number: 22793
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 25 Apr 2025 03:12:00 +0000
Gerrit-HasComments: Yes