Skip to content

HDDS-16142. Fix double-decrement of deleted-block summary on Ratis timeout - #11043

Merged
ChenSammi merged 1 commit into
apache:masterfrom
priyeshkaratha:HDDS-16142
Aug 19, 2026
Merged

HDDS-16142. Fix double-decrement of deleted-block summary on Ratis timeout#11043
ChenSammi merged 1 commit into
apache:masterfrom
priyeshkaratha:HDDS-16142

Conversation

@priyeshkaratha

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

HDDS-15726 introduced rollback logic that reverts those counters on IOException from addTransactionsToDB / removeTransactionsFromDB. That is correct for definitive failures (not-leader, connection error,
state-machine IO error), but wrong for a Ratis timeout.

SCMRatisServerImpl.submitRequest calls server.submitClientRequestAsync(request).get(requestTimeout, ms)

A TimeoutException from .get() only abandons the client-side wait. The CompletableFuture is discarded, but the in-flight log entry continues through the Ratis pipeline and can still be committed and applied after the caller has already seen the exception. Rolling back the summary at that point produces:

  • a double-decrement on the add path (summary goes negative), or
  • an overcount on the remove path (summary stays inflated).

Both divergences persist until the next leader transfer, which reconciles the counters by reloading from the durable state via
initDataDistributionData().

Fix

Introduce isRatisTimeout(IOException) to distinguish ambiguous outcomes from definitive failures. The check covers two cases:

  • SCMException(ResultCodes.TIMEOUT): translateException maps a java.util.concurrent.TimeoutException to this code.
  • SCMException(ResultCodes.INTERNAL_ERROR) whose cause is InterruptedException: .get() can also throw InterruptedException if the thread is interrupted while waiting; the request is already in-flight, giving the same ambiguous outcome as a timeout.

What is the link to the Apache JIRA

HDDS-16142

How was this patch tested?

Tested by added cases and existing tests.

@priyeshkaratha
priyeshkaratha marked this pull request as ready for review August 18, 2026 08:32

@ChenSammi ChenSammi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @priyeshkaratha . This is the question that I thought about during HDDS-15726. The change LGTM.

Thanks @smengcl for raising this issue.

@ChenSammi
ChenSammi merged commit 9ac58d9 into apache:master Aug 19, 2026
45 checks passed
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.

2 participants