dcapwell commented on code in PR #3694:
URL: https://github.com/apache/cassandra/pull/3694#discussion_r1873896779
##########
src/java/org/apache/cassandra/dht/AccordBytesSplitter.java:
##########
@@ -41,6 +42,20 @@ protected AccordBytesSplitter(Ranges ranges)
bytesLength = Integer.max(bytesLength, byteLength(range.start()));
bytesLength = Integer.max(bytesLength, byteLength(range.end()));
}
+ // In the single node single token case the ranges in TCM are merged
to +/-inf which have no token
+ // and no byte length. This isn't really a problem because byte length
isn't really that important it just means
+ // the shard boundaries will be arbitrary. You won't notice a problem
until you go to add nodes and more tokens
+ // and suddenly the splitter might use a different length and now your
shards are laid out slightly differently at
+ // each node which would result in a small amount of metadata moving
between command stores.
+ // Since BOP is already not working/supported I think it's fine to
punt on this.
+ if (bytesLength == 0)
Review Comment:
we spoke a lot in slack about this... I can't think of another code path
that can get an empty range... we did say we should improve repair testing of
ranges due to the split being used there... but even there we should have
single token ranges and not empty...
TL;DR - this should be fine...
--
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]
