-
Notifications
You must be signed in to change notification settings - Fork 551
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
rm_stm: prepping for log_state port #18516
Conversation
Includes partition context and producer context. This is used more in the upcoming PR, so good to fix.
/dt |
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad6c-4594-a95a-11caef4e4887:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad67-4514-bc0c-9f199df62647:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad6e-4028-b282-d420f089594a:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a69-4d2a-9d5b-e0aedbce0aac:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a6c-426f-9eec-58216adcd00e:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a64-41f7-b0e6-2799b4b7ede5:
new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a6e-4f8f-a5ea-9e15be984f4c:
new failures in https://buildkite.com/redpanda/redpanda/builds/49198#018f800f-51bb-4133-b531-d5c3427c00dd:
new failures in https://buildkite.com/redpanda/redpanda/builds/49230#018f8245-969f-4d16-aa28-222d196b08a4:
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad67-4514-bc0c-9f199df62647 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a6c-426f-9eec-58216adcd00e ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49198#018f8007-fe40-41eb-84eb-d67a748b4b16 |
/dt |
/dt |
/dt |
return false; | ||
} | ||
|
||
vlog(_logger.debug, "[{}] evicting producer", *this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the function name should be maybe_evict, not can_evict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right.. I noticed this too but changing this isn't quite as simple, it has implications in the naming of namespace_cache that takes a "pre_eviction_hook" which is effectively this method, thats a deeper naming change, probably better to do as a follow up.
Also renames it to partition_transaction_status to convey the intent better.
Rename them to convey intent better.
Next PR adds more such methods like apply_begin/apply_end etc, so renaming it to be consistent
Upon term change aggressively GC inflight requests from previous term(s) that haven't completed. Earlier this was done lazily on next request emplace. We use this improvement in the next commit to get rid of "relink_producer" logic.
relink_producer was intended to move the producer to the back of the LRU cache. This was done at different times on the leader/followers. On leaders, it was done right after the request was processed whereas on followers it was done after the batch was applied. In order not to do it twice on leader, the relink_producer was added to detect if it was already done. This commit consolidates the logic to do it in apply for all the replicas and adds a guardrail in can_evict() to prevent eviction of producers with unapplied batches.
As I was prototyping the port I identified some things that could facilitate an easier porting, factoring those fixes out into this separate PR for reviewers' convenience.
Backports Required
Release Notes