-
Notifications
You must be signed in to change notification settings - Fork 589
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
fix(ci): fix there is no reactor running in meta recovery tests, when kill-meta enabled #18852
Conversation
@@ -354,6 +354,7 @@ tokio-postgres = { git = "https://github.com/madsim-rs/rust-postgres.git", rev = | |||
# If we can merge in: https://github.com/madsim-rs/sqlx/pull/2, | |||
# we can change it to patch madsim version instead. | |||
sqlx = { git = "https://github.com/kwannoel/sqlx.git", rev = "ddf222f56cf99f865231a5383053645c6ea05ba3" } | |||
madsim = { git = "https://github.com/madsim-rs/madsim.git", rev = "f0ffd24864245aea236833f1ff3638f75bd51700" } |
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.
We can remove once madsim-rs/madsim#231 merged.
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.
@@ -44,7 +44,7 @@ filter_stack_trace_for_all_logs() { | |||
done | |||
} | |||
|
|||
trap filter_stack_trace_for_all_logs ERR | |||
# trap filter_stack_trace_for_all_logs ERR |
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.
We can just remove the log filter, until recovery tests stabilize. The recent logs are < 1MB, and we only upload the zipped archive for the logs.
The original bug of I will just disable kill-meta in this branch, then we can merge it in, as the original branch is fixed from the patch. Disabling kill-meta is required since there are still bugs that occur semi-frequently, so we don't want to block other's PR workflows. |
06f66b0
to
5c83d99
Compare
Sounds good! |
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!
5c83d99
to
930bb1c
Compare
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Apply patch: madsim-rs/madsim#231 which fixes an upstream bug: madsim-rs/madsim#228.
This patch fixes madsim-sqlx's drop on spawn error, and fixes one of the bugs in the meta recovery tests.
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.