[fix] [tx] [branch-2.10] Transaction buffer recover blocked by readNext - #18971
Merged
Conversation
congbobo184
reviewed
Dec 18, 2022
| + "transactionBufferSnapshot timeout!", topic.getName()); | ||
| log.error(errorMessage, t); | ||
| callBack.recoverExceptionally( | ||
| new BrokerServiceException.ServiceUnitNotReadyException(errorMessage, t)); |
Contributor
There was a problem hiding this comment.
should close the reader, right?
Contributor
Author
There was a problem hiding this comment.
Yes, Already fixed.
congbobo184
added
release/2.9.5
area/transaction
release/2.10.4
and removed
release/2.9.5
labels
congbobo184
approved these changes
Dec 19, 2022
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Jan 10, 2023
…xt (apache#18971) ### Motivation Since PR apache#18833 can not cherry-pick to `branch-2.10`, create a separate PR. #### Context for Transaction Buffer - If turn on `transactionCoordinatorEnabled`, then `TransactionBuffer` will be initialized when a user topic create. - The `TransactionBuffer` reads the aborted logs of transactions from topic `__transaction_buffer_snapshot` -- this process is called `recovery`. - During recovery, the reading from that snapshot ledger is done via a `Reader`; the reader works like this: ``` while (reader.hasMessageAvailable()){ reader.readNext(); } ``` #### Context for Compaction - After [pip-14](https://github.com/apache/pulsar/wiki/PIP-14:-Topic-compaction), the consumer that enabled feature read-compacted will read messages from the compacted topic instead of the original topic if the task-compaction is done, and read messages from the original topic if task-compaction is not done. - If the data of the last message with key k sent to a topic is null, the compactor will mark all messages for that key as deleted. #### Issue There is a race condition: after executing `reader.hasMessageAvailable`, the following messages have been deleted by compaction-task, so read next will be blocked because there have no messages to read. ---- ### Modifications - If hits this issue, do recover again. ---- #### Why not just let the client try to load the topic again to retry the recover? If the topic load is failed, the client will receive an error response. This is a behavior that we can handle, so should not be perceived by the users. (cherry picked from commit f7dc64f)
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Jan 11, 2023
…xt (apache#18971) ### Motivation Since PR apache#18833 can not cherry-pick to `branch-2.10`, create a separate PR. #### Context for Transaction Buffer - If turn on `transactionCoordinatorEnabled`, then `TransactionBuffer` will be initialized when a user topic create. - The `TransactionBuffer` reads the aborted logs of transactions from topic `__transaction_buffer_snapshot` -- this process is called `recovery`. - During recovery, the reading from that snapshot ledger is done via a `Reader`; the reader works like this: ``` while (reader.hasMessageAvailable()){ reader.readNext(); } ``` #### Context for Compaction - After [pip-14](https://github.com/apache/pulsar/wiki/PIP-14:-Topic-compaction), the consumer that enabled feature read-compacted will read messages from the compacted topic instead of the original topic if the task-compaction is done, and read messages from the original topic if task-compaction is not done. - If the data of the last message with key k sent to a topic is null, the compactor will mark all messages for that key as deleted. #### Issue There is a race condition: after executing `reader.hasMessageAvailable`, the following messages have been deleted by compaction-task, so read next will be blocked because there have no messages to read. ---- ### Modifications - If hits this issue, do recover again. ---- #### Why not just let the client try to load the topic again to retry the recover? If the topic load is failed, the client will receive an error response. This is a behavior that we can handle, so should not be perceived by the users. (cherry picked from commit f7dc64f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Since PR #18833 can not cherry-pick to
branch-2.10, create a separate PR.Context for Transaction Buffer
transactionCoordinatorEnabled, thenTransactionBufferwill be initialized when a user topic create.TransactionBufferreads the aborted logs of transactions from topic__transaction_buffer_snapshot-- this process is calledrecovery.Reader; the reader works like this:Context for Compaction
Issue
There is a race condition: after executing
reader.hasMessageAvailable, the following messages have been deleted by compaction-task, so read next will be blocked because there have no messages to read.Modifications
Why not just let the client try to load the topic again to retry the recover?
If the topic load is failed, the client will receive an error response. This is a behavior that we can handle, so should not be perceived by the users.
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: