-
Notifications
You must be signed in to change notification settings - Fork 555
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
feat(sink): support starrocks sink #12681
Conversation
8d799b4
to
b5b982d
Compare
fix fix fix
b5b982d
to
aed8ef7
Compare
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.
what does doris_connector
mean here? we have separate doris
and starrocks
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.
Starrocks and Doris both use HTTP for data import, and here is where their logic is identical (calling the HTTP interface).
Codecov Report
@@ Coverage Diff @@
## main #12681 +/- ##
==========================================
- Coverage 68.00% 67.89% -0.12%
==========================================
Files 1521 1522 +1
Lines 257858 258289 +431
==========================================
+ Hits 175364 175365 +1
- Misses 82494 82924 +430
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
pub(crate) const STARROCKS_DELETE_SIGN: &str = "__op"; | ||
const STARROCK_MYSQL_PREFER_SOCKET: &str = "false"; | ||
const STARROCK_MYSQL_MAX_ALLOWED_PACKET: usize = 1024; | ||
const STARROCK_MYSQL_WAIT_TIMEOUT: usize = 31536000; |
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.
Is this ms or s? Why 31536000? Any special consideration?
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 because initially we were directly querying in MySQL. Since Starrock doesn't have this table, it required us to set up bug prevention. We set the default value, but the value 31536000 was incorrect due to an error in the location of the query. After checking the official website, it should be 28800, and it has been updated.
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.
Rest LGTM
@@ -388,10 +390,12 @@ pub enum SinkError { | |||
Redis(String), | |||
#[error("Nats error: {0}")] | |||
Nats(anyhow::Error), | |||
#[error("Doris http error: {0}")] | |||
#[error("Doris/Starrocks connect error: {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.
Better name it http error?
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
support starrocks sink. Can't support struct and json. because starrocks's stream load don't support it.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.