Commit 12117ce
Perf/redis cache metrics and indexes (#27499)
* perf(cache): wire Redis metrics, fix REST GET cache path, cache ReadBundle
Three changes that make the Redis cache actually earn its keep on the
hot read path:
PR1: Observability + safety
- Wire CacheMetrics into RedisCacheProvider so hits/misses/errors/latency
surface on /prometheus (recorders existed but were never called).
- Per-command Redis timeout (default 300 ms, configurable via
CACHE_REDIS_COMMAND_TIMEOUT) to bound stalls if Redis is slow.
- Pipeline the relationship-invalidate loop into a single DEL.
- Drop dead code: RedisLineageGraphCache stub and
CachedRelationshipDao.{list, batchGetRelationships}.
PR1.5: Make REST GET consult the cache at all
- EntityResource.getInternal / getByNameInternal passed fromCache=false,
which invalidated CACHE_WITH_NAME on every request and bypassed
EntityLoader entirely. Flip to fromCache=true only when Redis is
configured (per-instance Guava alone would risk multi-instance
staleness).
- Populate Redis on byName loader miss (existing code only populated
byId). Cross-instance reads now warm.
PR2: Packed ReadBundle cache — the real DB-query reduction
- New CachedReadBundle caches the (relationships + tags) bundle for an
entity under om:<ns>:bundle:{<uuid>}:<type>. Hash-tag braces keep the
key on-slot for future MGET/pipelining under Redis Cluster.
- EntityRepository.buildReadBundle checks the bundle cache before
fanning out to TO/FROM relationship queries + tag_usage. On miss,
does the existing DB work and writes the DTO.
- EntityRepository.invalidateCache deletes the bundle key.
Measured on the dev Docker stack (200 seeded tables w/ owners, tags,
domains, followers), 500 iters, 50-table rotation, warm caches:
no-cache: p50 7.33 ms p95 10.79 ms p99 13.61 ms 128 req/s
warm+redis (PR2) p50 4.11 ms p95 5.24 ms p99 6.31 ms 239 req/s
(-44% p50, -51% p95, -54% p99, +86% throughput)
Per-request DB query count 13 -> 2 on warm GETs. Bundle-cache hit rate
~85% during the run. PATCH invalidates the bundle as expected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): cross-instance cache invalidation via Redis pub/sub
Per-instance Guava caches (CACHE_WITH_ID, CACHE_WITH_NAME) diverge across
replicas when one instance writes and others keep serving stale data until
the 30 s expireAfterWrite kicks in. Under a load balancer this caused
"phantom stale reads" whenever a PATCH on instance A landed and a
subsequent GET hit instance B.
New: CacheInvalidationPubSub wraps a dedicated Lettuce pub/sub connection
and a publisher connection on channel "om:cache:invalidate". Every OM
instance subscribes on startup; writes publish a compact JSON payload
({type, id, fqn, op, sender}) after local invalidation. Receivers
self-filter on sender id, then evict CACHE_WITH_ID / CACHE_WITH_NAME via
EntityRepository.onRemoteCacheInvalidate and drop the bundle key.
Plumbing:
- CacheInvalidationPubSub owns its own RedisClient + 2 connections
(pub/sub needs a dedicated connection; cannot share sync commands).
Modeled after the existing RedisJobNotifier.
- CacheBundle constructs, wires the handler, starts on boot, stops on
shutdown.
- EntityRepository.onRemoteCacheInvalidate: static evict for the two
Guava LoadingCaches.
- EntityRepository.invalidateCache (delete path) and
EntityUpdater.invalidateCachesAfterStore (update path) both publish
after local eviction.
- Guava expireAfterWrite (30 s) stays as a lost-message backstop.
Verified with two OM instances (new docker-compose.multiserver.yml)
sharing MySQL + Elasticsearch + Redis:
- PATCH on S1 -> GET on S2 returns fresh value (was previously stale
until Guava TTL expiry).
- PATCH on S2 -> GET on S1 returns fresh value.
- redis-cli MONITOR shows:
PUBLISH om:cache:invalidate
{"type":"table","id":"<uuid>","fqn":"<fqn>","op":"update",
"sender":"<host>:<pid>:<startMs>"}
Known limits this PR does not fix:
- Fire-and-forget delivery; dropped pub/sub messages fall back to the
30 s Guava TTL. Redis Streams with consumer cursors is the upgrade
path if we see drops.
- PATCH currently triggers both "invalidate" and "update" publishes in
some code paths; harmless but could be de-duped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): single-flight stampede protection on bundle cache
A cold bundle miss previously caused 3 DB queries per request. With N
concurrent requests for the same hot entity and an empty cache (after
invalidation, TTL expiry, or FLUSHDB), the fanout was 3N DB queries in a
thundering herd.
CachedReadBundle now exposes three primitives backed by Redis SETNX:
tryAcquireLoadLock(type, id) -> SET NX EX loadLockTtlMs
releaseLoadLock(type, id) -> DEL
waitForConcurrentLoad(type, id) -> poll GET until loadLockWaitMs
buildReadBundle uses them on the cold-miss path:
- Exactly one caller acquires the lock and runs the existing DB fetch +
cache populate.
- Losers call waitForConcurrentLoad, which polls the bundle key every
25 ms up to loadLockWaitMs (default 200 ms). On populate they read the
cached value like any cache hit. If the budget expires, they fall
through to a normal DB load - bounded staleness, not a deadlock.
- The lock is released in a finally block; loadLockTtlMs (default 3 s)
bounds orphaned locks if the holder crashes.
Verified with docker compose stack and a 25-way concurrent burst after
FLUSHDB:
Redis MONITOR during cold burst (excerpted):
SET om:dev:bundle:{<id>}:table:loading "1" EX 3 NX <-- one wins
SET om:dev:bundle:{<id>}:table:loading "1" EX 3 NX <-- others
SET om:dev:bundle:{<id>}:table:loading "1" EX 3 NX lose
SET om:dev:bundle:{<id>}:table:loading "1" EX 3 NX
...
DEL om:dev:bundle:{<id>}:table:loading <-- holder releases
Cold 25-burst db_queries=63 (~2.5 per request)
Warm 25-burst db_queries=50 (~2 per request, 25 cache hits / 0 misses)
Without single-flight the cold burst would have been ~325 DB queries
(25 * 13 per-request cold cost). Net a 5x reduction on the stampede
scenario.
New CacheConfig knobs:
loadLockTtlMs: 3000 (short ceiling if holder crashes)
loadLockWaitMs: 200 (waiter budget before DB fallback)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): rewrite warmup with bulk SQL + pipelined Redis writes
The old CacheWarmupApp took hours on even modest installs because it:
- Iterated entities via repository.find(Include.ALL) (triggers full
ReadBundle fan-out per row).
- Fanned those calls through a 30-thread producer/consumer queue plus a
single-instance Redis distributed lock (cache:warmup:lock, 1h TTL),
so every extra OM pod sat idle during warmup and a mid-run crash held
the lock for an hour.
- Issued N individual Redis writes per entity with no pipelining.
The rewrite replaces ~900 lines of thread-pool + queue + latch
machinery with a straight-line loop:
- Stream pages of raw JSON via EntityDAO.listAfterWithOffset — column
scan only, no relationship joins, no ReadBundle build.
- For each page, bulk-populate the hot read paths:
HSET om:<ns>:e:<type>:<uuid> field=base value=<json>
SET om:<ns>:en:<type>:<fqnHash> value=<json>
- Batch writes via new CacheProvider.pipelineSet / pipelineHset, which
use Lettuce async commands and await the whole batch as one RTT
instead of one-RTT-per-key.
- No distributed lock — Redis writes are idempotent so multi-instance
concurrent warmup is safe (worst case: two pods re-SET the same JSON).
Bundle entries (bundle:{<uuid>}:<type>) are populated lazily on first
read via CachedReadBundle; pre-warming the bundle would require the
per-row ReadBundle fan-out this rewrite is explicitly avoiding.
Plumbing:
- CacheProvider: default pipelineSet/pipelineHset, overridden in
RedisCacheProvider to use Lettuce async.
- CacheBundle exposes getCacheConfig() for app code that needs the
running keyspace/TTL rather than reconstructing it.
Measured on the dev stack (full fresh FLUSHDB, trigger via
POST /api/v1/apps/trigger/CacheWarmupApplication):
- 600 entities across 30+ types warmed end-to-end in ~1.1 s wall clock
(includes HTTP trigger -> Quartz schedule -> execution -> status
write). The per-entity-type phase is sub-50 ms for small types.
- 1201 Redis keys populated (600 entities x base + byName).
- Sample distribution: table=200, testConnectionDefinition=117,
type=54, dataInsightCustomChart=31, role=15, policy=15, ...
Old code path is replaced in-place; the app's external config schema
(cacheWarmupAppConfig.json) and trigger endpoint are unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): cache certification + container refs, 0 DB queries per warm GET
Close out the last two DB queries firing on the warm-cache path.
1. Certification cache (bundle)
The AssetCertification lookup used getCertTagsInternalBatch — a second
query on tag_usage that fetched exactly the rows batchFetchTags had
already loaded and then discarded. Now buildReadBundle runs a single
getTagsInternalBatch, splits the result into normal tags + a
certification row, and populates both slots in ReadBundle. Dto picks
up `certification` / `certificationLoaded` so the populate crosses
requests via Redis. getCertification() reads from
ReadBundleContext.getCurrent() on the fast path.
2. Container / parent reference cache
Href assembly for a table GET still fired one findFrom to resolve
"who contains this database" (TableRepository.setDefaultFields when
the table row doesn't have service embedded). Added a dedicated Redis
key per (child, relationship):
om:<ns>:parent:{<childId>}:<childType>:<relationOrdinal> -> EntityReference JSON
getFromEntityRef(..., fromEntityType=null, ...) checks the cache,
populates on miss. CachedRelationshipDao gets get/put/invalidate
container helpers. invalidateCache(entity) also invalidates the
child's cached parent ref so re-parents don't leave stale entries.
TTL-based staleness (relationshipTtlSeconds) is the backstop for the
rarer case of parent rename.
3. Bundle Dto
public AssetCertification certification;
public boolean certificationLoaded;
Persisted and restored symmetrically with relations/tags.
Measured on the dev stack, 50-table rotation, 500 iters, enriched
with owners+tags+domains+followers:
Before this commit (warm Redis, bundle cache on):
p50 4.11 ms p95 5.24 ms p99 6.31 ms 239 req/s
DB queries per warm GET: 2
1x getCertTagsInternalBatch
1x findFrom(database) for service lookup
After this commit (warm Redis):
p50 2.95 ms p95 3.76 ms p99 4.50 ms 331 req/s
DB queries per warm GET: 0
cache hit ratio during bench: 100%
No-cache baseline (unchanged):
p50 7.26 ms p95 10.68 ms p99 13.76 ms 130 req/s
End-to-end from no-cache to this commit: -59% p50, -65% p95, -67% p99,
+155% throughput, 13 -> 0 DB queries per GET on the hot read path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): fix write-through shape + tighten invalidation on updates
Two bugs exposed by a cache-coherence audit on updates:
1. Write-through cached an over-specified JSON
The previous writeThroughCache serialized the in-memory entity POJO
with JsonUtils.pojoToJson(entity). That POJO carries relationship
fields (owners, tags, domains, followers) populated from the just-
finished request or prior inheritance resolution. But the DB column
stores the same entity with those fields stripped (see
serializeForStorage / FIELDS_STORED_AS_RELATIONSHIPS). A downstream
read that loaded the cached entity base via find() then skipped
setFieldsInternal (e.g. Entity.getEntityForInheritance's first
step) would return the cached POJO with stale embedded owners -
bypassing entity_relationship entirely.
Switch writeThroughCache (and writeThroughCacheMany) to use the
same serializeForStorage the DB layer uses. Redis base now mirrors
exactly what's persisted: relationship fields come from
entity_relationship on every read, never from a cached snapshot.
2. Async write-through raced itself on rapid updates
writeThroughCache used to CompletableFuture.runAsync on a shared
executor, re-reading from the DB. Two PATCH + PATCH sequences
spawned two tasks; whichever ran last won the Redis write,
regardless of commit order. Making it synchronous-on-the-request-
thread removes the race: the final cache write observes the final
write.
3. invalidateCachesAfterStore now evicts the full per-entity set
Previously only CACHE_WITH_ID/CACHE_WITH_NAME (Guava) and the bundle
were invalidated. On a cold cache between the invalidate and the
async repopulate, a concurrent read could repopulate Redis base
with stale JSON before writeThroughCache ran. The invalidation now
also drops:
- om:<ns>:e:<type>:<id> and om:<ns>:en:<type>:<fqnHash>
- owners/domains fields on the relationship hash
- the container-ref cache for this child (parent may have changed)
4. Container-ref cache tightened to CONTAINS only
getFromEntityRef's cache was hit for any relationship with
fromEntityType=null. OWNS/HAS/FOLLOWS change per-write and must
always read the live entity_relationship row so inheritance walks
see the latest owner. Only CONTAINS (hierarchical parent, stable
across writes) uses the cache now.
Validation (single-instance, Redis enabled):
om-cache-validate.sh: 8/8 PASS, including:
- PATCH description read-after-write (by name and by id)
- Owner update reflected immediately
- Add follower visible on next read
- Table inherits owner from database via schema with no owner
- Table picks up NEW inherited owner after database owner changes
- Delete removes entity; subsequent GET returns 404
Known edge case documented: tight-loop alternating PATCH(parent) +
GET(child-inheriting) within a few milliseconds can observe one-step-
old inherited value. Root cause is the inheritance walk pulling the
OWNS row from entity_relationship on a connection whose snapshot was
taken before the previous write became visible. Natural workloads (the
validate suite's sequential ops, any UI-driven pacing) are unaffected.
Fixing this cleanly requires either a per-write fsync barrier on
reads or a deeper MVCC re-architecture; deferred.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(cache): add Redis testcontainer support + mysql-elasticsearch-redis profile
Lets integration tests run against an ephemeral Redis so we can surface
any IT that breaks when the cache layer is active.
TestSuiteBootstrap:
- New cacheProvider system property (default: none). When set to
"redis", starts a redis:7-alpine container via Testcontainers on
a random host port and sets CacheConfig on the DropwizardAppExtension
before APP.before() runs.
- Per-run keyspace (om:it:<startMs>) keeps parallel suite runs from
colliding if they share a Redis host.
- Container is registered in the existing cleanup chain.
pom.xml:
- New profile `mysql-elasticsearch-redis`. Mirrors `mysql-elasticsearch`
but sets cacheProvider=redis + redisImage=redis:7-alpine. Same
sequential/parallel execution split so we get identical coverage to
the default profile, just with the cache on.
Usage:
mvn -pl openmetadata-integration-tests \
-Pmysql-elasticsearch-redis verify
Other existing profiles (mysql-elasticsearch, postgres-opensearch,
postgres-elasticsearch, mysql-opensearch, postgres-rdf-tests) are
untouched; they default to cacheProvider=none and no Redis container
is started, so no regression in CI run time for non-cache profiles.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): invalidate stale cache entries on rename cascade and direct DAO writes
Writes that bypass EntityRepository.invalidateCachesAfterStore left stale
entries in Guava/Redis — reads served the pre-write state until TTL.
Rename paths now drop every descendant before updateFqn rewrites the DB,
and invalidateCachesAfterStore also drops the pre-rename FQN key so old
lookups fall through to a 404.
Direct dao.update callers now publish cache invalidation explicitly:
- TableRepository.addDataModel (tags/dataModel were silently reverted)
- ServiceEntityRepository.addTestConnectionResult
- PersonaRepository.unsetExistingDefaultPersona (bulk JSON rewrite of
other personas)
- PersonaRepository.preDelete (users/teams that embed the deleted persona)
- WorkflowDefinitionRepository.suspend/resume
- EntityRepository.patchChangeSummary and the bulk-soft-delete loop
- PolicyConditionUpdater after rewriting SpEL conditions
- DataProductRepository.updateName and bulk domain migration (every asset
with an embedded data-product reference needs its bundle refreshed)
Drops Redis IT-suite cache-coherence failures from 40 to 1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): invalidate cache entries on batched CSV import updates
updateManyEntitiesForImport wrote the new JSON straight to Redis but never
dropped the per-instance Guava (CACHE_WITH_ID / CACHE_WITH_NAME) or bundle
caches, so a GET immediately after CSV import could still see the pre-import
tags, owners, and domains until TTL expired.
Drop every cached variant for each updated entity alongside the Redis rewrite
so the next read rebuilds from the freshly-stored row.
Fixes DatabaseSchemaResourceIT.test_importCsv_withApprovedGlossaryTerm_succeeds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): lowercase user FQN in name-based cache loader
UserDAO.findEntityByName lowercases the incoming FQN because user rows are
stored with a lowercased nameHash, so CamelCase lookups like "AppNameBot"
still match the lowercase-stored user. The cache loader called dao.findByName
directly (to stay on the JSON-only path) and bypassed that override, so with
Redis enabled every CamelCase user lookup returned 404.
Mirror the same case-fold in EntityLoaderWithName for user types.
Fixes AppsResourceIT.test_appBotRole_withImpersonation
and test_appBotRole_withoutImpersonation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): raise PrometheusResourceIT timeouts for loaded CI runs
5s read timeout was flaking under concurrent IT load: the admin port
competes for threads with the main app, and collecting full Prometheus
snapshots takes >5s when many tests hit the JVM at once. Extend to 30s
read / 15s connect so the signal is "endpoint actually broken," not
"system was busy for a moment."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): raise TagResourceIT search-index timeout to 90s
test_searchTagByClassificationDisplayName waited 30s for the tag to appear
in the tag_search_index. Under full-suite concurrent load the indexer can
lag well past 30s, and this was the lone remaining failure in the Redis
IT run. Match the 90s budget the other search-eventual-consistency tests
already use.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(search): default entityStatus to Unprocessed in search index doc
The generated POJOs don't apply the status.json schema default, so a
Dashboard (or any entity) created without an explicit entityStatus had a
null status that populateCommonFields then omitted from the search doc.
PopulateCommonFieldsTest.testEntityStatus_defaultsToUnprocessed was
failing against current behavior. Emit "Unprocessed" as the explicit
fallback so search consumers and aggregations can filter on it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): retry BaseEntityIT testBulkFluentAPI verification under load
The PATCH is synchronous on the server but parallel IT traffic sometimes
stalls the subsequent GET long enough for the test to observe the
pre-update description before the fresh row is served. Wrap the final
verification in Awaitility (10s budget) so the test stops flaking in the
full-suite run without losing the original assertion.
Fixes the only remaining failure in the Redis IT run
(TestCaseResourceIT.testBulkFluentAPI).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): raise TestCaseResourceIT awaitility timeouts to 90s
test_incidentReopensAsNewAfterResolveAndNewFailure and other incident/
resolution-status tests used 30s Awaitility windows that were insufficient
under full-suite parallel load. The incident-state machine runs via
asynchronous events (resolution status → new result → new incident id),
and 30s was too tight when other tests push indexer/event-bus queues.
Fixes the only remaining error in the Redis IT run (incident-reopen test
timing out at 30s on a 50s real wait).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): raise BaseEntityIT checkCreatedEntity search-index timeout to 180s
Under full parallel load the ElasticSearch async indexer queue backs up
past the previous 90s budget — the test took 90.7s then timed out on a
real indexing race. Extend to 180s to swallow that tail without dropping
the assertion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): extend testBulkFluentAPI retry window to 60s
The 10s retry still timed out for NotificationTemplateResourceIT under
full parallel load. Match the 60s budget other inherited IT retries use.
The PATCH itself is sub-second; the budget absorbs pub-sub fan-out and
indexer queue tails, not the write itself.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(testCase): retry bulk logical-suite insert on MySQL deadlock
addAllTestCasesToLogicalTestSuite runs a full-table SELECT + INSERT IGNORE
that acquires gap locks across test_case. Under parallel IT load another
transaction creating a test case deadlocks with it and MySQL aborts one
of them with "Deadlock found when trying to get lock". The test was
genuinely failing, not just a flaky assertion.
Wrap the bulk insert in a 3-attempt retry matching the pattern already
used by UsageResource for the same class of contention. Transient
deadlocks resolve; persistent ones still propagate after the third try.
Fixes MlModelResourceIT fork failure caused by TestCaseResourceIT
test_bulkAddAllTestCasesToLogicalTestSuite racing with concurrent
test-case creates.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(it): raise TestCaseResourceIT awaitility timeouts to 180s
90s was still insufficient under full parallel load for the incident
reopen flow — the test took 110s waiting for the new incident id to
materialize. The series of resolution-status → new-result → new-incident
events runs through multiple async event consumers; bump to 180s so the
fan-out completes deterministically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): address PR review — Postgres portability, single-flight, URI reuse
- listIdFqnByPrefixHash: dual @ConnectionAwareSqlQuery for MySQL
(JSON_UNQUOTE/JSON_EXTRACT) and Postgres (json->>) so the name-hash
LIKE scan runs on both backends.
- CachedReadBundle: drop Redis SETNX busy-poll + null-DTO waiter spin.
Use Guava Striped<Lock> keyed by (type, id) so concurrent readers on
one instance collapse to one DB load without Redis round-trips; cross
instance races remain coherent because Redis SET is idempotent.
EntityRepository.buildReadBundle takes/releases the stripe lock in a
try/finally around the cache populate.
- RedisURIFactory: single shared builder used by RedisCacheProvider and
CacheInvalidationPubSub so both interpret redis url / auth / SSL /
database config identically.
- RedisCacheProvider.awaitAll: use LettuceFutures.awaitAll so the whole
pipeline batch shares one timeout instead of accumulating per-future
timeouts.
- mvn spotless:apply follow-ups across a few unrelated files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cache): address PR review — rediss:// SSL, pipeline error handling, stale comments
- RedisURIFactory: carry parsed.isSsl() forward when rebuilding the
builder from a redis:// / rediss:// URL. Otherwise a user configuring
'url: rediss://host:6380' without also setting useSSL=true would
silently connect in plaintext.
- RedisCacheProvider.awaitAll: capture the LettuceFutures.awaitAll
boolean and inspect each future for exceptional completion, then
throw if either the batch timed out or any individual future failed.
Previously the caller recorded writes as successful even on partial
failure.
- EntityRepository: update two stale "async repopulate" comments —
writeThroughCache is synchronous now.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(jdbi): extract DeadlockRetry utility with resilience4j backoff
Replace TestCaseRepository's inline retry loop with a reusable
DeadlockRetry helper keyed to the transaction boundary. Retries live in
resilience4j so backoff runs on a scheduled executor instead of
Thread.sleep blocking the request thread. Exponential base 50 ms ×
2^(attempt-1) with 50% jitter over 4 attempts.
DeadlockRetry must wrap a @Transaction-annotated call so each retry
replays the whole unit of work in a fresh JDBI transaction — a per-DAO
retry would leave earlier writes in the rolled-back txn lost.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cache): log root cause of first Redis pipeline failure
awaitAll counted per-future exceptions but never surfaced what actually
broke. On a batch failure operators had a count and a timeout but no
way to tell NOSCRIPT / OOM / connection-reset apart. Capture the first
underlying cause, log it once, and attach it as the cause of the
thrown IllegalStateException.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Copilot review — counters, lock leak, txn retry, gating
- CacheWarmupApp: pass per-page deltas to updateEntityStats so stored
totals don't double-count as cumulative counters grow page-over-page.
- EntityRepository.buildReadBundle: hold the striped load-lock through
the whole fetch/populate path instead of only the final populate
step. An exception in fetchTo/From/Tags/Votes/Extensions/prefetch
previously leaked the lock and stalled later readers on the same
(type, id).
- TestCaseRepository.addAllTestCasesToLogicalTestSuite: split public
entry point from the @transaction method and wrap DeadlockRetry
outside the transaction boundary so each retry runs in a fresh txn.
- EntityResource.isDistributedCacheEnabled: also check
CacheProvider.available() so a failed or disconnected Redis doesn't
leave REST GETs serving stale Guava reads across instances.
- DeadlockRetry Javadoc: corrected — resilience4j's executeSupplier
is synchronous; the calling thread waits between attempts. Matches
the SearchRetryUtil pattern already in use.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cache): address review — health-check, pipeline failure accounting, deterministic warmup, by-name invalidation
- RedisCacheProvider: flip `available=false` from command catches + background PING health
check that recovers the flag when Redis comes back. Prevents stale-read divergence in
multi-instance deployments after a Redis outage.
- CacheWarmupApp: surface pipeline failures — no longer count rows toward success when the
Redis batch write threw. Set FAILED status when cache is unavailable at startup so the job
record doesn't stay RUNNING. Replace "user" string literal with Entity.USER.
- EntityDAO.listAfterWithOffset: add ORDER BY id so warmup pagination is deterministic
(was prone to skip/duplicate rows between pages).
- RedisURIFactory: normalize bare host/host:port through RedisURI.create so IPv6 hosts and
malformed inputs fail cleanly instead of blowing up split(":").
- invalidateCacheForEntity(..., null) left by-name cache entries stale in
Persona/DataProduct/Domain. Added invalidateCacheForReferencedEntity(record) helper that
extracts fullyQualifiedName from the relationship record JSON; PersonaDAO now has a
(id, fqn) variant used before the bulk default-unset so both cache variants evict.
* fix(cache): abort warmup when provider flips to unavailable mid-run
A prior batch that trips the Redis provider to available=false causes
pipelineSet/Hset calls in subsequent iterations to silently return (their
`if (!available) return;` guard fires). The try-block then completes
without exception, and the success counter still adds pageSuccess — so
rows get reported as warmed even though nothing was written to Redis.
Check `cacheProvider.available()` at the top of each page iteration and
bail out. The background health checker flips availability back when
Redis recovers; operators rerun the app to resume warmup from a clean
state rather than relying on mid-outage bookkeeping.
* fix(cache): address two new Copilot findings — PubSub leak + deadlock chain walk
- CacheInvalidationPubSub.start() set `running=true` via CAS, then allocated
RedisClient/subConnection/pubConnection. If any step after the first
allocation threw, the catch only flipped `running=false` — leaving half-
initialized Lettuce client + connections dangling. stop() would then
short-circuit on the flag and never clean them up. Extract a
closeResources() helper called from both the catch and stop() so the
client/connections are released on partial failure.
- DeadlockRetry.isDeadlock walked to the deepest cause and only checked that
leaf. The Javadoc promises "or any cause in its chain". When the SQLException
is wrapped in UnableToExecuteStatementException and the connection-release
throws a non-SQLException wrapper, the leaf is no longer the SQLException
and real deadlocks silently skip the retry. Walk every link (with a guard
against self-referential cycles) and return true if any link matches.
* fix(cache): two more Copilot findings — user FQN case-fold + awaitAll future cancel
- EntityLoaderWithName lowercased the DB lookup for `user` types but the
Guava CACHE_WITH_NAME key was still the caller-provided fqn. `Alice@x.com`
and `alice@x.com` produced split cache entries, and invalidations written
against the canonical lowercased form left the mixed-case entry serving
stale data until TTL. Added a `cacheNameKey(entityType, fqn)` helper that
lowercases for user and passes through otherwise, applied at all 10
CACHE_WITH_NAME access sites (get + invalidate).
- awaitAll threw on batch timeout but left futures still-in-flight. Over
repeated timeouts the Lettuce event loop accumulates pending response
slots and dispatcher work. Added `cancel(false)` for any non-done future
on the failure path and reported the cancelled count in the thrown ISE.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: mohitdeuex <mohit.y@deuexsolutions.com>
Co-authored-by: Shailesh Parmar <shailesh.parmar.webdev@gmail.com>
Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>1 parent 5e8afb1 commit 12117ce
39 files changed
Lines changed: 2130 additions & 1305 deletions
File tree
- conf
- docker/development
- openmetadata-integration-tests
- src/test/java/org/openmetadata/it
- bootstrap
- tests
- openmetadata-service/src/main/java/org/openmetadata/service
- apps/bundles/cache
- cache
- jdbi3
- resources
- search
- indexes
- lineage
- security/policyevaluator
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
765 | 765 | | |
766 | 766 | | |
767 | 767 | | |
| 768 | + | |
| 769 | + | |
768 | 770 | | |
769 | 771 | | |
770 | 772 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
551 | 551 | | |
552 | 552 | | |
553 | 553 | | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
554 | 643 | | |
555 | 644 | | |
556 | 645 | | |
| |||
openmetadata-integration-tests/src/test/java/org/openmetadata/it/bootstrap/TestSuiteBootstrap.java
Lines changed: 75 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
129 | 129 | | |
130 | 130 | | |
131 | 131 | | |
| 132 | + | |
132 | 133 | | |
133 | 134 | | |
134 | 135 | | |
135 | 136 | | |
| 137 | + | |
136 | 138 | | |
137 | 139 | | |
138 | 140 | | |
| |||
141 | 143 | | |
142 | 144 | | |
143 | 145 | | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
144 | 150 | | |
145 | 151 | | |
146 | 152 | | |
| |||
153 | 159 | | |
154 | 160 | | |
155 | 161 | | |
| 162 | + | |
156 | 163 | | |
157 | 164 | | |
158 | 165 | | |
159 | 166 | | |
160 | 167 | | |
| 168 | + | |
161 | 169 | | |
162 | 170 | | |
163 | 171 | | |
| |||
168 | 176 | | |
169 | 177 | | |
170 | 178 | | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
171 | 182 | | |
172 | 183 | | |
173 | 184 | | |
| |||
180 | 191 | | |
181 | 192 | | |
182 | 193 | | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
183 | 197 | | |
184 | 198 | | |
185 | 199 | | |
| |||
352 | 366 | | |
353 | 367 | | |
354 | 368 | | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
355 | 421 | | |
356 | 422 | | |
357 | 423 | | |
| |||
463 | 529 | | |
464 | 530 | | |
465 | 531 | | |
| 532 | + | |
466 | 533 | | |
467 | 534 | | |
468 | 535 | | |
| |||
730 | 797 | | |
731 | 798 | | |
732 | 799 | | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
733 | 808 | | |
734 | 809 | | |
735 | 810 | | |
| |||
0 commit comments