-
Notifications
You must be signed in to change notification settings - Fork 591
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(error): log error field in stream reader retry loop #14792
Conversation
"stream source reader error, actor: {:?}, source: {:?}", | ||
self.actor_ctx.id, | ||
core.source_id, | ||
error = %e.as_report(), |
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.
On the sink side, an error would trigger actor exit and recovery loop, with the error Debug
ged with backtrace. Shall we follow the same practice here for source and switch from Display
to Debug
?
risingwave/src/stream/src/task/stream_manager.rs
Lines 694 to 695 in 71898b4
// Intentionally use `?` on the report to also include the backtrace. | |
tracing::error!(actor_id, error = ?err.as_report(), "actor exit with error"); |
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.
Either LGTM
"stream source reader error, actor: {:?}, source: {:?}", | ||
self.actor_ctx.id, | ||
core.source_id, | ||
error = %e.as_report(), |
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.
Either LGTM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
In #14755 (comment) (Unauthorized) and #14787 (
unknown yetlz4 unsupported), we can only see the error messagestream source reader error
without knowing what the underlying error is.On the contrary, the error message is included as a metrics label. This is an anti-pattern as error message can have high cardinality. However it is not removed in this PR to avoid breaking some people's existing workflow of checking the detailed error on grafana.
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.