[CASSANDRA-14801] calculatePendingRanges no longer safe for multiple adjacent range movements

• Type: Bug
• Status: Resolved
• Priority: Normal
• Resolution: Fixed
• Fix Version/s: 4.0-beta3, 4.0
• Component/s: Legacy/Coordination, Legacy/Distributed Metadata
• Labels: pull-request-available
• Severity: Normal
• Complexity: Normal
• Since Version: 4.0-alpha1

Source Control Link

https://github.com/apache/cassandra/commit/8ba163f25a56cb507e621b89b6928c2aef0ecc57

Test and Documentation Plan

 

• a534b2 adds tests that reproduce both bugs.
• bb36cd fixes the newly discovered bug (test1Leave1Move in a534b2). There, the failure happens because for the same pending range we can include 2 replicas with the same endpoint and different ranges (violation of PendingRangeMaps Conflict.DUPLICATE policy). This happens because right now we include in PendingRangeMaps the entire new replica after leave for leave-affected ranges and only the pending part of it for move-affected ranges. This commit marks as pending only the missing part of the new replica.
• a33b03 solves the original issue. Without this commit getPendingRanges can fail if the same replica covers more than 1 pending range. I think that this is a valid situation. Consider testLeave2 in a534b2. In this case replica Full(127.0.0.1:7012,(-9,0]) covers 2 ranges - (-9,-4] and (-4,0]. If we run the same test against 3.11 (-9, 0] is represented as {{ {(-9, -4], (-4, 0]}

  }} and that possible representation would not trigger the bug in 4.0. However, I think that we should not base safety of getPendingRanges execution on the way a particular AbstractReplicationStrategy represents a range and we should allow duplicate entries while building pending RangesAtEndpoint.

• Assignee: Alex Sorokoumov
• Reporter: Benedict Elliott Smith

Authors

Alex Sorokoumov

Reviewers

Marcus Eriksson, Sam Tunnicliffe

• Created: 03/Oct/18 11:21
• Updated: 15/Jul/21 09:15
• Resolved: 14/Sep/20 12:11

Description

Correctness depended upon the narrowing to a Set<InetAddressAndport>, which we no longer do - we maintain a collection of all Replica. Our RangesAtEndpoint collection built by getPendingRanges can as a result contain the same endpoint multiple times; and our EndpointsForToken obtained by TokenMetadata.pendingEndpointsFor may fail to be constructed, resulting in cluster-wide failures for writes to the affected token ranges for the duration of the range movement.

Issue Links

• is part of: CASSANDRA-14697 Transient Replication 4.0 pre-release followup work
• links to: GitHub Pull Request #495