rfellows commented on code in PR #11842:
URL: https://github.com/apache/nifi/pull/11842#discussion_r4392713882
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-canvas/connector-canvas.reducer.ts:
##########
@@ -117,9 +121,19 @@ export const connectorCanvasReducer = createReducer(
remoteProcessGroups,
processGroups,
processors,
- connections
+ connections,
+ reconciliationSnapshot,
+ connectedStateChanged
}
) => {
+ const requestedAddedIds = new
Set(reconciliationSnapshot.addedCache);
+ const requestedRemovedIds = new
Set(reconciliationSnapshot.removedCache);
+ const addedCache = connectedStateChanged
Review Comment:
This can discard a tombstone for a move that is still in flight. For example:
1. `moveCanvasComponents` removes the component locally and adds its ID to
`removedCache`.
2. This load captures that cache while the move request is still pending.
3. The GET captures the source group before the move commits.
4. Authoritative reconciliation removes the requested tombstone and restores
the component.
5. `moveCanvasComponentsSuccess` only clears `saving`, so the component
remains visible.
Could we invalidate and retry an authoritative response when a mutation
overlaps it? One relatively simple option is a canvas-wide mutation generation:
capture the generation and `saving` state when the GET starts, increment the
generation on mutation success, and discard/retry the response if saving is
active or the generation changed.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-canvas/connector-canvas.reducer.ts:
##########
@@ -48,15 +48,18 @@ function processComponentCollection<E extends { id: string;
revision: Revision }
proposedComponents: E[],
currentComponents: E[],
addedCache: string[],
- removedCache: string[]
+ removedCache: string[],
+ connectedStateChanged: boolean
): E[] {
const currentById = new Map(currentComponents.map((component) =>
[component.id, component]));
const proposedIds = new Set(proposedComponents.map((component) =>
component.id));
const components = proposedComponents
.filter((component) => !removedCache.includes(component.id))
.map((proposedComponent) => {
const currentComponent = currentById.get(proposedComponent.id);
- return currentComponent &&
!hasCurrentOrNewerRevision(currentComponent.revision,
proposedComponent.revision)
+ return currentComponent &&
Review Comment:
`connectedStateChanged` bypasses the revision guard for every entity,
including entities updated after the GET began. A position, resize, or
connection update can succeed locally, then be replaced by an older entity from
the in-flight GET.
Please preserve mutations completed after the request started, or discard
the stale response and issue another GET. A regression test should cover: start
GET → complete component update → return older authoritative response.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
