cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <>
Subject [jira] [Created] (CASSANDRA-9711) Refactor AbstractBounds hierarchy
Date Thu, 02 Jul 2015 10:28:04 GMT
Sylvain Lebresne created CASSANDRA-9711:

             Summary: Refactor AbstractBounds hierarchy
                 Key: CASSANDRA-9711
             Project: Cassandra
          Issue Type: Improvement
            Reporter: Sylvain Lebresne
             Fix For: 3.x

As it has been remarked in CASSANDRA-9462 and other tickets, the API of {{AbstractBounds}}
is pretty messy. In particular, it's not terribly consistent nor clear on it's handling of
wrapping ranges. It also doesn't make it easily identifiable if an {{AbstractBounds}} can
be wrapping or not, and there is a lot of place where the code assumes it's not but without
really checking it, which is error prone. It's also not a very nice API to use (the fact their
is 4 different classes that don't even always support the same methods is annoying).

So we should refactor that API. How exactly is up for discussion however.
At the very least we probably want to stick to a single concrete class that know if it's bounds
are inclusive or not. But one other thing I'd personally like to explore is to separate ranges
that can wrap from the one that cannot in 2 separate classes (which doesn't mean they can't
share code, they may even be subtypes). Having 2 separate types would:
# make it obvious what part of the code expect what.
# would probably simplify the actual code: we unwrap stuffs reasonably quickly in the code,
so there probably is a lot of operations that we don't care about on wrapping ranges and lots
of operations are easier to write if we don't have to deal with wrapping.
# for the non-wrapping class, we could trivially use a different value for the min and max
values, which will simplify stuff a lot. It might be harder to do the same for wrapping ranges
(especially since a single "wrapping" value is what IPartitioner assumes; of course we can
change IPartitioner but I'm not sure blowing the scope of this ticket is a good idea).

As a side note, Guava has a [Range|].
If we do separate wrapping and non-wrapping ranges, we might (emphasis on "might") be able
to reuse it for the non-wrapping case, which could be nice (they have a [RangeMap|]
in particular that could maybe replace our custom {{IntervalTree}}).

This message was sent by Atlassian JIRA

View raw message