Skip to content

KAFKA-7519 Clear pending transaction state when expiration fails#5820

Merged
ijuma merged 2 commits intoapache:trunkfrom
BirdHowl:kafka-7519-pending-state-on-expired-transactions
Oct 22, 2018
Merged

KAFKA-7519 Clear pending transaction state when expiration fails#5820
ijuma merged 2 commits intoapache:trunkfrom
BirdHowl:kafka-7519-pending-state-on-expired-transactions

Conversation

@BirdHowl
Copy link
Copy Markdown
Contributor

Description:

Make sure that the transaction state is properly cleared when the transactionalId-expiration task fails. Operations on that transactional id would otherwise return aCONCURRENT_TRANSACTIONS error and appear "untouchable" to transaction state changes, preventing transactional producers from operating until a broker restart or transaction coordinator change.

Testing:

Unit tested by verifying that having the transactionalId-expration task won't leave the transaction metadata in a pending state if the replica manager returns an error.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 20, 2018

retest this please

@BirdHowl BirdHowl force-pushed the kafka-7519-pending-state-on-expired-transactions branch from 7c0f7f0 to f1ab055 Compare October 21, 2018 16:04
@BirdHowl
Copy link
Copy Markdown
Contributor Author

Decided to change the tests to always check that metadata we find isn't in a pending state after transactional ID expiration.

My reasoning is that since the transactional id cleanup task is a background activity, I'd expect that it should only leave metadata in a pending state for a very short period, to avoid disrupting normal requests. Limiting that to the duration of a single instance of the cleanup task is a good restriction if we can follow it; it also avoids cases where we have to wait for the period of the cleanup task (1 hour by default is pretty bad) in addition to my original problem.

Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

LGTM. I'll push a minor formatting fix before merging. Thanks for the fix!

Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

LGTM. I'll push a minor formatting fix before merging. Thanks for the fix!

Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

LGTM. I'll push a minor formatting tweak before merging. Thanks for the fix!

@ijuma ijuma merged commit ed94cf4 into apache:trunk Oct 22, 2018
ijuma pushed a commit that referenced this pull request Oct 22, 2018
Make sure that the transaction state is properly cleared when the
`transactionalId-expiration` task fails. Operations on that transactional
id would otherwise return a `CONCURRENT_TRANSACTIONS` error
and appear "untouchable" to transaction state changes, preventing
transactional producers from operating until a broker restart or
transaction coordinator change.

Unit tested by verifying that having the `transactionalId-expiration` task
won't leave the transaction metadata in a pending state if the replica
manager returns an error.

Reviewers: Jason Gustafson <jason@confluent.io>
ijuma pushed a commit that referenced this pull request Oct 22, 2018
Make sure that the transaction state is properly cleared when the
`transactionalId-expiration` task fails. Operations on that transactional
id would otherwise return a `CONCURRENT_TRANSACTIONS` error
and appear "untouchable" to transaction state changes, preventing
transactional producers from operating until a broker restart or
transaction coordinator change.

Unit tested by verifying that having the `transactionalId-expiration` task
won't leave the transaction metadata in a pending state if the replica
manager returns an error.

Reviewers: Jason Gustafson <jason@confluent.io>
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 22, 2018

Jason asked me to merge this. JDK 8 tests passed, JDK 11 test failures unrelated. Merged to trunk, 2.1 and 2.0 branches.

pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…che#5820)

Make sure that the transaction state is properly cleared when the
`transactionalId-expiration` task fails. Operations on that transactional
id would otherwise return a `CONCURRENT_TRANSACTIONS` error
and appear "untouchable" to transaction state changes, preventing
transactional producers from operating until a broker restart or
transaction coordinator change.

Unit tested by verifying that having the `transactionalId-expiration` task
won't leave the transaction metadata in a pending state if the replica
manager returns an error.

Reviewers: Jason Gustafson <jason@confluent.io>
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.

3 participants