morningman opened a new pull request, #68860:
URL: https://github.com/apache/doris/pull/68860
### What problem does this PR solve?
Issue Number: None
Related PR: #67023, #67395, #67996
Problem Summary:
**In short.** Since #67395, every Paimon table lookup on master clears that
table's cached metadata before reloading it, so the Paimon metadata cache never
hits. Every statement reloads the table and its latest snapshot from the remote
catalog, and a catalog with caching enabled no longer serves its cached
snapshot. This PR removes the invalidation and restores the regression
expectation that #67395 changed.
**Background.** #67395 forward-ported branch-4.1 #67023, which keeps the
Paimon write cache coherent. In branch-4.x, the Doris table cache sits above
Paimon's SDK `CachingCatalog`. On a Doris cache miss, #67023 invalidates the
SDK entry before reloading, so the reload cannot hand back a stale SDK table.
#67395 placed that invalidation in `CatalogBackedPaimonCatalogOps.getTable`. On
master, however, #67996 had already replaced the SDK `CachingCatalog` with the
Doris-owned `PaimonMetaCacheCatalog`, and `PaimonCatalogFactory` always
disables the SDK wrapper. The catalog that `getTable` wraps is therefore the
Doris cache itself.
| | branch-4.x | master since #67395 |
|---|---|---|
| Table cache | Doris table cache above Paimon's SDK `CachingCatalog` |
`PaimonMetaCacheCatalog` only (SDK wrapper disabled) |
| When the reload path runs | only on a Doris cache miss | on every
`getTableHandle`, several times per statement |
| What the invalidation clears | the SDK entry about to be reloaded | the
Doris entries for the table: table, latest snapshot, schema-at memo, partition
view |
**The problem, and what it cost.**
- **The cache never hits.** On a local filesystem catalog, 7 SELECT
statements made 86 table-cache requests, with 0 hits and 38 invalidations. The
latest-snapshot cache had 0 hits in 24 requests. On HMS, REST or object-store
catalogs, each of those reloads is a remote call, and it happens in every
statement.
- **The cache semantics changed.** A catalog with caching enabled used to
keep reading its cached snapshot until REFRESH or the cache TTL. Now it reads
data committed by other engines at once. #67395 changed
`test_paimon_table_meta_cache` to expect the rows Spark wrote without a
REFRESH, and attributed this to Paimon 1.4. branch-4.2 runs the same Paimon
1.4.2, and the same test there still expects the cached rows.
**How this PR fixes it.**
- `CatalogBackedPaimonCatalogOps.getTable` no longer invalidates the table.
Nothing is lost by this: REFRESH TABLE / CATALOG, Doris writes
(`refreshTableAfterExternalMutation`) and DDL all invalidate through
`CatalogMetaCache`, which clears the `PaimonMetaCacheCatalog` entries, and no
other layer caches the table.
- `test_paimon_table_meta_cache` expects the cached rows again before the
REFRESH.
- `PaimonTableOptionsTest#catalogOpsInvalidatesPaimonCacheBeforeReload`
pinned the invalidation. It is replaced by
`PaimonMetaCacheCatalogTest#connectorLookupsHitTheTableCacheUntilTheTableIsInvalidated`:
repeated lookups through `CatalogBackedPaimonCatalogOps` load the table once,
and a lookup after the table is invalidated loads it again.
**Results.** Measured on a local FE + BE with a filesystem Paimon catalog,
with a second catalog on the same warehouse playing the external writer:
| | before | after |
|---|---|---|
| Cached catalog, after the other catalog wrote a row | sees the new row |
sees the cached rows until REFRESH TABLE |
| Cached catalog, after its own INSERT | sees it | sees it |
| Table-cache hits | 0 of 86 requests (7 SELECTs) | 20 of 20 requests for 5
repeated SELECTs |
Putting the invalidation back makes the new unit test fail. All 675 tests of
`fe-connector-paimon` pass.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_paimon_table_meta_cache` needs the Paimon docker fixture and
was not run locally.
- [x] Unit Test
- `PaimonMetaCacheCatalogTest`, `PaimonTableOptionsTest` and the
rest of `fe-connector-paimon` (675 tests) pass locally.
- [x] Manual test (add detailed scripts or steps below)
- Local FE + BE with two filesystem Paimon catalogs on one warehouse
(one with `meta.cache.paimon.table.ttl-second` = 0); results in the table above.
- [ ] 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? -->
- Behavior changed:
- [ ] No.
- [x] Yes. A Paimon catalog with metadata caching enabled serves
repeated queries from its cache again. Data committed by other engines becomes
visible after REFRESH or the cache TTL, as in branch-4.x and on master before
#67395.
- 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]
