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

rm_stm: prepping for log_state port #18516

Merged
merged 7 commits into from
May 21, 2024

Conversation

bharathv
Copy link
Contributor

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

  • 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
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

Includes partition context and producer context. This is used more
in the upcoming PR, so good to fix.
@bharathv
Copy link
Contributor Author

/dt

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented May 16, 2024

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad6c-4594-a95a-11caef4e4887:

"rptest.tests.tx_reads_writes_test.TxReadsWritesTest.test_reads_writes"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad67-4514-bc0c-9f199df62647:

"rptest.tests.transactions_test.TransactionsTest.check_progress_after_fencing_test"
"rptest.tests.tx_admin_api_test.TxAdminTest.test_delete_topic_from_ongoin_tx"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f12-ad6e-4028-b282-d420f089594a:

"rptest.tests.topic_creation_test.TopicRecreateTest.test_topic_recreation_while_producing.workload=IDEMPOTENT.cleanup_policy=compact"
"rptest.tests.tx_verifier_test.TxVerifierTest.test_all_tx_tests"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a69-4d2a-9d5b-e0aedbce0aac:

"rptest.tests.topic_creation_test.TopicRecreateTest.test_topic_recreation_while_producing.workload=IDEMPOTENT.cleanup_policy=compact"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a6c-426f-9eec-58216adcd00e:

"rptest.tests.tx_reads_writes_test.TxReadsWritesTest.test_reads_writes"
"rptest.tests.transactions_test.TxUpgradeTest.upgrade_does_not_change_tx_coordinator_assignment_test"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a64-41f7-b0e6-2799b4b7ede5:

"rptest.tests.tx_admin_api_test.TxAdminTest.test_delete_topic_from_ongoin_tx"

new failures in https://buildkite.com/redpanda/redpanda/builds/49188#018f7f1b-2a6e-4f8f-a5ea-9e15be984f4c:

"rptest.tests.transactions_test.TransactionsTest.check_progress_after_fencing_test"
"rptest.tests.tx_verifier_test.TxVerifierTest.test_all_tx_tests"

new failures in https://buildkite.com/redpanda/redpanda/builds/49198#018f800f-51bb-4133-b531-d5c3427c00dd:

"rptest.tests.nodes_decommissioning_test.NodesDecommissioningTest.test_decommissioning_and_upgrade"

new failures in https://buildkite.com/redpanda/redpanda/builds/49230#018f8245-969f-4d16-aa28-222d196b08a4:

"rptest.tests.nodes_decommissioning_test.NodesDecommissioningTest.test_decommissioning_and_upgrade"

@bharathv
Copy link
Contributor Author

/dt

@bharathv
Copy link
Contributor Author

/dt

@bharathv
Copy link
Contributor Author

/dt

@bharathv bharathv marked this pull request as ready for review May 16, 2024 23:25
@bharathv bharathv requested review from ztlpn and bashtanov May 16, 2024 23:25
ztlpn
ztlpn previously approved these changes May 17, 2024
return false;
}

vlog(_logger.debug, "[{}] evicting producer", *this);
Copy link
Contributor

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

Copy link
Contributor Author

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.
@piyushredpanda piyushredpanda merged commit e3f4b94 into redpanda-data:dev May 21, 2024
14 of 18 checks passed
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.

None yet

5 participants