morningman opened a new pull request, #70165:
URL: https://github.com/apache/doris/pull/70165
### 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_list` or
`exclude_database_list` keeps 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 that
`exclude_database_list` names or, when `include_database_list` is set, one that
it does not name. On an HMS catalog with
`hive.enable_hms_events_incremental_sync`, `MetastoreEventSyncDriver` applies
the metastore's events on every FE. A CREATE DATABASE event, and the new name
of an ALTER DATABASE rename, reach
`PluginDrivenExternalCatalog#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.** `registerDatabaseFromEvent` adds the
database without the check. A database that the catalog is configured to hide
then shows in SHOW DATABASES, `USE` and queries reach it (privileges still
apply), and table events register tables in it. It goes away only when the
cached names are reloaded: after `external_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 `getFilteredDatabaseNames`
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, 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 when `lower_case_database_names` is not 0, which would change the
listing as well.
**Results.**
| Event, with the database names cached | Before | After |
|---|---|---|
| CREATE DATABASE `archived`, `exclude_database_list = archived` |
`archived` listed | not listed |
| CREATE DATABASE `other`, `include_database_list = sales,sales2` | `other`
listed | not listed |
| CREATE DATABASE `sales2`, allowed by either list | listed | listed |
Original 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 <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
New `PluginDrivenExternalCatalogEventFilterTest` runs 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 the
`MetastoreEventSyncDriver` rename, 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:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
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?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
