[ExternalCatalog](fix) external catalog meta data inconsistent when frequent refreshing

[qzsee]

What problem does this PR solve?

Issue Number: close #xxx

Race condition in lowerCaseToTableName cache:

• The catalog refresh thread clears lowerCaseToTableName.
• Query threads call getLocalTableName; on a cache miss for finalName, they fall back to listTableNames to rebuild the cache.
• listTableNames itself clears lowerCaseToTableName before repopulating it.
• If thread A has already rebuilt the cache via listTableNames, and thread B subsequently enters listTableNames, B will clear the cache again.
• Consequently, when thread A re-reads finalName from the cache, the entry is gone and the lookup fails.

Caused by: java.lang.NullPointerException: Cannot invoke "java.util.List.stream()" because the return value of "org.apache.doris.catalog.TableIf.getBaseSchema()" is null
        at org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation.computeOutput(LogicalCatalogRelation.java:104) ~[doris-fe.jar:1.2-SNAPSHOT]
        at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:186) ~[guava-33.2.1-jre.jar:?]
        at org.apache.doris.nereids.properties.LogicalProperties.getOutput(LogicalProperties.java:104) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.rules.analysis.BindRelation.lambda$build$0(BindRelation.java:123) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.pattern.PatternMatcher$1.transform(PatternMatcher.java:92) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.PlanTreeRewriteJob.rewrite(PlanTreeRewriteJob.java:57) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.PlanTreeRewriteBottomUpJob.rewriteThis(PlanTreeRewriteBottomUpJob.java:91) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.PlanTreeRewriteBottomUpJob.execute(PlanTreeRewriteBottomUpJob.java:75) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.scheduler.SimpleJobScheduler.executeJobPool(SimpleJobScheduler.java:44) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.RootPlanTreeRewriteJob.execute(RootPlanTreeRewriteJob.java:66) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.executor.AbstractBatchJobExecutor.execute(AbstractBatchJobExecutor.java:139) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.executor.Analyzer.analyze(Analyzer.java:83) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.rules.analysis.AnalyzeCTE.analyzeCte(AnalyzeCTE.java:98) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.rules.analysis.AnalyzeCTE.lambda$build$0(AnalyzeCTE.java:71) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.pattern.PatternMatcher$1.transform(PatternMatcher.java:92) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.PlanTreeRewriteJob.rewrite(PlanTreeRewriteJob.java:57) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.PlanTreeRewriteTopDownJob.execute(PlanTreeRewriteTopDownJob.java:50) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.scheduler.SimpleJobScheduler.executeJobPool(SimpleJobScheduler.java:44) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.rewrite.RootPlanTreeRewriteJob.execute(RootPlanTreeRewriteJob.java:66) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.executor.AbstractBatchJobExecutor.execute(AbstractBatchJobExecutor.java:139) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.jobs.executor.Analyzer.analyze(Analyzer.java:83) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.lambda$analyze$4(NereidsPlanner.java:365) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.keepOrShowPlanProcess(NereidsPlanner.java:898) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.analyze(NereidsPlanner.java:365) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.planWithoutLock(NereidsPlanner.java:250) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.planWithLock(NereidsPlanner.java:224) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.planWithLock(NereidsPlanner.java:180) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.NereidsPlanner.planWithLock(NereidsPlanner.java:173) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.nereids.trees.plans.commands.CreateTableCommand.run(CreateTableCommand.java:103) ~[doris-fe.jar:1.2-SNAPSHOT]
        at org.apache.doris.qe.StmtExecutor.executeByNereids(StmtExecutor.java:781) ~[doris-fe.jar:1.2-SNAPSHOT]

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

• Test

  • Regression test
  • 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

• Behavior changed:

  • No.
  • Yes.

• Does this need documentation?

  • No.
  • Yes.

Check List (For Reviewer who merge this PR)

• Confirm the release note
• Confirm test cases
• Confirm document
• Add branch pick label

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[dqz123]

run buildall

[morningman]

Improvement Suggestions for PR #63033

PR: [ExternalCatalog external catalog meta data inconsistent when frequent refreshing](#63033)
Review Date: 2026-05-06

---

The suggestions below are ordered by priority — most impactful first.

1. [P1] Bring registerTable / unregisterTable into the same publication model

registerTable and unregisterTable still mutate the live lowerCaseToTableName map directly via put / remove. If listTableNames() swaps in a fresh map between the moment one of these methods reads the volatile reference and the moment it performs the mutation, the write lands on the now-discarded map and is silently lost. The PR title explicitly calls out "frequent refreshing", so this sister race deserves attention.

Two reasonable options:

Option A — minimal change, copy-on-write: wrap the mutation in synchronized (this) and follow the same "build new, publish" pattern.

public void unregisterTable(String tableName) {
    // ... existing code ...
    if (isInitialized()) {
        synchronized (this) {
            metaCache.invalidate(...);
            Map<String, String> next = Maps.newConcurrentMap(lowerCaseToTableName);
            next.remove(dorisTable.getName().toLowerCase());
            this.lowerCaseToTableName = next;
        }
    }
    // ...
}

Option B — accept the gap: keep the in-place mutation but document explicitly that brief inconsistency is expected and will be reconciled by the next listTableNames(). Add a TODO so the assumption is visible to future maintainers.

At minimum, the PR description should acknowledge this path so reviewers and future readers don't assume the fix already covers it.

2. [P1] Add a concurrent regression test

The PR ships without any new tests. Race-condition fixes are particularly prone to regression during later refactors, and a deterministic-enough test guards the invariant cheaply. A skeleton:

@Test
public void testLowerCaseToTableNameUnderConcurrentListAndLookup() throws Exception {
    int threads = 16;
    int iterations = 5000;
    ExternalDatabase<?> db = newDbWithKnownTables("Tbl1", "Tbl2", "Tbl3");
    ExecutorService pool = Executors.newFixedThreadPool(threads);
    AtomicInteger failures = new AtomicInteger();

    // Half the threads do lookups; half drive listTableNames() to simulate refresh.
    // Any null return for a known table counts as a failure.

    pool.shutdown();
    pool.awaitTermination(30, TimeUnit.SECONDS);
    assertEquals(0, failures.get());
}

Even a 100ms hammer test in CI is far better than nothing. It would have caught the original bug and will catch its reintroduction.

3. [P2] Use a dedicated lock instead of sharing the this monitor

The this monitor is already used by makeSureInitialized() and resetMetaToUninitialized(). With the new fallback paths added on top, every fallback now holds this for the entire duration of a remote listTableNames() network call. When the upstream catalog is slow, this serializes all initialization paths on the same database, including queries that touch unrelated tables.

A dedicated lock object scopes the contention to the lowerCaseToTableName cache only:

private final Object listLock = new Object();
// ...
synchronized (listLock) {
    finalName = lowerCaseToTableName.get(tableName.toLowerCase());
    if (finalName == null) {
        listTableNames();
        finalName = lowerCaseToTableName.get(tableName.toLowerCase());
    }
}

If the author prefers the simplicity of synchronized(this), please leave a short comment explaining the deliberate intent — e.g. "intentionally using this so this section serializes with makeSureInitialized / resetMetaToUninitialized" — so the choice is preserved against future refactors.

4. [P2] Document that MetaCache.namesCache also drives listTableNames()

buildMetaCache() registers ignored -> listTableNames() as the loader of MetaCache.namesCache. When refreshAfterWrite triggers, that loader invokes listTableNames() outside the new synchronized(this) block. Correctness is fine — the volatile publication makes both writers self-consistent and last-writer-wins is acceptable — but this is non-obvious from the diff. Add a method-level comment on listTableNames():

/**
 * Builds and atomically publishes a fresh lowerCaseToTableName map.
 * May be invoked from:
 *   1. The MetaCache.namesCache loader (no external lock).
 *   2. Fallback paths in isTableExist / getLocalTableName (under `synchronized(this)`).
 * Concurrent (1) and (2) are safe by virtue of the volatile publication;
 * the latest writer wins, and either writer publishes a self-consistent map.
 */

This makes the two entry points and their concurrency relationship visible at a glance.

5. [P3] Log the freshly built map, not the volatile field

this.lowerCaseToTableName = newLowerCaseToTableName;
if (LOG.isDebugEnabled()) {
    LOG.debug("after list tables in {}.{}, lowerCaseToTableName: {}",
            getCatalog().getName(), getFullName(), lowerCaseToTableName);  // <-- volatile read
}

Because lowerCaseToTableName is volatile, another writer can swap it again between the publication and this log statement, so the debug message may show a different map than the one the current call just built. Reference the local variable instead for accurate diagnostics:

LOG.debug("after list tables in {}.{}, lowerCaseToTableName: {}",
        getCatalog().getName(), getFullName(), newLowerCaseToTableName);

6. [P3] Consider clearer naming for lowerCaseToTableName

The field actually holds lower(remoteName) -> remoteName. The current name does not say which side is local vs. remote, and a reader has to chase the call sites to find out. A name such as lowerCaseRemoteToRemoteName is more self-explanatory. Not worth doing in this PR; record it as a small follow-up cleanup.

7. [P3] Lighter internal map after publication

Once published, the new map is largely read-only (modulo the register/unregister paths discussed in suggestion 1). Using Maps.newConcurrentMap() pays a CAS cost on every put during construction even though that phase is single-threaded. If suggestion 1 (Option A) lands, the internal map could be downgraded to a plain HashMap or even an ImmutableMap, making immutability after publication explicit. Purely optional — the impact is marginal.

8. [P3] Long-term direction: let MetaCache.namesCache be the single source of truth

MetaCache.namesCache already holds (remote, local) pairs with refreshAfterWrite semantics. lowerCaseToTableName is essentially a secondary index built solely for case-insensitive lookups. A future refactor could fold the case-insensitive lookup into MetaCache itself — for example, a MetaCache.getRemoteByLowerCase(name) that derives the answer from the current namesCache content (or a derived index built at load time). That would collapse two caches into one and eliminate the cross-cache synchronization problem entirely. This is a follow-up topic, not in scope for this PR.

---

Summary

The PR correctly identifies and fixes a real race in the lowerCaseToTableName cache. Suggestions 1 and 2 are the most important: extending the fix to register/unregister and adding a regression test would substantially raise confidence. Suggestions 3 and 4 are quality-of-implementation improvements that prevent surprises down the road. Suggestions 5 through 8 are minor polish items that can land separately.

[morningman]

run buildall

[github-actions]

PR approved by at least one committer and no changes requested.

[github-actions]

PR approved by anyone and no changes requested.