Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

c/rm_stm: cleanup tx_locks #15797

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Conversation

rockwotj
Copy link
Contributor

Currently tx_locks are never cleaned up and the _tx_locks map grows
unboundedly. Fix this by removing the lock when we cleanup the producer
state. I'm not sure if we can remove the lock (optimistically?) when
transactions are aborted or committed, as we erase some in memory state
at that point. I'm being more catious and not doing that in this PR, but
it seems like if nobody is holding the lock at the time when we
abort/commit a transaction, we can clean the lock up, as it's only there
to prevent concurrent tx related requests for a single pid.

NOTE: I already have a backport up in 23.2.x

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

Bug Fixes

  • Fix a memory leak when using transactions with many different producer IDs.

@rockwotj
Copy link
Contributor Author

I'm not really fully convinced that we even need the tx_lock map, can't we use the op_lock in producer state?

@rockwotj
Copy link
Contributor Author

Actually nevermind do we need that to guard against different epochs still?

@rockwotj rockwotj force-pushed the cleanup-the-tx-locks branch from b26e947 to 05b59cc Compare December 20, 2023 02:53
Currently tx_locks are never cleaned up and the _tx_locks map grows
unboundedly. Fix this by removing the lock when we cleanup the producer
state. I'm not sure if we can remove the lock (optimistically?) when
transactions are aborted or committed, as we erase some in memory state
at that point. I'm being more catious and not doing that in this PR, but
it seems like if nobody is holding the lock at the time when we
abort/commit a transaction, we can clean the lock up, as it's only there
to prevent concurrent tx related requests for a single pid.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj rockwotj force-pushed the cleanup-the-tx-locks branch from 05b59cc to 2c42ea1 Compare December 20, 2023 03:14
@rockwotj rockwotj merged commit 70ebd2b into redpanda-data:dev Dec 20, 2023
19 checks passed
@rockwotj rockwotj deleted the cleanup-the-tx-locks branch December 20, 2023 08:37
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants