github-actions[bot] commented on code in PR #68860:
URL: https://github.com/apache/doris/pull/68860#discussion_r4236402847
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -308,11 +308,9 @@ public List<String> listTables(String databaseName) throws
Catalog.DatabaseNotEx
*/
@Override
public Table getTable(Identifier identifier) throws
Catalog.TableNotExistException {
- // Doris owns the outer table-metadata cache and only reaches this
method on an outer-cache
- // load. Paimon's CachingCatalog owns a second cache for the Table
and its lower-level readers.
- // Invalidate that inner entry before reloading, otherwise REFRESH
or outer-cache expiry can
- // immediately repopulate Doris with the same stale bucket, path,
or table options.
- catalog.invalidateTable(identifier);
+ // Every metadata, scan and write lookup lands here, several times
per statement. The wrapped
Review Comment:
[P1] Keep the snapshot pin in step with the table cache. With
`paimon.cache-enabled=false`, the table is reloaded on every lookup but
`PaimonLatestSnapshotCache` still holds the prior snapshot id for its default
24-hour access TTL. Likewise, when a configured
`paimon.cache.expire-after-write` reloads the table, the old snapshot id
survives; `beginQuerySnapshot` keeps pinning scans to it, and frequent reads
can extend that staleness indefinitely after an external INSERT. The removed
invalidation previously cleared both entries. Disable or expire the snapshot
cache whenever the table cache is disabled or expires, and cover both settings
with external-write reads.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -308,11 +308,9 @@ public List<String> listTables(String databaseName) throws
Catalog.DatabaseNotEx
*/
@Override
public Table getTable(Identifier identifier) throws
Catalog.TableNotExistException {
- // Doris owns the outer table-metadata cache and only reaches this
method on an outer-cache
- // load. Paimon's CachingCatalog owns a second cache for the Table
and its lower-level readers.
- // Invalidate that inner entry before reloading, otherwise REFRESH
or outer-cache expiry can
- // immediately repopulate Doris with the same stale bucket, path,
or table options.
- catalog.invalidateTable(identifier);
+ // Every metadata, scan and write lookup lands here, several times
per statement. The wrapped
+ // catalog is the Doris table cache (PaimonMetaCacheCatalog; the
SDK CachingCatalog is disabled),
+ // so this must not invalidate: REFRESH, Doris writes and DDL
invalidate through CatalogMetaCache.
Review Comment:
[P2] Resolve the current schema for INSERT targets. After an external `ALTER
TABLE ... ADD COLUMN`, this retained Table can still have the old
`schema().fields()`. `PaimonWritePlanProvider.getWriteColumns` uses those
fields for the statement's writer schema, so an INSERT naming the new column
fails as `Unknown column` until table expiry or REFRESH, even when
`schema.cache.ttl-second=0` lets DESC show the new column. The previous
per-lookup invalidation reloaded the target Table. Read the live target schema
at the writer boundary, and cover external ALTER followed by Doris INSERT.
--
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]