You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "yixiaodan (via GitHub)" <gi...@apache.org> on 2026/10/28 09:07:41 UTC
[PR] KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback [kafka]
yixiaodan opened a new pull request, #23995:
URL: https://github.com/apache/kafka/pull/23995
### JIRA
https://issues.apache.org/jira/browse/KAFKA-21380
### Problem
When the DLQ flow is enabled for a share group (KIP-1191), records whose delivery count reaches the limit are moved to `ARCHIVING` on the normal release path so they are written to the DLQ topic (`InFlightState.tryUpdateState` with `dlqSupportEnabled=true`).
However, the rollback path in `InFlightState.completeStateTransition(false)` - taken when a two-phase state transition fails (e.g. the persister write fails) and the acquisition lock has expired - archives the records directly:
```java
state = previousState.deliveryCount() >= rollbackState.maxDeliveryCount ?
RecordState.ARCHIVED : RecordState.AVAILABLE;
```
This bypasses the DLQ flow entirely: records that should have been dead-lettered are silently archived and never appear in the DLQ topic. Note that `completeStateTransition(boolean)` has no access to the `dlqSupportEnabled` flag, so the rollback path cannot currently reach `ARCHIVING`.
Two related inconsistencies:
1. `ARCHIVING` (normal path) vs `ARCHIVED` (rollback path) for the same limit-reached condition.
2. The normal path evaluates the limit with the delivery count including the current delivery (`deliveryCount` incremented before the check), while the rollback path uses `previousState.deliveryCount()` - the counts differ by one.
### Fix
- Thread the DLQ-support flag through `completeStateTransition`; on rollback with an expired lock and the delivery count at the limit, records move to `ARCHIVING` when DLQ is enabled (mirroring `tryUpdateState`), otherwise to `ARCHIVED`.
- Start the DLQ flow (`initiateDLQAndArchive`) for batches that move to `ARCHIVING` in both the local rollback (`rollbackOrProcessStateUpdates`) and the persister-failure path, outside the partition lock as in the successful path.
- Derive the DLQ cause from the delivery count when the batch has no recorded cause.
### Testing
- New `InFlightStateTest` (5 cases), which also fills the existing coverage gap for `completeStateTransition`:
- normal release path at the limit -> `ARCHIVING` (baseline)
- rollback with expired lock at the limit, DLQ enabled -> `ARCHIVING`
- rollback with expired lock at the limit, DLQ disabled -> `ARCHIVED`
- rollback with expired lock below the limit -> `AVAILABLE`
- rollback without an expired lock restores the previous state and member
- The DLQ-enabled rollback case fails on trunk and passes with this change.
- Regression: `SharePartitionTest` (289), `SharePartitionManagerTest` (60), `ShareGroupDLQStateManagerTest` (71), `ShareGroupDLQRecordFetcherTest` (29) - all pass with the change.
- `checkstyle` passes for the changed modules.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback [kafka]
Posted by "yixiaodan (via GitHub)" <gi...@apache.org>.
yixiaodan commented on PR #23995:
URL: https://github.com/apache/kafka/pull/23995#issuecomment-6486957247
Thanks Sushant — understood. I've closed the PR and left KAFKA-21380
open for the record.
Best,
xiaodan
On Thu, Oct 29, 2026 at 3:02 AM Sushant Mahajan ***@***.***>
wrote:
> *smjn* left a comment (apache/kafka#23995)
> <https://github.com/apache/kafka/pull/23995#issuecomment-6477981433>
>
> @yixiaodan <https://github.com/yixiaodan> This was deliberately avoided as
> a persister fail => a 5 time call to persister with exponential backoff
> failed. Now with DLQ again, the first step is ARCHIVING which requires a
> persister write. This too has good chance of failing (same partition key,
> so same broker). Hence the decision was taken to not stall the in-flight
> records queue over this.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/kafka/pull/23995?email_source=notifications&email_token=BEEXK3N73OONJCV4KRFV7KD5WI7CVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTMNBXG44TQMJUGMZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-6477981433>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BEEXK3MITPFK47YDLP2DO6D5WI7CVAVCNFSNUABDKJSXA33TNF2G64TZHMZDEMJRGI2DGO2JONZXKZJ3GYYDONJYHAYTCNJWUF3AE>.
> Triage notifications, keep track of coding agent tasks and review pull
> requests on the go with GitHub Mobile for iOS
> <https://github.com/notifications/mobile/ios/BEEXK3OZABK2LNRQQGFBLF35WI7CVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTMNBXG44TQMJUGMZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
> and Android
> <https://github.com/notifications/mobile/android/BEEXK3KXFJTKZVUPFOMQHKL5WI7CVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTMNBXG44TQMJUGMZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
> Download it today!
> You are receiving this because you were mentioned.
>
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback [kafka]
Posted by "yixiaodan (via GitHub)" <gi...@apache.org>.
yixiaodan closed pull request #23995: KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback
URL: https://github.com/apache/kafka/pull/23995
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback [kafka]
Posted by "yixiaodan (via GitHub)" <gi...@apache.org>.
yixiaodan commented on PR #23995:
URL: https://github.com/apache/kafka/pull/23995#issuecomment-6486791868
Thanks @smjn — understood, and thanks for the explanation.
Since the direct ARCHIVED transition is intentional to avoid stalling the in-flight queue when the persister has already failed after retries, I'll close this PR. I'll leave KAFKA-21380 open for the record. If useful, I can follow up separately with a narrower observability change (metric/log when records are archived on this path) rather than changing the state transition.
Thanks for the review.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21380: Move limit-reached records to ARCHIVING on failed transition rollback [kafka]
Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on PR #23995:
URL: https://github.com/apache/kafka/pull/23995#issuecomment-6477981433
@yixiaodan This was deliberately avoided as a persister fail => a 5 time call to persister with exponential backoff failed. Now with DLQ again, the first step is ARCHIVING which requires a persister write. This too has good chance of failing (same partition key, so same broker). Hence the decision was taken to not stall the in-flight records queue over this.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org