nielifeng commented on PR #11458: URL: https://github.com/apache/seatunnel/pull/11458#issuecomment-4989049507
Reviewed the diff and traced the call paths on `dev`. The approach looks solid to me: - `getReusableSlot` doesn't blindly trust the persisted `ownedSlotProfilesIMap` entry — it confirms with `resourceManager.slotActiveCheck(slotProfile)` that the worker still holds the matching slot sequence before reusing. That's the right guard against a stale slot record after failover. - The cleanup filter `!reusedSlotProfiles.contains(slotProfile)` correctly preserves reused slots and only releases newly-applied ones. I checked `SlotProfile.equals/hashCode` (slotID + worker + sequence) — identity is stable, so `contains` is reliable here. - The regression test genuinely exercises the fix: it exhausts all free fixed slots (`assertEquals(0, getUnassignedSlots)`) before restore, so the old code would leave the job pending and only the reuse path lets it recover. `dynamicSlot=false` + fixed `slotNum` targets the right scenario. I also double-checked the multi-pipeline concern: master failover recovery goes through `CoordinatorService#pendingJobSchedule` → `preApplyResources()` (no-arg, all pipelines in one call), and `masterFailoverRestore` is only cleared after that whole call succeeds — so a JobMaster-level flag covers all pipelines correctly. The per-SubPlan `preApplyResources(this)` path is normal-operation pipeline restart where reuse is intentionally skipped. So no issue there. One minor suggestion (non-blocking): `masterFailoverRestore` is a plain `boolean`. It's written in `init()` and in `preApplyResources`, and read from the per-SubPlan path which runs on `executorService` threads. For consistency with the adjacent `volatile boolean needRestore`, it might be worth marking it `volatile` to avoid any visibility gap. Low priority given the coordinator-thread write happens well before pipeline restarts. Nice fix overall. -- 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]
