joewitt commented on PR #11621: URL: https://github.com/apache/nifi/pull/11621#issuecomment-5649431313
looks good. ran full build and tests including IT offload tests. All good. One thing to note/ask about: Before the PR, `offload()` called `stopProcessing()` and did not wait on the returned future. `nifi.flowcontroller.graceful.shutdown.period` applied to FlowController / save-executor shutdown only. The flow-service write lock was still held for terminate, RPG stop, and the queue drain, but not for a bounded processor-stop wait. With the current PR, offload waits on that future for up to `nifi.flowcontroller.graceful.shutdown.period` while still holding the write lock, then always falls through to terminate. The default (10 sec) is fine. A minutes-long value, set for JVM/controller shutdown, now stalls offload under that lock for the entire interval before terminate. Should we consider documenting that coupling as the intended reuse (as the admin-guide tweak does), or a dedicated offload timeout so a long graceful-shutdown period cannot stretch the offload critical section? I think i'm ok either way and would merge but want to check. -- 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]
