-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add cassandra v4 support #3225
Add cassandra v4 support #3225
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3225 +/- ##
==========================================
- Coverage 95.99% 95.99% -0.01%
==========================================
Files 242 242
Lines 14789 14813 +24
==========================================
+ Hits 14197 14220 +23
- Misses 513 514 +1
Partials 79 79
Continue to review full report at Codecov.
|
08cf2ac
to
522fd50
Compare
6aa5398
to
6ee5ec1
Compare
We don't need to add v003tov004.sh because the change (removal of dclocal_read_repair_chance config) is in table metadata which gets handled seemlessly within cassandra upgrade when users upgrade their cassandra cluster from v3.11 to v4.0. Refactor cassandra integration tests to run on both cassandra v3.11 and v4.0. Signed-off-by: Ashmita Bohara <[email protected]>
…h one Signed-off-by: Ashmita Bohara <[email protected]>
Signed-off-by: Ashmita Bohara <[email protected]>
6ee5ec1
to
4d93c07
Compare
Signed-off-by: Ashmita Bohara <[email protected]>
fi | ||
} | ||
|
||
setup_cassandra() { |
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 is a nice refactoring, makes the script much more maintainable
Signed-off-by: Ashmita Bohara <[email protected]>
Do we need any documentation changes? Also, we build a certain Docker image for schema initialization, how should it be changed vis-a-vis v4? |
Good point. I will check update the documentation.
Good point. Let me check this part too. As far as I know, cqlsh from v3.11 is compatible with cassandra v4, that's how it is working in integration tests right now but we can also publish two separate images for schema-initialization for v3.11 and v4. |
I would rather make it an option to the single image, or even better if it could auto-detect v4 automatically. |
Signed-off-by: Ashmita Bohara <[email protected]>
Signed-off-by: Ashmita Bohara <[email protected]>
@@ -1,4 +1,4 @@ | |||
FROM cassandra:3.11 | |||
FROM cassandra:4.0 |
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 is done because cassandra v4.0 image contains cqlsh 6.0.0 which is backward compatible with cassandra v3.x (https://pypi.org/project/cqlsh/)
| awk -F "|" '{print $2}' \ | ||
| awk -F " " '{print $2}' \ | ||
| awk -F "." '{print $1}' \ | ||
) |
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.
Tested with cassandra v3.11 and v4.0 containers:
root@fa16f4b2b767:/# cqlsh -e "show version"
[cqlsh 6.0.0 | Cassandra 4.0.0 | CQL spec 3.4.5 | Native protocol v5]
root@fa16f4b2b767:/# cas_version=$(cqlsh -e "show version" \
| awk -F "|" '{print $2}' \
| awk -F " " '{print $2}' \
| awk -F "." '{print $1}' \
)
root@fa16f4b2b767:/# echo $cas_version
4
root@c120e02677e6:/# cqlsh -e "show version"
[cqlsh 5.0.1 | Cassandra 3.11.11 | CQL spec 3.4.4 | Native protocol v4]
root@c120e02677e6:/# cas_version=$(cqlsh -e "show version" \
> | awk -F "|" '{print $2}' \
> | awk -F " " '{print $2}' \
> | awk -F "." '{print $1}' \
> )
root@c120e02677e6:/# echo $cas_version
3
Which problem is this PR solving?
Resolves #3080
Short description of the changes
We don't need to add v003tov004.sh because the change (removal of dclocal_read_repair_chance config) is in table metadata which gets handled seemlessly within cassandra upgrade (tested locally) when users upgrade their cassandra cluster from v3.11 to v4.0. So there won't be any scenario, where existing user will need migrate their data.