Skip to content

Pipe: Cache table-model pattern matches by table - #18648

Open
shuwenwei wants to merge 2 commits into
masterfrom
pipe-table-granularity-cache
Open

shuwenwei wants to merge 2 commits into
masterfrom
pipe-table-granularity-cache

Conversation

@shuwenwei

Copy link
Copy Markdown
Member

Summary

  • Cache table-model source matches by database and table name instead of device ID.
  • Match each table only once per event.
  • Keep table-model TsFile table names complete for privilege checks.

Validation

  • git diff --check
  • No compile/build/test run per repository preference.

Cache table-model source matches by database and table name instead of device id.\n\nMatch each table once per event and keep table-model TsFile table names complete for privilege checks.
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.82%. Comparing base (a05c63e) to head (77731ba).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
...ava/org/apache/iotdb/db/auth/AuthorityChecker.java 0.00% 4 Missing ⚠️
...n/realtime/matcher/CachedSchemaPatternMatcher.java 84.61% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18648      +/-   ##
============================================
- Coverage     42.82%   42.82%   -0.01%     
  Complexity      442      442              
============================================
  Files          5451     5451              
  Lines        395425   395430       +5     
  Branches      51805    51808       +3     
============================================
+ Hits         169349   169351       +2     
- Misses       226076   226079       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shuwenwei
shuwenwei requested a review from Caideyipi September 16, 2026 08:01

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this optimization. The table-level cache is a useful direction. I left two inline comments, including one authorization-cache invalidation race that can affect correctness.

Could you add focused coverage for the changed matching behavior as well?

  • Multiple devices from one table should invoke table matching only once.
  • A multi-table TsFile should still record every table after all sources have matched.
  • Ideally, a cache-invalidation/concurrent-refill test should demonstrate that stale authorization results cannot survive an invalidation.

// Use full cache to avoid queue stuck and block insertion
protected final Map<IDeviceID, Set<PipeRealtimeDataRegionSource>> deviceToSourcesCache;
protected final Map<Pair<String, IDeviceID>, Set<PipeRealtimeDataRegionSource>>
protected final Map<Pair<String, String>, Set<PipeRealtimeDataRegionSource>>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Prevent stale authorization results from refilling this cache

AuthorityChecker.invalidateCache() currently clears this matcher before invalidating the authority cache. That leaves a stale-refill window: a concurrent event can miss this cache, read the old authorization result, and cache a table-wide denial here. After the grant invalidation completes, later events can keep hitting that stale entry, remain unmatched, and advance progress until another invalidation or source change.

Please invalidate the authority cache before this matcher (also in invalidateAllCache()), or coordinate the two caches with a generation/version so that an entry computed before invalidation cannot be installed afterward.

return new Pair<>(matchedSources, findUnmatchedSources(matchedSources));
}

final String tableModelDatabaseName =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Resolve the table-model database name only for table events

getTableModelDatabaseName() is evaluated before the model check, so every insertion event, including tree-model events, now performs its lazy substring/toLowerCase initialization on the realtime write path even though the value is only used for table-model matching below. Please move this lookup into the table-model branch.

Invalidate the authority cache before the pipe matcher cache to avoid stale authorization refills.\n\nResolve the table-model database name only for table events and add matcher orchestration coverage.
@shuwenwei

Copy link
Copy Markdown
Member Author

Addressed in 77731ba.

  • P2: reordered cache invalidation so the authority cache is invalidated before the pipe matcher cache in both invalidateCache() and invalidateAllCache(), preventing stale authorization results from being cached back into the matcher.
  • P3: tableModelDatabaseName is now resolved only inside the table-model matching branch, so tree-model insertion events no longer trigger the lazy substring/lowercase work.
  • Added focused matcher tests covering per-table matching deduplication and complete tableNames collection for multi-table table-model TsFile events after all sources have matched.

The testCachedMatcher performance test is unchanged from master; its runtime is pre-existing.

@shuwenwei
shuwenwei requested a review from Caideyipi September 16, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants