Skip to content

PHOENIX-7984 Fence writer on sync failure to prevent false-success RPO loss - #2596

Open
tkhurana wants to merge 1 commit into
apache:PHOENIX-7562-feature-newfrom
tkhurana:PHOENIX-7984
Open

PHOENIX-7984 Fence writer on sync failure to prevent false-success RPO loss#2596
tkhurana wants to merge 1 commit into
apache:PHOENIX-7562-feature-newfrom
tkhurana:PHOENIX-7984

Conversation

@tkhurana

Copy link
Copy Markdown
Contributor

A SYNC write failure could partially mutate a block before the durability barrier, then a retry on the same writer would false-succeed -- acking a sync that never reached the peer, a silent RPO loss.

Fence the writer on first append/sync IOException (mirroring HDFS DFSOutputStream single-shot semantics): the first fault is latched and every subsequent append/sync fails fast rather than touching the stream. A fenced writer is never re-driven; recovery is by rotating to a fresh writer (new HDFS pipeline on healthy DataNodes) and replaying the unsynced batch.

apply()'s retry obtains that fresh writer through a guarded wait on rotationSignal that re-drives requestRotation() each spin. This fixes the swallowed-request missed-retry: a rotation request coalesced away while a soon-to-complete rotation held the CAS gate is reissued once the gate clears, so a fresh task actually gets scheduled instead of the retry giving up and prematurely downgrading SYNC to STORE_AND_FORWARD. Writer creation stays async on the rotation executor, so the consumer stall is bounded by retryDelayMs and decoupled from standby FS latency. requestRotation() returns a boolean so the waiter exits immediately when rotation is permanently suppressed (failover pending / executor shut down) rather than burning the full budget.

Tests:

  • LogFileWriterSyncTest: writer stays fenced after a sync failure; rejects subsequent append/sync.
  • testNoSameWriterRetryWhenRotationCannotStageWriter: no second sync on the fenced writer when rotation cannot stage a fresh one; flips to SAF and the unsynced record survives onto the SAF writer.
  • testRetryReDrivesRotationAfterFirstRotationFails: regression guard for the re-drive -- a first rotation that fails is retried within the budget instead of downgrading to SAF.

…O loss

A SYNC write failure could partially mutate a block before the durability
barrier, then a retry on the same writer would false-succeed -- acking a sync
that never reached the peer, a silent RPO loss (S17b).

Fence the writer on first append/sync IOException (mirroring HDFS
DFSOutputStream single-shot semantics): the first fault is latched and every
subsequent append/sync fails fast rather than touching the stream. A fenced
writer is never re-driven; recovery is by rotating to a fresh writer (new HDFS
pipeline on healthy DataNodes) and replaying the unsynced batch.

apply()'s retry obtains that fresh writer through a guarded wait on
rotationSignal that re-drives requestRotation() each spin. This fixes the
swallowed-request missed-retry: a rotation request coalesced away while a
soon-to-complete rotation held the CAS gate is reissued once the gate clears,
so a fresh task actually gets scheduled instead of the retry giving up and
prematurely downgrading SYNC to STORE_AND_FORWARD. Writer creation stays async
on the rotation executor, so the consumer stall is bounded by retryDelayMs and
decoupled from standby FS latency. requestRotation() returns a boolean so the
waiter exits immediately when rotation is permanently suppressed (failover
pending / executor shut down) rather than burning the full budget.

Tests:
- LogFileWriterSyncTest: writer stays fenced after a sync failure; rejects
  subsequent append/sync.
- testNoSameWriterRetryWhenRotationCannotStageWriter: no second sync on the
  fenced writer when rotation cannot stage a fresh one; flips to SAF and the
  unsynced record survives onto the SAF writer.
- testRetryReDrivesRotationAfterFirstRotationFails: regression guard for the
  re-drive -- a first rotation that fails is retried within the budget instead
  of downgrading to SAF.
@tkhurana
tkhurana requested a review from apurtell August 11, 2026 23:33
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