-
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
refactor(pgwire): allow sharing of pg stream #20362
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
||
/// The logic of Conn is very simple, just a static dispatcher for TcpStream: Unencrypted or Ssl: | ||
/// Encrypted. | ||
pub enum Conn<S> { |
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.
Conn
is removed because the variants are now under the Arc
in PgStream
, so that we can upgrading the underlying stream for all shared PgStream
instances.
pub struct PgStream<S> { | ||
/// The underlying stream. | ||
stream: Option<S>, | ||
stream: Arc<Mutex<PgStreamInner<S>>>, |
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 overhead for this Mutex
will be minimal since there won't be any contention during actual use.
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
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
3d41055
to
cb33f0e
Compare
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Refactor pgwire to allow
PgStream
to be cloned and used concurrently, so that we can use it to write messages asynchronously, typically forNotice
s.Checklist
Documentation
Release note