wilfred-s commented on PR #1111:
URL: https://github.com/apache/yunikorn-core/pull/1111#issuecomment-5102986381
> There are some tricky race problems stil in the code. I performed a manual
+ AI review and following came out:
>
> 1. Events are dropped silently after a runtime restart. The root cause is
identical in both versions (before / after) and is untouched by the PR:
`Stop()` sets `ec.channel = nil`, and `StartServiceWithPublisher()` never
recreates it. So after any `Stop()`→start cycle (i.e. `Restart()` /
`restart()`), the input channel stays `nil` and events are dropped.
I had not seen that one, fixed now. Added test for dropped events on restart.
> To be fair, the goroutine leak + panics are fixed, but we're still
losing data.
yep that was the focus of what I saw happen.
> 2. **Regression**: config responsiveness is lost after the first restart.
The callback is now registered in `Init()`, but `Stop()` still calls
`RemoveConfigMapCallback()` and start no longer re-adds it. Since `restart()` =
`Stop()` + `StartServiceWithPublisher()`, the callback is removed and never
restored.
The callback should not be removed, fixed now.
> 3. Non-atomic stopped gate. Both start and stop do `Load()` then `Store()`
outside the lock instead of `CompareAndSwap`. Two concurrent `Stop()` calls
(reachable: rapid config updates each spawn `go reloadConfig()` → `restart()` →
`Stop()`) can both pass the gate; the second reaches `ec.stop <- struct{}{}`
after the handler already exited and blocks forever holding `ec.Lock()` - this
means deadlock. Using `stopped.CompareAndSwap(false, true)` /
`CompareAndSwap(true, false)` closes this window. Very unlikely to happen, but
who knows.
I had thought about that but left it as to small a chance. Changed now to
`CompareAndSwap`
--
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]
