Skip to content

[fix](regression) Deflake test_sql_cache_over_arrow_flight - #67734

Open
morningman wants to merge 1 commit into
apache:masterfrom
morningman:deflake-arrow-flight-sql-cache
Open

[fix](regression) Deflake test_sql_cache_over_arrow_flight#67734
morningman wants to merge 1 commit into
apache:masterfrom
morningman:deflake-arrow-flight-sql-cache

Conversation

@morningman

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #67381

Problem Summary:

arrow_flight_sql_p0/test_sql_cache_over_arrow_flight is flaky. It primes four sql
cache entries through the MySQL control session, runs the same statements over Arrow
Flight, and then asserts the entries are still there, to prove that a flight query
never consumes the cache. That closing block fails intermittently on both master and
branch-4.1 — six times since the suite was added on 2026-09-02, most recently in p0
build 124525, where the select 1 as c, 'x' as s entry was gone 299ms after it had
been primed:

Exception in arrow_flight_sql_p0/test_sql_cache_over_arrow_flight.groovy(line 163):
        assertTrue(hasSqlCache(constantSql))
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>

Root cause. The FE sql cache is a single Caffeine map shared by every session
(NereidsSqlCacheManager.sqlCaches), bounded by Config.sql_cache_manage_num, which
defaults to 100. Caffeine admits a newcomer through a window sized at 1% of that
bound
, so at the default the admission window holds a single entry: a just cached
statement has frequency ~1 and loses the admission contest to an established victim as
soon as any other session caches anything. SessionVariable.enableSqlCache defaults to
true, so the rest of the p0 suite running concurrently against the same FE is already
enough to evict it.

The audit log rules out ordinary LRU pressure: only 15 distinct selects ran cluster
wide during that 299ms window, far fewer than the 100 an LRU would have needed. A local
run against caffeine 3.2.4 reproduces the admission behaviour directly:

maximumSize=100      1 other insert after mine -> survived  7/20
maximumSize=100     15 other inserts after mine -> survived  8/20
maximumSize=10000    1 other insert after mine -> survived 20/20
maximumSize=10000   15 other inserts after mine -> survived 20/20

This is not an FE bug — the sql cache is best effort and gives no retention guarantee.
It is the suite asserting a property the cache does not provide. The other five sql
cache suites in the repo (mv_with_sql_cache, mtmv_with_sql_cache,
parse_sql_from_sql_cache, union_all_compensate, union_rewrite_grace_big) already
raise the bound at their start for exactly this reason; this one was missing it. In p0
build 124525 the failing suites ran at 21:54 and 22:01, before any of those suites
raised the bound at 22:12.

Fix. Raise sql_cache_manage_num to 10000 while the suite runs, and restore the
previous value afterwards. The restore is deliberate: this suite runs about half an
hour earlier in the p0 run than the five existing ones, and the ones that raise the
bound without restoring it left 97k live SqlCacheContext instances in the FE heap
in the same build (post-GC live heap peaked at 4G of 8G in that window).

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01Njd8iDxdqc19QbLdNtZ7Pt

The suite primes four sql cache entries through the MySQL control session, runs
the same statements over Arrow Flight, and finally asserts the entries are still
there, to prove that a flight query never consumes the cache. That last block
fails intermittently: in p0 build 124525 the `select 1 as c, 'x' as s` entry was
gone 299ms after it had been primed, and the suite has failed six times on
master and branch-4.1 since it was added.

The FE sql cache is a single Caffeine map shared by every session, bounded by
Config.sql_cache_manage_num, which defaults to 100. Caffeine admits a newcomer
through a window sized at 1% of that bound, so at the default the window holds a
single entry and a just cached statement is dropped again as soon as any other
session caches anything. enable_sql_cache defaults to true, so the rest of the
p0 suite running concurrently against the same FE is already enough: only 15
distinct selects ran cluster wide during that 299ms window, far fewer than the
100 an LRU would have needed. The other five sql cache suites in the repo
(mv_with_sql_cache, mtmv_with_sql_cache, parse_sql_from_sql_cache,
union_all_compensate, union_rewrite_grace_big) all raise the bound for this
reason; this one was missing it.

Raise it here as well while the suite runs, and put it back afterwards so the
rest of the run does not keep 10000 cached plans and their result rows alive in
the FE heap. The suites that raise it without restoring it left 97k live
SqlCacheContext instances behind in the same build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Njd8iDxdqc19QbLdNtZ7Pt
@hello-stephen

Copy link
Copy Markdown
Contributor

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?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants