morningman opened a new pull request, #68846:
URL: https://github.com/apache/doris/pull/68846

   ### What problem does this PR solve?
   
   Issue Number: None
   
   Related PR: #66717
   
   Problem Summary:
   
   **In short.** This brings master in line with branch-4.x (#66717) on what 
turning off an Iceberg catalog's table cache means. On master, 
`meta.cache.iceberg.table.enable=false` or 
`meta.cache.iceberg.table.capacity=0` turns off only the table cache. The 
latest-snapshot cache keeps pinning the snapshot id that an earlier query 
cached, so queries keep returning old data after new commits for up to 24 
hours. With this PR the latest snapshot, its schema, the inferred file format 
and the table comment are read live in that configuration, as they already are 
for `ttl-second=0`.
   
   **Background.** An Iceberg catalog's table metadata cache is configured by 
`meta.cache.iceberg.table.enable`, `ttl-second` and `capacity`. On branch-4.x 
the latest snapshot used to live inside the table cache value; #66717 moved it 
into its own `snapshot` entry and mapped the three `table` keys onto it, so 
they keep governing both. The schema cache there is keyed by schema id, so it 
follows whichever snapshot is pinned. On master the connector keeps several 
caches next to the table cache that have no keys of their own: the 
latest-snapshot pin (snapshot id and schema id per table), the file format 
inferred from the first data file, and, for REST vended-credentials catalogs, 
the table comment. All three are built from `ttl-second` alone, with a fixed 
capacity of 1000. The latest schema comes from the FE schema cache, which is 
keyed by table name. `IcebergConnector.schemaCacheTtlSecondOverride` turns that 
schema cache off only for `ttl-second<=0`.
   
   **The problem, and what it cost.** With `table.enable=false` or 
`table.capacity=0`, master loads the table live on every query but still serves 
the snapshot id cached by an earlier query. That entry expires only after it 
has gone unread for 24 hours, so a catalog that is queried regularly can stay 
on one snapshot until someone runs REFRESH. The latest schema comes from the 
name-keyed schema cache, so a column added after it was cached stays hidden 
too. The comment of a vended-credentials catalog is cached in the same way. An 
operator who switched the table cache off to see external writes at once gets 
neither the new data nor the new schema. In addition, `table.capacity` does not 
limit these caches. This has been the case since the catalog moved to the 
connector framework (#64304, #66633).
   
   **How this PR fixes it.**
   
   - `IcebergConnector` derives one spec from the table entry's `enable`, 
`ttl-second` and `capacity` and builds the latest-snapshot, format and comment 
caches from it. The cache classes take a `CacheSpec`, like `IcebergTableCache` 
and `IcebergPartitionCache` already do; their `(ttl, capacity)` constructors 
used by tests are kept.
   - The table entry's `max-weight` is not passed on. It bounds only the table 
cache, as on branch-4.x, where the snapshot entry inherits enable, ttl and 
capacity but not the weight. The catalog-level and FE-global weight limits 
still apply to these caches.
   - `schemaCacheTtlSecondOverride` returns 0 when `enable=false` or 
`capacity=0` turns the table entry off, as it already does for `ttl-second=0`, 
so the latest schema is read live together with the snapshot.
   - Caches that have their own keys keep using them: 
`meta.cache.iceberg.partition.*`, `partition_view.*` and `manifest.*` are 
unchanged.
   
   **Results.**
   
   | Catalog setting | Before | After |
   |---|---|---|
   | `table.enable=false` | table loaded live; snapshot pin, format and comment 
cached for 24 h; latest schema cached | snapshot pin, schema, format and 
comment all read live |
   | `table.capacity=0` | same as above | same as above |
   | `table.capacity=N` (N > 0) | snapshot, format and comment caches hold up 
to 1000 entries | up to N entries |
   | `table.max-weight` | bounds the table cache | unchanged |
   | `table.ttl-second` (any value), default settings | unchanged | unchanged |
   
   **Classes, and how they call each other.**
   
   ```
   IcebergConnector(properties)
     ├─ cacheSpec("table") ─────────────► IcebergTableCache            (enable, 
ttl, capacity, max-weight)
     └─ tableFactsCacheSpec = enable/ttl/capacity of the table spec
          ├─► IcebergLatestSnapshotCache   ◄── 
IcebergConnectorMetadata.beginQuerySnapshot
          ├─► IcebergFormatCache           ◄── IcebergScanPlanProvider (file 
format inference)
          ├─► IcebergCommentCache          ◄── 
IcebergConnectorMetadata.getTableComment (vended REST only)
          └─► schemaCacheTtlSecondOverride ──► 
PluginDrivenExternalCatalog.overlayMetaCacheConfig
                                               
PluginDrivenMvccExternalTable.getLatestSchemaCacheValue
   ```
   
   ### Release note
   
   For Iceberg catalogs, `meta.cache.iceberg.table.enable=false` and 
`meta.cache.iceberg.table.capacity=0` now also turn off caching of the latest 
snapshot, the latest schema, the inferred file format and the table comment, as 
`meta.cache.iceberg.table.ttl-second=0` does, so every query reads the latest 
snapshot. `meta.cache.iceberg.table.capacity` now also limits those caches.
   
   ### 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?  -->
   
       `IcebergConnectorCacheTest`:
       - `disabledTableEntryReadsSnapshotFormatAndCommentLive`: with 
`table.enable=false`, and again with `table.capacity=0`, a second snapshot 
lookup reloads instead of hitting the cache, the format and comment caches are 
off, and the schema cache override is 0.
       - `tableEntryKeysExceptMaxWeightSizeSnapshotFormatAndCommentCaches`: 
`table.ttl-second` and `table.capacity` reach the three caches; 
`table.max-weight` does not.
   
       `run-fe-ut.sh --run 'org/apache/doris/connector/iceberg/**/*Test'`: 1483 
tests, 0 failures. Each of the following changes makes the new cases fail: 
building the caches from `ttl-second` alone (the previous wiring), passing them 
the table entry's `max-weight`, or dropping the schema override. Not covered 
end to end by a regression case.
   
   - Behavior changed:
       - [ ] No.
       - [x] Yes. With `meta.cache.iceberg.table.enable=false` or `capacity=0`, 
Iceberg queries read the latest snapshot and schema live instead of from 
caches, and `table.capacity` now also limits the snapshot, format and comment 
caches.
   
   - 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]

Reply via email to