-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feat][io] Add ScyllaDB tests #24931
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
base: master
Are you sure you want to change the base?
[feat][io] Add ScyllaDB tests #24931
Conversation
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.
Thanks for the PR. Please remove the changes to the top-level README.md file.
README.md
Outdated
| ## Database Integrations | ||
|
|
||
| Pulsar provides native sink connectors for NoSQL databases: | ||
|
|
||
| - **Apache Cassandra**: Official Cassandra sink connector for streaming key-value data | ||
| - **ScyllaDB**: Drop-in compatible with Cassandra sink connector. ScyllaDB offers high performance and low latency while maintaining full Cassandra protocol compatibility. See [ScyllaDB Integration Guide](SCYLLADB_INTEGRATION.md) for details. | ||
|
|
||
| **Learn more:** | ||
| - [Connect Pulsar to Cassandra/ScyllaDB](https://pulsar.apache.org/docs/4.1.x/io-quickstart/#connect-pulsar-to-cassandra) | ||
| - [Streaming Real-Time Chat Messages into ScyllaDB with Apache Pulsar](https://www.scylladb.com/2022/04/25/streaming-real-time-chat-messages-into-scylladb-with-apache-pulsar/) | ||
|
|
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.
The database integrations shouldn't be high-lighted in apache/pulsar README.
Please remove the changes to README.md.
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.
Please avoid statements like "ScyllaDB offers high performance and low latency while maintaining full Cassandra protocol compatibility." that is marketing instead be more neutral like "ScyllaDB offers full Cassandra protocol compatibility." Users choosing ScyllaDB already have chosen it over 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.
Thank you for the feedback! I have removed the change altogether, since it does not belong in this README as per @lhotari. I have also double-checked the rest of the codebase - don't think I have other ScyllaDB performance claims, so hope this works
…the PR suggestion
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.
LGTM
Resolves issue 24924
Motivation
ScyllaDB is a drop-in, more performance-focused replacement for Apache Cassandra. It works out of the box with Pulsar simply by replacing the Cassandra connection string with a ScyllaDB one in the liquibase-cassandra extension (verified this locally)
There are already some articles on integrating Pulsar with Scylla here and here. So it might be worth solidifying this in Pulsar's docs and tests.
Modifications
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeDoc PR available here
Matching PR in forked repository
PR in forked repository: andrii-kysylevskyi#1