fix(amber, v1.2): persist operator port result URIs through a ClientEvent - #7102
Closed
Yicong-Huang wants to merge 1 commit into
Closed
Conversation
…pache#5434) ### What changes were proposed in this PR? `WorkflowExecutionsResource.insertOperatorPortResultUri` was a static method on a JAX-RS resource that the engine (`RegionExecutionManager`, named `RegionExecutionCoordinator` before apache#6123) called directly to write `operator_port_executions` rows — two layering problems in one helper: a JAX-RS resource that wasn't an HTTP endpoint, and an engine actor reaching across into the web layer to write the DB. The fix follows the codebase's existing engine→web channel. Engine actors call `asyncRPCClient.sendToClient(event: ClientEvent)`; web services call `client.registerCallback[T](handler)` to react. `RuntimeStatisticsPersist` is the closest precedent — engine emits, `ExecutionStatsService` writes the DB. Concretely: - Add `OperatorPortResultUriAvailable(globalPortId, uri)` to `ClientEvent`. The event is execution-scoped, so it carries no `eid` — the subscriber already holds its own `executionId`, which also removes the old `decodeURI(resultURI)` round-trip in `RegionExecutionManager`. - `RegionExecutionManager` emits this event in place of calling the web helper. - `ExecutionResultService.attachToExecution` registers a callback that inserts the row via the generated `OperatorPortExecutionsDao` (extracted as `ExecutionResultService.persistOperatorPortResultUri` so tests can drive the same insert). The registration site carries a comment documenting the ordering invariant: the callback must be attached before `startWorkflow`, otherwise result URIs are silently never persisted. - Delete `WorkflowExecutionsResource.insertOperatorPortResultUri`. The engine no longer imports `WorkflowExecutionsResource`; the resource spec inserts its fixture rows through `OperatorPortExecutionsDao` directly. - E2E specs (`DataProcessingSpec`, reconfiguration specs) register the same production callback body via `TestUtils.registerResultUriPersistence` and read results back through `getResultUriByLogicalPortId`, so the event→DB-insert seam stays covered end to end. ### Any related issues, documentation, discussions? Closes apache#5430. Sub-issue of apache#5424 — eliminates one of the four engine→web import leaks blocking the amber Dropwizard upgrade tracked in apache#5423. ### How was this PR tested? `sbt 'WorkflowExecutionService/testOnly *ExecutionResultServiceSpec *WorkflowExecutionsResourceSpec *DataProcessingSpec *ReconfigurationSpec'` — 58 tests pass across the 4 affected suites. `sbt scalafmtCheckAll` clean. ### Was this PR authored or co-authored using generative AI tooling? (backported from commit e42f6b3) Generated-by: Claude Code (Opus 4.7)
Contributor
Author
|
The cherry-pick conflicted and was committed with conflict markers. Resolve the conflicts on this branch, then mark this PR ready for review. Conflicting files:
|
Contributor
Automated Reviewer SuggestionsBased on the
|
|
Contributor
Author
|
this is more of a refactoring, closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Backport of #5434 to
release/v1.2, cherry-picked from e42f6b3.Follows the Direct Backport Push convention; opened as a PR (rather than a direct push) as part of a backport-coverage audit for fixes merged to
mainsince early June that were never labeled for backport.Any related issues, documentation, discussions?
Backport of #5434. Originally linked #5430.
How was this PR tested?
Release-branch CI runs once the conflicts are resolved and this PR is marked ready for review. The cherry-pick conflicted and was committed with conflict markers.
Was this PR authored or co-authored using generative AI tooling?
Yes — backport prepared with Claude Code (mechanical cherry-pick; conflicts left as markers for the original author to resolve; the change itself is #5434 by its original author).