Fix some concurrent memory access problems in partition balancer #18305
Conversation
This file contains 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
Partition balancer relies on _topics_map_revision checks to safely iterate over topic table collections with partition granularity (i.e. references to partition data and replica sets are stored and accessed across yield points). To make this safe, increment _topics_map_revision every time _topics, _updates_in_progress, _disabled_partitions or nested collections are modified in a way that invalidates references or iterators.
|
This test is good, another bug uncovered. |
bashtanov
reviewed
May 10, 2024
There was a problem hiding this comment.
where do we bump _topics_map_revision after this emplace? https://github.com/redpanda-data/redpanda/pull/18305/files#diff-fe5b0d76af2702db674fdadb63ce097a8ea25f5b420f921374e0d1332464f549R1182
It is handy to have a base class for all instances of concurrent modifications.
Shard-local allocation_state object is replaced when we are applying a controller snapshot. After this happens, all live allocated_partition objects become invalid. Detect this and throw concurrent_modification_error in case these objects are still used and make destructors no-op.
ztlpn
force-pushed
the
fix-balancer-memory-corruption
branch
from
4fa0e47
to
1c63990
Compare
It's incremented here. Well, technically it is before, but what matters is that it is in the same continuation. |
|
/backport v24.1.x |
|
/backport v23.3.x |
|
/backport v23.2.x |
|
Failed to create a backport PR to v23.3.x branch. I tried: |
|
Failed to create a backport PR to v23.2.x branch. I tried: |
This was referenced May 10, 2024
This was referenced May 13, 2024
This was referenced May 14, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Partition balancer relies on
_topics_map_revisionchecks to safely iterate over topic table collections with partition granularity (i.e. references to partition data and replica sets are stored and accessed across yield points). To make this safe, increment_topics_map_revisionevery time_topics,_updates_in_progress,_disabled_partitionsor nested collections are modified in a way that invalidates references or iterators.Likely fixes:
RandomNodeOperationsTest.test_node_operations#18130RandomNodeOperationsTest.test_node_operations#17751RandomNodeOperationsTest.test_node_operations#16533RandomNodeOperationsTest.test_node_operations#16510RandomNodeOperationsTest.test_node_operations#13301Backports Required
Release Notes