Skip to content

PHOENIX-7960 ReplicationLogGroup.close() racing init() can swallow the disruptor halt alert, leaking the writer#2579

Open
tkhurana wants to merge 2 commits into
apache:PHOENIX-7562-feature-newfrom
tkhurana:PHOENIX-7960
Open

PHOENIX-7960 ReplicationLogGroup.close() racing init() can swallow the disruptor halt alert, leaking the writer#2579
tkhurana wants to merge 2 commits into
apache:PHOENIX-7562-feature-newfrom
tkhurana:PHOENIX-7960

Conversation

@tkhurana

Copy link
Copy Markdown
Contributor

A ReplicationLogGroup.close() that immediately follows init() can halt() the disruptor before its consumer thread has entered BatchEventProcessor.run(). run()'s opening clearAlert() then wipes the alert raised by halt(), the wait loop parks forever, and onShutdown() never runs — so the underlying ReplicationLogWriter is never closed.

tkhurana added 2 commits July 21, 2026 15:46
…e disruptor halt alert, leaking the writer

A close() that immediately follows init() could halt() the disruptor before
its consumer thread entered BatchEventProcessor.run(). run()'s opening
clearAlert() then wiped the alert raised by halt(), the wait loop parked
forever, and onShutdown never ran, so the underlying writer was never closed.
This surfaced as a flaky testClose (Mockito "wanted but not invoked:
close()") and latently affects every recreateLogGroup()/tearDown() that
closes a zero-append group right after creation.

Add a startedLatch counted down in LogEventHandler.onStart() and await it
(bounded by shutdownTimeoutMs) in close() before disruptor.shutdown(), so the
consumer is guaranteed past clearAlert() when halt() raises the alert.

Extract createDisruptorExecutor() as a @VisibleForTesting seam and add
testCloseImmediatelyAfterInitClosesWriterWhenConsumerStartsLate, which delays
the consumer start to force halt() into the pre-clearAlert window and verifies
the writer is still closed.
…start/shutdown race test

- Add @test(timeout = 60000) so a regression reintroducing the swallowed
  halt() alert fails fast instead of spinning until shutdownTimeoutMs.
- Correct comment: YieldingWaitStrategy spins, it does not park.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant