mitre88 opened a new pull request, #10170:
URL: https://github.com/apache/gravitino/pull/10170

   ### What changes were proposed in this pull request?
   
   Replace the plain `Instant` field `lastRecordDropEventTime` with 
`AtomicReference<Instant>` in `AsyncQueueListener` to fix a race condition in 
the drop event logging throttle.
   
   ### Why are the changes needed?
   
   Fix: #10169
   
   The `lastRecordDropEventTime` field is read and written by multiple 
concurrent event producer threads (via `enqueueEvent` → 
`logDropEventsIfNecessary`), but it was a non-volatile, unsynchronized 
`Instant` field. This causes two issues:
   
   1. **Visibility**: Without `volatile` or atomic semantics, writes by one 
thread may never be seen by other threads due to CPU caching / Java Memory 
Model.
   2. **Consistency**: A thread could read a stale `lastRecordDropEventTime` 
for the time-gate check, then log an incorrect "since" timestamp.
   
   ### How was this patch tested?
   
   - Added `TestAsyncQueueListenerConcurrency` with a test that spawns 10 
threads each enqueuing 100 events into a capacity-1 queue, exercising the 
concurrent drop path and verifying no exceptions are thrown.
   - Existing tests in `TestEventListenerManager` continue to pass.
   
   ### Changes summary
   
   | File | Change |
   |------|--------|
   | `AsyncQueueListener.java` | `Instant lastRecordDropEventTime` → 
`AtomicReference<Instant> lastRecordDropEventTime`; snapshot to local var 
before time-gate check |
   | `TestAsyncQueueListenerConcurrency.java` | New test for concurrent event 
drop safety |


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