Fixes 30119: Stabilize integration-test CI#30120
Conversation
github-actions
Bot
added
backend
safe to test
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Too many files changed for review. ( Bypass the limit by tagging |
|
Code Review 👍 Approved with suggestions 9 resolved / 10 findingsStabilizes integration tests through deterministic outcome classification and robust cache consistency, while addressing search retry races and relationship revision fencing. Consider addressing the potential for stuck states in the context file extraction service due to retry exhaustion. 💡 Edge Case: Embedding generation failure indexes doc without vectorWhen an entity's state has changed (no reusable cached embedding), ✅ 9 resolved✅ Bug: Unbounded conditional-update retry can spin indefinitely
✅ Edge Case: failedRecords==0 branch ignores recorded PROCESS-stage failures
✅ Bug: Advisory lock ties up a pooled connection, risks pool starvation
✅ Bug: Session advisory lock can leak to pooled connection on release failure
✅ Performance: Postgres lock key hashing can over-serialize distinct suites
...and 4 more resolved from earlier reviews 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Describe your changes:
Fixes #30119
This change makes Java, Playwright, and Python integration-test outcomes deterministic without reducing parallel throughput.
It builds each Java runtime once before four concurrent isolation lanes, classifies assertion, setup, infrastructure, missing-report, cancellation, and hung-test outcomes, and retains fail-closed diagnostics.
It also fixes hard-delete cache consistency, stale context/search updates, bulk-update fallback ordering, restore/test-suite update races, and retry-dependent Oracle, Trino, and Playwright readiness checks.
The Playwright zero-retry policy remains shadow-only while stable retry signatures are collected.
Type of change:
High-level design:
parallel,global-state,multi-node, andretry-queueFailsafe lanes with unchanged required check namesTests:
Use cases covered
Unit tests
TestCaseRepositoryTestandSearchRepositoryTest: 22 passedBackend integration tests
HardDeleteCacheConsistencyITMissing fenced relationship update; fixed behavior passed 1/1 against MySQL and ElasticsearchIncludemode under parallel workersPython integration tests
Ingestion integration tests
Playwright (UI) tests
playwright-summarypassedManual testing performed
actionlinton all 16 changed workflowsbash -n, YAML parsing, and targeted Docker-storage helper validationUI screen recording / screenshots:
Not applicable — test synchronization changed, not product UI behavior.
Checklist:
Fixes <issue-number>: <short explanation>.Fixes #<issue-number>above.Greptile Summary
This PR stabilizes Java, Playwright, and Python integration-test CI outcomes by making test outcomes deterministic without reducing parallel throughput. The core backend changes fix hard-delete cache consistency, stale context/search updates, bulk-update fallback ordering, restore/test-suite update races, and retry-dependent readiness checks.
PENDINGrecords; staleness is detected and metrics are emitted for superseded claims.testSuitesRevision/testsRevisioncounters inentity_extensionand a revision-fenced Painless script so concurrent test-case membership changes converge instead of overwriting each other in Elasticsearch/OpenSearch.CachedEntityDao.getBaseto returnOptionaland gates Redis writes behind an epoch equality check, preventing a stale loader from re-populating L1/Redis after a hard-delete or concurrent write.Confidence Score: 5/5
Safe to merge. The claim-token guards, revision-fenced Painless scripts, and propagation-context preservation all work together correctly.
The three findings are quality-of-life concerns rather than defects that produce wrong data. The PR includes 258 focused unit tests, full MySQL/Elasticsearch parallel lane results with zero failures, and targeted regression coverage for every race addressed.
SearchIndexRetryWorker.java(double-write pattern and stability-loop exhaustion path) andSearchRepository.addConfirmedPropagationCandidates(propagation skip on under-reported failures) are the areas worth revisiting before the next increment.Important Files Changed
ScriptedPartialUpdaterecord,RelationshipRevisionSpecfor revision-fenced Painless scripts,updateEntitiesIndexoverload with relationship revisions,enqueueEntityRetrywith propagation context capture, andpropagateEntityAfterRetry. Logic is correct; see comment on propagation being skipped when failure count is under-reported.propagateAfterRetry,loadStableEntitySnapshotwith 3-attempt stability loop, and double-write to bulk sink for entities with relationship revisions.claimTokencolumn tosearch_index_retry_queue, claim-token–guarded delete/update, andincrementRevisionsbatch SQL. Propagation context is preserved on conflict upsert in both MySQL and PostgreSQL dialects.flushInOneTransaction(which retries deadlocks), separates revision increment + snapshot from post-commit search publication, and introducesLogicalSuiteRelationshipChangerecord.PROPAGATION_CONTEXT_MARKER, Base64-encoded propagation context embedding/extraction,preservePropagationContext, andvisibleFailureReason.getBaseto returnOptional<String>(cache-only lookup, no DB fallback). DB fetch and Redis population are now in the caller guarded by an epoch check, eliminating the stale-entry race.addScriptedPartialUpdateandupdateScriptedDocumentDirectly, correctly forwarding oversized-document failures to the failure callback.claimToken VARCHAR(36)via a guarded prepared-statementALTER TABLE, safe for re-execution on an already-migrated schema.claimToken VARCHAR(36) NULLwith an idempotentADD COLUMN IF NOT EXISTS. Clean and safe.markEntityNotFoundto usecacheNameKey-normalized FQN, gates Redis writes behind epoch equality checks, and addsupdateIfCurrentwith optimistic-locking variant.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant TC as TestCaseRepository participant DB as Database participant SR as SearchRepository participant ES as Elasticsearch participant RQ as SearchIndexRetryQueue participant RW as SearchIndexRetryWorker TC->>DB: flushInOneTransaction() incrementRevisions DB-->>TC: revisionN TC->>SR: updateEntitiesIndex(testCases, revisionMap) SR->>ES: bulkSink scripted partial update ES-->>SR: success or failure alt succeeded SR->>SR: propagateEntitiesAfterBulkFlush() else failed SR->>RQ: enqueueWithPropagation(entity, changeDesc) end RW->>RQ: claimPending with claimToken RW->>DB: loadStableEntitySnapshot 3-attempt DB-->>RW: entity + revision RW->>ES: upsertEntitiesInBulk Write1+Write2 RW->>SR: propagateAfterRetry RW->>RQ: deleteClaimed with claimToken%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant TC as TestCaseRepository participant DB as Database participant SR as SearchRepository participant ES as Elasticsearch participant RQ as SearchIndexRetryQueue participant RW as SearchIndexRetryWorker TC->>DB: flushInOneTransaction() incrementRevisions DB-->>TC: revisionN TC->>SR: updateEntitiesIndex(testCases, revisionMap) SR->>ES: bulkSink scripted partial update ES-->>SR: success or failure alt succeeded SR->>SR: propagateEntitiesAfterBulkFlush() else failed SR->>RQ: enqueueWithPropagation(entity, changeDesc) end RW->>RQ: claimPending with claimToken RW->>DB: loadStableEntitySnapshot 3-attempt DB-->>RW: entity + revision RW->>ES: upsertEntitiesInBulk Write1+Write2 RW->>SR: propagateAfterRetry RW->>RQ: deleteClaimed with claimTokenComments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/drive/ContextFileExtractionService.java, line 286-296 (link)Three rapid optimistic-lock conflicts make this helper return
false, after which callers abandon the remaining extraction transition. In the result and failure paths, the context file can remain inAnalyzingindefinitely instead of reaching a terminal or retryable state.Context Used: CLAUDE.md (source)
Reviews (35): Last reviewed commit: "test(drive): isolate search upload fixtu..." | Re-trigger Greptile
Context used: