-
Notifications
You must be signed in to change notification settings - Fork 3.8k
CASSANDRA-18675: (Accord): C* stores keyspace in Range which will cause ranges to be removed from Accord when DROP KEYSPACE is performed #3574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| public static void visitUDTs(TableMetadata metadata, Consumer<UserType> fn) | ||
| { | ||
| Set<UserType> udts = CassandraGenerators.extractUDTs(metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could make this more efficient but figured reusing is best as its been stable...
| Invariants.checkState(cfs != null, "Unable to find table %s", id); | ||
| BigInteger targetSplitSize = BigInteger.valueOf(Math.max(1, cfs.estimateKeys() / 1_000_000)); | ||
|
|
||
| List<AsyncChain<?>> syncs = new ArrayList<>(ranges.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add visibility into what's going on, but there is an assumption that Benedict is making a cheaper option, so holding off doing that work hoping that option can replace this splitting.
| { | ||
| Transformation.Result result = iter.next().applyTo(metadata); | ||
| assert result.isSuccess(); | ||
| assert result.isSuccess() : result.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when debugging my patch this was useful...
| { | ||
| out.writeUTF(dc.name); | ||
| out.writeUnsignedVInt(dc.weight); | ||
| out.writeUnsignedVInt32(dc.weight); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by actually trying to serialize this I found this bug... why do we keep out.writeUnsignedVInt around if we don't allow it anymore? 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this now conflicts with https://issues.apache.org/jira/browse/CASSANDRA-19954 on trunk....
Believe Sam/Marcus plan to also add more to V3... so Accord is likely V4, but with the MIN_ACCORD_VERSION it should be easier to handle when that happens
beobal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly lgtm, but left a few comments.
src/java/org/apache/cassandra/tcm/sequences/DropAccordTable.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/tcm/sequences/DropAccordTable.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/tcm/sequences/DropAccordTable.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/tcm/sequences/DropAccordTable.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/tcm/transformations/PrepareDropAccordTable.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/cql3/statements/schema/DropKeyspaceStatement.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/service/accord/AccordService.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/tcm/transformations/FinishDropAccordTable.java
Outdated
Show resolved
Hide resolved
1c1fa50 to
2f0e712
Compare
…ved from Accord when DROP TABLE is performed patch by David Capwell, Sam Tunnicliffe; reviewed by Sam Tunnicliffe for CASSANDRA-18675
76414e9 to
e693a10
Compare
Accord PR: apache/cassandra-accord#125