yifan-c commented on PR #234: URL: https://github.com/apache/cassandra-analytics/pull/234#issuecomment-5460838467
Sorry for the belated review. First of all, thanks for adding more test cases. I think the fix can be much smaller than what's here. The actual bug is just that `.filter(Range::isEmpty)` in `validateCompleteRangeCoverage()` is a no-op: Guava's TreeRangeSet never materializes empty ranges in `asRanges()`, so that filter can never keep anything, and the missing-range check silently always passes. The minimal and correct fix is: - Drop the `.filter(Range::isEmpty)` filter - Change `Range.closed(minToken, maxToken)` to `Range.openClosed(minToken, maxToken)`, since sub-ranges everywhere else are open-closed and closed would falsely flag minToken as a gap on an otherwise healthy ring Would you mind trimming the patch down to that? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
