Repository navigation
KAFKA-21230: Pause processing on uncommitted bytes - #23728
Conversation
903c013 to
b22b452
Compare
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
bbejeck
left a comment
There was a problem hiding this comment.
Thanks @nicktelford LGTM - kicking off a system test and will merge after that passes
There was a problem hiding this comment.
🟡 Changes recommended
Initialization writes above the limit can indefinitely pause the thread because the affected task is not yet commit-eligible.
1 open issue
What changed in this PR
This PR limits native-memory growth in Kafka Streams during prolonged rebalances by pausing processing when uncommitted state-store bytes exceed the configured limit.
Changes:
- Skip record processing and punctuation above the limit while continuing polling and commit attempts.
- Log transitions into and out of the paused state.
- Add tests for skipped commits and resuming processing after a successful commit.
| File | Description |
|---|---|
streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java |
Tests pause behavior and resumption after committing buffered bytes. |
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java |
Adds the byte-limit processing guard and pause/resume logging. |
Review effort: Balanced
dismissing review waiting for co-pilot findings to be addressed
During a long rebalance, a StreamThread can use native memory without bound and fail with an out-of-memory error. When the uncommitted state store bytes exceed statestore.uncommitted.max.bytes, StreamThread#maybeCommit forces a commit. But TaskManager skips commits during a rebalance, and runOnceWithoutProcessingThreads continues to process records. Thus the uncommitted bytes, which RocksDB keeps in a WriteBatchWithIndex in native memory, continue to increase. At the start of each iteration of the processing loop, StreamThread now calls maybePauseProcessingForUncommittedBytes(), which checks shouldCommitDueToUncommittedBytes(). Because maybeCommit commits at the end of each iteration, the bytes exceed the limit at this point only when that commit was skipped or failed. While processing is paused, the thread does not process records or run punctuators, because both write to the state stores. It continues to poll and to call maybeCommit. Processing resumes when a commit succeeds, or when a commit on partition revocation flushes the buffers. The pause and the resume are logged at INFO level. Processor.init() can also write to transactional stores. Such a task has no records processed, so it is not eligible for commit, and the pause can hold it with nothing to process. When initialization of the topology leaves uncommitted bytes in the stores, StreamTask now marks the task as needing a commit, so that the next commit drains them. This change decreases throughput during a long rebalance, but prevents the out-of-memory error. A separate change will apply the same limit to processing threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b22b452 to
458ef39
Compare
RocksDBTransactionBuffer.approximateNumUncommittedBytes() included the fixed header of an empty WriteBatch, which no commit releases. With a per-thread limit below the total of these headers, processing stayed paused after every commit. The empty header also made StreamTask request a commit after every restoration of a task with a transactional RocksDB store. The buffer now counts the WriteBatch only when it holds writes. The integration test is renamed to UncommittedBytesLimitIntegrationTest and gets a test with a one-byte limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved blocking issues remain, and the regression coverage addresses both previously identified permanent-pause conditions; tests were not executed.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
bbejeck
left a comment
There was a problem hiding this comment.
Thanks for the fix @nicktelford LGTM.
I kicked off another system test run and will merge this once that completes.
|
merged #23728 into trunk |
During a long rebalance, a `StreamThread` can use native memory without limit and fail with an out-of-memory error. When the uncommitted state store bytes exceed `statestore.uncommitted.max.bytes`, `StreamThread#maybeCommit` forces a commit. But `TaskManager` skips commits during a rebalance, and `runOnceWithoutProcessingThreads` continues to process records. Thus the uncommitted bytes continue to increase. RocksDB keeps these bytes in a `WriteBatch` in native memory, outside the block cache and the write buffers. At the start of each iteration of the processing loop, `StreamThread` now calls `maybePauseProcessingForUncommittedBytes()`, which checks `shouldCommitDueToUncommittedBytes()`. Because `maybeCommit` commits at the end of each iteration, the bytes exceed the limit at this point only when that commit was skipped or failed. So the check needs no extra state. While processing is paused, the thread does not process records or run punctuators, because both write to the state stores. It continues to poll, so the rebalance can complete, and it continues to call `maybeCommit`. Processing resumes when a commit succeeds, or when a commit on partition revocation flushes the buffers. The thread logs the pause and the resume at INFO level. This change decreases throughput during a long rebalance, but it prevents the out-of-memory error. `GlobalStreamThread` does not need this change, because one thread writes to its stores, checks the bytes and flushes them, and nothing skips that flush. A separate pull request will apply the same pause to processing threads. Two new `StreamThreadTest` tests cover the change. `shouldNotProcessOrPunctuateWhileUncommittedBytesExceedLimit` shows that the thread polls and tries to commit, but does not process or punctuate, while the commit is skipped. `shouldResumeProcessingOnceUncommittedBytesAreCommitted` shows that processing stays paused for the iteration in which the rebalance completes, and resumes only after the commit succeeds. 4.4 backport: StreamThreadTest uses mock(TaskManager.class) instead of the no-arg mockTaskManager() helper, which only exists on trunk (#23313). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewers: Bill Bejeck <bbejeck@apache.org> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
cherry-picked to 4.4 |

During a long rebalance, a
StreamThreadcan use native memory withoutlimit and fail with an out-of-memory error. When the uncommitted state
store bytes exceed
statestore.uncommitted.max.bytes,StreamThread#maybeCommitforces a commit. ButTaskManagerskipscommits during a rebalance, and
runOnceWithoutProcessingThreadscontinues to process records. Thus the uncommitted bytes continue to
increase. RocksDB keeps these bytes in a
WriteBatchin native memory,outside the block cache and the write buffers.
At the start of each iteration of the processing loop,
StreamThreadnow calls
maybePauseProcessingForUncommittedBytes(), which checksshouldCommitDueToUncommittedBytes(). BecausemaybeCommitcommits atthe end of each iteration, the bytes exceed the limit at this point only
when that commit was skipped or failed. So the check needs no extra
state. While processing is paused, the thread does not process records
or run punctuators, because both write to the state stores. It continues
to poll, so the rebalance can complete, and it continues to call
maybeCommit. Processing resumes when a commit succeeds, or when acommit on partition revocation flushes the buffers. The thread logs the
pause and the resume at INFO level.
This change decreases throughput during a long rebalance, but it
prevents the out-of-memory error.
GlobalStreamThreaddoes not needthis change, because one thread writes to its stores, checks the bytes
and flushes them, and nothing skips that flush. A separate pull request
will apply the same pause to processing threads.
Two new
StreamThreadTesttests cover the change.shouldNotProcessOrPunctuateWhileUncommittedBytesExceedLimitshows thatthe thread polls and tries to commit, but does not process or punctuate,
while the commit is skipped.
shouldResumeProcessingOnceUncommittedBytesAreCommittedshows thatprocessing stays paused for the iteration in which the rebalance
completes, and resumes only after the commit succeeds.
🤖 Generated with Claude Code
Reviewers: Bill Bejeck bbejeck@apache.org