-
Notifications
You must be signed in to change notification settings - Fork 613
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(pgwire): send notice asynchronously and promptly #20374
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
src/utils/pgwire/src/pg_protocol.rs
Outdated
let next_notice = async { | ||
let notice = session.next_notice().await; | ||
notice_stream.write(&BeMessage::NoticeResponse(¬ice)).await.inspect_err(|e| { | ||
tracing::error!(error = %e.as_report(), notice, "failed to send notice"); | ||
}).ok(); | ||
}; |
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 not cancellation-safe.
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.
Could you explain more? I guess here no leaking will happen. And if the task is cancelled, it is ok to not send full notice to the user?
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.
Changed to not create a next_notice
future every time when processing a message.
3d41055
to
cb33f0e
Compare
fe8e9be
to
fc06eb5
Compare
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
f495a10
to
91f9e6f
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.
L🐮TM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR refactors the
Session
trait andPgProtocol
to enable sending notices asynchronously and promptly to users before a command completes.This is achieved by
select
ing a new task for forwarding notice and the task for processing the command.Checklist
Documentation
Release note