Repository navigation
[fix](fe) Skip HMS database events that include/exclude_database_list filters out - #70165
Merged
Merged
Conversation
… filters out ### What problem does this PR solve? Issue Number: None Related PR: apache#67417, apache#68196 Problem Summary: A catalog lists its databases through getFilteredDatabaseNames, which drops a remote database that `exclude_database_list` names or, when `include_database_list` is set, one it does not name. On an HMS catalog with `hive.enable_hms_events_incremental_sync`, a CREATE DATABASE event (and the new name of an ALTER DATABASE rename) reaches PluginDrivenExternalCatalog#registerDatabaseFromEvent, which added the database to the cached database names without that check: on every FE that applies the event, SHOW DATABASES lists it and queries can use it until the names are reloaded (external_cache_refresh_time_minutes, 10 by default, REFRESH CATALOG or a restart). branch-4.x skips such events since apache#67417. The check moves into ExternalCatalog#isDatabaseListed, which the listing and registerDatabaseFromEvent share; an event for a database the lists keep out returns before it touches any cache. Like the listing, it compares the remote name exactly. Not ported: skipping the table, partition and refresh events of such databases (on master they find no database and return), and comparing names ignoring case when lower_case_database_names is not 0, which would change the listing too. ### Release note An HMS catalog with incremental metastore event sync no longer lists a newly created database that include_database_list or exclude_database_list filters out. ### Check List (For Author) - Test: Unit Test - New PluginDrivenExternalCatalogEventFilterTest: with the names cached, an event for a database that either list keeps out leaves them unchanged, and one for an allowed database adds it; both cases fail without the check. - ExternalCatalogTest, PluginDrivenExternalCatalogIdentityTest, PluginDrivenExternalCatalogDdlRoutingTest and the MetastoreEventSyncDriver tests pass. - Behavior changed: Yes. An HMS event no longer adds a database that the catalog's include/exclude database lists keep out. - Does this need documentation: No Co-authored-by: 924060929 <lanhuajian@selectdb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed all three changed files at head 20efa876dbac76deccca01cf695ffee8cfae17d3. No new actionable findings. No existing inline review findings were present to retain or duplicate. No additional user focus was supplied.
The traced flow is metastore CREATE / rename target -> MetastoreEventSyncDriver -> CatalogMgr -> registerDatabaseFromEvent -> shared remote-name predicate -> database/cache publication. Rename removes the old local identity first; connector invalidation runs before registration.
Critical checkpoint conclusions:
- Goal and proving tests: The guard prevents include/exclude-filtered databases entering the cached names through events. The two added JUnit tests cover each denial mode and allowed insertion into a hot snapshot; their assertions would expose the previous insertion behavior. This is static assessment, not a test-run result.
- Scope and reuse: The change is focused. The extracted predicate is equivalent to the previous listing lambda and is reused by event registration.
- Concurrency and locking: The event daemon can overlap query loads and catalog ALTER/refresh. Predicate maps are local; no new lock or lock-order edge is added. Existing catalog reset and guarded cache publication remain in place. No new concurrency defect was substantiated.
- Lifecycle: Rejected events return before database construction. Allowed events retain existing finally invalidation. ALTER retires cache owners and invalidates names/object generations; no new resource or static-initialization dependency is introduced.
- Configuration: No new item. Existing include/exclude properties are read when applying an event and retain the existing ALTER/reset behavior.
- Compatibility: No storage, journal, wire format or existing method signature is changed. Older FEs retain their previous event behavior until upgraded.
- Thrift/protobuf: Not applicable; no definitions change.
- Parallel paths: Cached and session-bypass listing share the filter. CREATE and database rename use the guarded registration; DROP still removes state. Table/partition paths cannot register a missing filtered database.
- Special conditions: Remote names are filtered before local mapping, exactly as in listing. Exclude precedence, empty-list behavior, case-sensitive comparison and the information_schema/mysql exemptions are preserved. The guard's comment explains its purpose.
- Test coverage: Inspected the new positive/negative tests and existing listing, identity, mapping, rename, seeding and cache-publication tests. The new fixture genuinely warms names, and its allowed name is absent from the remote listing, so successful insertion depends on the event path. Live HMS and concurrent runtime behavior were not exercised.
- Test results: No expected-output files change. New assertions match the intended behavior; no test pass is claimed by this review.
- Observability: Ignoring configured-out databases is expected behavior. Existing descriptor/catalog failure context is retained; no additional metric or log is needed for this small predicate guard.
- Transactions and persistence: No new journal payload or transaction operation. Existing ordered per-FE application and master cursor replication remain unchanged; ignored registration completes normally before cursor advancement.
- Data writes and crash atomicity: No user-data write is added. Accepted events keep existing names/object/index publication; rejected events publish nothing. No new crash-recovery defect was identified.
- FE/BE variable passing: Not applicable; no transmitted variable changes.
- Performance: Listing still parses filters once and scans the names once. Database-event registration parses the two lists, then avoids database/cache work for rejected events. No material performance issue was found.
- Other core invariants: No substantiated metadata-visibility, error-propagation or ownership regression was found. Partition versions, delete bitmaps and BE memory accounting are outside the changed behavior.
Both initial risk focuses were dismissed with code evidence: event identity matches listing identity, and the early return does not bypass rename removal or preceding connector invalidation. There are no unresolved code candidates or uncovered changed files. Validation limits: static inspection only; no builds, tests, benchmarks, or live HMS integration were run.
Review converged after 2 round(s).
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28853 ms |
Contributor
TPC-DS: Total hot run time: 151826 ms |
Contributor
ClickBench: Total hot run time: 24.41 s |
924060929
approved these changes
Nov 5, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
Normal database listing filters remote names before local-name mapping. HMS create/rename events previously bypassed that filter when updating FE caches. This PR shares the existing predicate with event registration.
Independent static review of all three changed files and the parser, driver, catalog routing, cache publication, and related tests found no actionable P0/P1/P2 regression. Exclude precedence, system-database exemptions, rename cleanup, and allowed registration remain intact. APPROVE within the FE scope; no FE/BE protocol changes are involved.
No builds or tests were run. The supplied current-head snapshot reports successful P0 checks, compilation and FE UT, but check_coverage_fe failed. Required-check rules are unknown; this approval does not establish merge readiness.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #67417, #68196
Problem Summary:
In short. This ports from branch-4.x #67417 and #68196 the check that keeps a metastore event from adding a database that
include_database_listorexclude_database_listkeeps out of an HMS catalog. On master, a CREATE DATABASE event for such a database puts it into the catalog's database list on every FE that applies the event: SHOW DATABASES lists it and queries can reach it until the list is reloaded.Background. A catalog lists its databases through
ExternalCatalog#getFilteredDatabaseNames, which drops a remote database thatexclude_database_listnames or, wheninclude_database_listis set, one that it does not name. On an HMS catalog withhive.enable_hms_events_incremental_sync,MetastoreEventSyncDriverapplies the metastore's events on every FE. A CREATE DATABASE event, and the new name of an ALTER DATABASE rename, reachPluginDrivenExternalCatalog#registerDatabaseFromEvent, which adds the database to the cached database names. branch-4.1 and branch-4.2 have checked the two lists for these events since #67417.The problem, and what it cost.
registerDatabaseFromEventadds the database without the check. A database that the catalog is configured to hide then shows in SHOW DATABASES,USEand queries reach it (privileges still apply), and table events register tables in it. It goes away only when the cached names are reloaded: afterexternal_cache_refresh_time_minutes(10 by default), on a lookup of a name that is not cached, on REFRESH CATALOG, or after a restart.How this PR fixes it. The check moves out of
getFilteredDatabaseNamesintoExternalCatalog#isDatabaseListed, which the listing andregisterDatabaseFromEventshare; an event for a database the lists keep out returns before it touches any cache. Like the listing, the check compares the remote name exactly. Not ported from branch-4.x: skipping the table, partition and refresh events of such databases, a performance item (on master they find no database and return, after one reload of the names), and comparing names ignoring case whenlower_case_database_namesis not 0, which would change the listing as well.Results.
archived,exclude_database_list = archivedarchivedlistedother,include_database_list = sales,sales2otherlistedsales2, allowed by either listOriginal author: @924060929
Release note
An HMS catalog with incremental metastore event sync no longer lists a newly created database that include_database_list or exclude_database_list filters out.
Check List (For Author)
Test
New
PluginDrivenExternalCatalogEventFilterTestruns the three events of the table on a catalog whose database names are cached: both cases fail without the check, with the filtered-out database in the names.ExternalCatalogTest(50),PluginDrivenExternalCatalogIdentityTest,PluginDrivenExternalCatalogDdlRoutingTest(90) and theMetastoreEventSyncDriverrename, mapped name and seeding tests pass, and fe-core Checkstyle reports nothing. The end-to-end path needs a Hive metastore with event notifications, which was not run locally.Behavior changed:
A metastore event no longer adds a database that the catalog's include_database_list or exclude_database_list keeps out.
Does this need documentation?
Check List (For Reviewer who merge this PR)