github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3775098115
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalMetaCache.java:
##########
@@ -86,7 +100,14 @@ public Table getPaimonTable(NameMapping nameMapping) {
public PaimonSnapshotCacheValue getSnapshotCache(ExternalTable dorisTable)
{
NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
- return
tableEntry.get(nameMapping.getCtlId()).get(nameMapping).getLatestSnapshotCacheValue();
+ PaimonTableCacheValue tableValue =
tableEntry.get(nameMapping.getCtlId()).get(nameMapping);
+ Table table = tableValue.getPaimonTable();
+ PaimonSnapshot fence =
latestSnapshotProjectionLoader.loadFence(nameMapping, table).getSnapshot();
Review Comment:
[P2] Avoid resolving the remote fence before every snapshot-cache lookup.
`loadFence()` runs before `snapshotEntry.get()`, and its path calls
`copyWithLatestSchema()`, `latestSnapshot()`, and `schemaManager().latest()`,
so even a hit on an admitted snapshot still performs latest-metadata discovery.
Before this change, `PaimonTableCacheValue` memoized the projection, so stable
repeated reads avoided that work. Please retain or refresh the fence under the
table generation (or otherwise put discovery behind a cache) and add a
repeated-hit call-count test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1367,9 +1381,21 @@ public int hashCode() {
public void notifyPropertiesUpdated(Map<String, String> updatedProps) {
CatalogIf.super.notifyPropertiesUpdated(updatedProps);
String schemaCacheTtl =
updatedProps.getOrDefault(SCHEMA_CACHE_TTL_SECOND, null);
- if (java.util.Objects.nonNull(schemaCacheTtl)) {
- ExternalMetaCacheMgr extMetaCacheMgr =
Env.getCurrentEnv().getExtMetaCacheMgr();
+ ExternalMetaCacheMgr extMetaCacheMgr =
Env.getCurrentEnv().getExtMetaCacheMgr();
+ if (java.util.Objects.nonNull(schemaCacheTtl)
+ ||
updatedProps.containsKey(ExternalMetaCacheBudgetManager.CATALOG_MAX_WEIGHT_PROPERTY))
{
Review Comment:
[P1] Fence this new quota invalidation against an in-flight first
initialization. `prepareCatalogByEngine()` can copy the old properties while no
group exists; if ALTER commits this setting next, `removeCatalog()` skips the
absent group, and the delayed initializer then publishes the old count-only
policy indefinitely. That silently defeats the configured memory bound. Please
version/serialize the property snapshot with removal and publication, and add a
paused ALTER-vs-init test that verifies the new weighted policy wins.
--
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]
