visit2rahul opened a new pull request, #4487:
URL: https://github.com/apache/polaris/pull/4487

   ## Summary
   
   Fixes the concurrent-`onNext()` race in 
`InMemoryBufferEventListener.processEvent` that issue #2568 was originally 
opened for.
   
   The per-realm `UnicastProcessor` is not safe for concurrent `onNext()` calls 
— Reactive Streams §1.3 requires `onNext` to be called sequentially and 
Mutiny's `UnicastProcessor` relies on that contract; concurrent emissions can 
silently drop events. Synchronize on the processor in `processEvent` so 
concurrent calls for the same realm serialize on the same underlying processor.
   
   Per @adutra's suggested approach in 
https://github.com/apache/polaris/issues/2568#issuecomment-4477653089.
   
   ## Changes
   
   - `InMemoryBufferEventListener.processEvent` — wrap 
`processor.onNext(event)` in `synchronized (processor)` with an explanatory 
comment citing the spec rule.
   - New `InMemoryBufferEventListenerThreadSafetyTest` — fires 10 threads × 100 
events concurrently against the same realm via a `CountDownLatch` barrier and 
asserts all 1000 events land in the events table. Uses the small-buffer profile 
(size=10) so both the concurrent `onNext` path and the rapid flush path are 
exercised.
   
   ## Note on test reproducibility
   
   The race is hard to reliably trigger via unit test on modern CPUs with 
strong memory ordering — on M-series macOS, even 50 threads × 200 events 
(10,000 events) didn't produce a deterministic failure without the fix. The 
test therefore serves as **regression coverage that documents intent and 
asserts the contract**, rather than a deterministic race trigger. The spec 
violation itself is provable per Reactive Streams §1.3 (acknowledged by @adutra 
on the issue) and the fix is mechanically correct regardless of whether a given 
CI runner happens to hit the window.
   
   ## Scope
   
   Out of scope for this PR (to keep it focused): the related TTL-eviction race 
where the Caffeine `evictionListener` calls `onComplete()` on a processor a 
thread may be concurrently mid-`onNext` on. That's a separate, narrower window 
and warrants its own issue/PR with a different fix shape.
   
   ## Test plan
   
   - [x] `:polaris-runtime-service:test --tests 
'InMemoryBufferEventListenerThreadSafetyTest'` passes
   - [x] `:polaris-runtime-service:test --tests 
'org.apache.polaris.service.events.listeners.inmemory.*'` — all 4 test classes 
pass (BufferSize, BufferTime, Integration, ThreadSafety)
   - [x] `:polaris-runtime-service:spotlessJavaCheck` passes


-- 
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]

Reply via email to