Fixes 33538: evict both name aliases in one DEL; stop pinning SystemRepository at class load

[harshach]

Describe your changes:

Fixes #33538. Salvaged from the closed PR #33248 (tracking: #33528).

Two independent papercuts in the same area.

1. A name-keyed cache entry is evicted with two round trips. CachedEntityDao.invalidateByName
and deleteByName each issued two DELs — one for the entity alias, one for the reference alias.
Between them a concurrent reader can see one alias evicted and the other still live, and cache a view
of the entity assembled from both halves. CacheProvider.del is already del(String... keys), so
this is one call.

2. SearchSettingsMergeUtil pinned SystemRepository at class-load time.

private static final SystemRepository systemRepository = Entity.getSystemRepository();

SystemRepository's constructor ends with Entity.setSystemRepository(this), and callers construct
one freely — OpenMetadataApplication:315, OpenMetadataOperations:3416, the @Repository scan in
Entity.initializeRepositories, and EmailUtil:440 on every use. So the global is replaced
repeatedly over a JVM's life while this field keeps whichever instance was current when the class
first loaded — and it is a migration utility, loaded at whatever point the first migration touches it.

On severity, honestly: the instances are functionally equivalent as long as they share the same
CollectionDAO, so this is a latent correctness hazard rather than a reproducible production
failure. It is worth fixing because it is a service-locator call frozen at class-init, the failure
mode is silent, and any change to bootstrap ordering turns it into an NPE for the life of the
process. It is a concrete instance of #33523.

I deliberately did not take the branch's other edits to these two files: its
EntityRepository.class → EntityPolicy.class classloader swap does not apply to main, and the
rest is an automated comment reflow.

Type of change:

• Bug fix

High-level design:

N/A — two small, independent changes. 6 production lines net.

Tests:

Use cases covered

• Renaming or deleting an entity evicts both of its name-keyed aliases atomically.
• A migration that reads or writes search settings uses the SystemRepository that is registered
  now, not the one that was registered when the utility class happened to load.

Unit tests

Four new tests, all of which fail on main:

CachedEntityDaoAliasEvictionTest (a recording CacheProvider over NoopCacheProvider — no mocks
of our own classes):

• invalidateByNameEvictsBothAliasesInOneCall — on main: expected: <1> but was: <2>
• deleteByNameEvictsBothAliasesInOneCall — on main: expected: <1> but was: <2>
• invalidateByIdStillEvictsTheSingleKeyItOwns — guards against over-collapsing

SearchSettingsMergeUtilTest (@Isolated, swaps the registered repository):

• searchSettingsResolveTheCurrentlyRegisteredSystemRepository — on main it reads back the stale
  repository's Settings
• savingSettingsWritesThroughToTheCurrentlyRegisteredSystemRepository

After the change, 34 tests green across the new classes plus every pre-existing test in
service/cache (EntityCacheBypassTest, ListCountCacheTest, BundleWarmupBatcherTest,
RedisCacheProviderStateMachineTest).

Backend integration tests

Not applicable — no API change.

Ingestion integration tests

Not applicable.

Playwright (UI) tests

Not applicable — no UI change.

Manual testing performed

1. mvn -pl openmetadata-service test -Dtest='SearchSettingsMergeUtilTest,CachedEntityDaoAliasEvictionTest,...' — 34 tests, BUILD SUCCESS.
2. Same run with both production files checked out from origin/main — 4 failures, exactly the 4 new tests, with the messages quoted above.
3. mvn spotless:apply -pl openmetadata-service — clean.

UI screen recording / screenshots:

Not applicable.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #33538.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: not applicable.
• For UI changes: not applicable.
• I have added tests and listed them above.
• I have added a test that covers the exact scenario we are fixing.

🤖 Generated with Claude Code

Confidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violations remain.

Summary

This PR fixes two lifecycle and consistency hazards in service-layer utilities:

• Evicts both name-based cache aliases through one multi-key deletion.
• Resolves the currently registered SystemRepository at each search-settings read and write instead of retaining a class-load-time instance.
• Adds focused tests for atomic alias eviction and current-repository resolution.

_{Reviews (3) · Last reviewed commit: "Merge branch 'main' into harshach/fix-st..."}

[github-actions]

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

[github-actions]

✅ Playwright Results — workflow succeeded

Validated commit 33a737f0ef397041d7ab73787a65dad38a07dbab in Playwright run 35267076316, attempt 1.

✅ 4471 passed · ❌ 0 failed · 🟡 10 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 1h 2m 32s

⏱️ Max setup 4m 27s · max shard execution 22m 23s · max shard-job elapsed before upload 25m 30s · reporting 18s

🌐 219.67 requests/attempt · 2.23 app boots/UI scenario · 36.41% common-shard skew

Optimization targets still in progress:

• Common shard skew was 36.41% (convergence target: at most 15%).
• Browser traffic was 219.67 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 2.23 per UI scenario (10645 boots / 4779 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard advanced-search-01	130	0	0	0	0	0
✅ Shard chromium-01	165	0	0	0	0	0
🟡 Shard chromium-02	169	0	3	0	0	0
✅ Shard chromium-03	177	0	0	0	0	0
✅ Shard chromium-04	227	0	0	0	0	0
🟡 Shard chromium-05	150	0	1	0	0	0
🟡 Shard chromium-06	189	0	1	0	0	0
🟡 Shard chromium-07	158	0	1	0	0	0
✅ Shard chromium-08	180	0	0	0	0	0
✅ Shard chromium-09	172	0	0	0	0	0
🟡 Shard chromium-10	165	0	1	0	0	0
✅ Shard chromium-11	175	0	0	0	0	0
✅ Shard chromium-12	165	0	0	0	0	0
🟡 Shard chromium-13	200	0	1	0	0	0
✅ Shard chromium-14	179	0	0	0	0	0
✅ Shard chromium-15	182	0	0	0	0	0
✅ Shard chromium-16	218	0	0	1	0	0
🟡 Shard chromium-17	210	0	1	0	0	0
✅ Shard chromium-18	153	0	0	0	0	0
✅ Shard chromium-19	199	0	0	0	0	0
✅ Shard chromium-20	147	0	0	0	0	0
✅ Shard chromium-21	167	0	0	0	0	0
✅ Shard chromium-22	163	0	0	0	0	0
✅ Shard data-asset-rules-01	65	0	0	0	0	0
✅ Shard domain-isolation-01	16	0	0	0	0	0
✅ Shard global-state-01	34	0	0	0	0	0
✅ Shard import-export-01	80	0	0	0	0	0
✅ Shard import-export-02	70	0	0	0	0	0
🟡 Shard ingestion-01	42	0	1	0	0	0
✅ Shard ingestion-02	55	0	0	0	0	0
✅ Shard reindex-01	28	0	0	0	0	0
✅ Shard search-01	12	0	0	0	0	0
✅ Shard search-rbac-01	29	0	0	0	0	0

🟡 10 flaky test(s) (passed on retry)

• Features/UserProfileOnlineStatus.spec.ts › Should show online status badge on user profile for active users (shard chromium-02, 1 retry)
• Features/UserProfileOnlineStatus.spec.ts › Should show "Active recently" for users active within last hour (shard chromium-02, 1 retry)
• Features/UserProfileOnlineStatus.spec.ts › Should not show online status for inactive users (shard chromium-02, 1 retry)
• Features/CustomizeDetailPage.spec.ts › API Endpoint - customization should work (shard chromium-05, 1 retry)
• Pages/EntityDataConsumer.spec.ts › Tier Add, Update and Remove (shard chromium-06, 1 retry)
• Flow/AddRoleAndAssignToUser.spec.ts › Verify assigned role to new user (shard chromium-07, 1 retry)
• Pages/UserDetails.spec.ts › Admin user can edit teams from the user profile (shard chromium-10, 1 retry)
• Pages/ExplorePageRightPanel.spec.ts › Should perform CRUD and Removal operations for database (shard chromium-13, 1 retry)
• Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard chromium-17, 1 retry)
• Features/IncidentManager.spec.ts › Complete Incident lifecycle with table owner (shard ingestion-01, 1 retry)

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[gitar-bot]

Code Review ✅ Approved

Consolidates name-alias cache eviction into a single Redis deletion and resolves SystemRepository at each search-settings access instead of pinning it at class-load time. Both fixes address correctness hazards in cache consistency and service-locator initialization, with four new unit tests confirming the changes work as intended.

Review coverage

Rules No rules evaluated

Functional validation Not enabled · Set up

Auto-approval Not enabled · Set up

Options

Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[manerow]

LGTM