[PR] [fix][fn] Avoid scheduler shutdown deadlock [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari opened a new pull request, #26557:
URL: https://github.com/apache/pulsar/pull/26557

   ### Motivation
   
   [Broker Group 2 in PR #26547](https://github.com/apache/pulsar/actions/runs/34691724745/job/103603479879) hit its one-hour timeout during `PulsarFunctionTlsTest.tearDown`. The uploaded JVM dump reports a Java-level deadlock:
   
   - The test worker holds the `SchedulerManager` monitor inside `close()` and waits for `schedulerLock`.
   - `worker-scheduler-0` holds `schedulerLock` while running `invokeScheduler()` and waits for the same monitor in `getCurrentAvailableWorkers()`.
   
   This can also block function-worker shutdown or loss of leadership when scheduling is active.
   
   ### Modifications
   
   Acquire `schedulerLock` before entering the manager's synchronized block in `close()`, matching the order used by scheduling. Retain mutual exclusion with `initialize()` and keep executor and producer shutdown protected against concurrent scheduling.
   
   Add a bounded regression test that holds the exposed scheduler lock, waits until shutdown queues for it, and checks that membership queries can still finish. It also verifies that the producer remains open until the scheduler lock is released, then closes successfully. Cleanup releases the lock even on failure, so the test detects the inversion without hanging the test JVM.
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   - The new regression test fails on the original implementation with `TimeoutException` while waiting for the membership query.
   - `SchedulerManagerTest` and `LeaderServiceTest` pass with retries disabled: 27 invocations, including ten repetitions of the regression test. The temporary repetition setting was removed afterward.
   - `PulsarFunctionTlsTest` passes both tests with retries disabled, including teardown.
   - `./gradlew quickCheck` passes.
   
   ### Does this pull request potentially affect one of the following parts:
   
   - [ ] Dependencies (add or upgrade a dependency)
   - [ ] The public API
   - [ ] The schema
   - [ ] The default values of configurations
   - [x] The threading model
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] The metrics
   - [ ] Anything that affects deployment
   
   The existing locks and lifecycle serialization are retained; only shutdown's lock acquisition order changes.
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][test] Cover scheduler shutdown membership-lock deadlock [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat merged PR #26557:
URL: https://github.com/apache/pulsar/pull/26557


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org