-
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(frontend): memory throttling for frontend messages #20649
base: main
Are you sure you want to change the base?
Conversation
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.
PR Overview
This PR implements memory throttling for frontend messages to mitigate OOM issues by introducing a MessageMemoryManager in the frontend and propagating its usage through the pgwire protocol. Key changes include:
- Adding MessageMemoryManager and MessageMemoryGuard to track and limit memory usage for messages.
- Updating pgwire protocol and message handling to integrate memory throttling and server throttle responses.
- Extending configuration and documentation to include new memory-related thresholds.
Reviewed Changes
File | Description |
---|---|
src/utils/pgwire/src/pg_protocol.rs | Added MessageMemoryManager, integrated memory management in read_message, and adjusted error handling for throttling. |
src/utils/pgwire/src/pg_message.rs | Introduced new message types and refactored header/body reading functions. |
src/common/src/config.rs | Added new fields to configure frontend message memory limits. |
src/frontend/src/lib.rs | Updated connection context creation to include MessageMemoryManager. |
src/utils/pgwire/src/error.rs | Added ServerThrottle error variant. |
src/config/example.toml | Updated example configuration with memory thresholds. |
src/frontend/src/scheduler/local.rs | Adjusted drop order for memory release in query execution. |
src/config/docs.md | Documented the new memory-related configuration options. |
src/utils/pgwire/src/pg_server.rs | Updated connection setup to use the new ConnectionContext. |
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/utils/pgwire/src/pg_protocol.rs:111
- Consider whether using Ordering::Relaxed is sufficient for concurrent updates to current_running_bytes. If ordering issues could arise, a stronger memory ordering might be necessary to ensure consistency.
.fetch_add(bytes, Ordering::Relaxed);
src/utils/pgwire/src/pg_protocol.rs:597
- [nitpick] Enhance the error message by including the actual message size (or payload size) along with the limit to provide clearer context for troubleshooting throttling issues.
return Err(PsqlError::ServerThrottle(anyhow::anyhow!(format!("frontend_throttling_filter_max_bytes {} has been exceeded, please either reduce the message size or increase the limit", self.message_memory_manager.max_filter_bytes)).into()));
src/frontend/src/scheduler/local.rs:122
- [nitpick] Verify that the new drop order—dropping plan_node before dropping self—is intentional and does not inadvertently affect the lifespan of large objects or resource cleanup.
drop(self);
@@ -506,15 +592,43 @@ where | |||
} | |||
} | |||
FeMessage::HealthCheck => self.process_health_check(), | |||
FeMessage::ServerThrottle(reason) => match reason { | |||
ServerThrottleReason::TooLargeMessage => { | |||
return Err(PsqlError::ServerThrottle(anyhow::anyhow!(format!("frontend_throttling_filter_max_bytes {} has been exceeded, please either reduce the message size or increase the limit", self.message_memory_manager.max_filter_bytes)).into())); |
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.
return Err(PsqlError::ServerThrottle(anyhow::anyhow!(format!("frontend_throttling_filter_max_bytes {} has been exceeded, please either reduce the message size or increase the limit", self.message_memory_manager.max_filter_bytes)).into())); | |
return Err(PsqlError::ServerThrottle(anyhow::anyhow!("frontend_throttling_filter_max_bytes {} has been exceeded, please either reduce the message size or increase the limit", self.message_memory_manager.max_filter_bytes).into())); |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR is a follow-up to #20470. It aims to address the frontend OOM issue caused by a surge of large incoming queries, which may occur due to numerous concurrent DML or a few extremely large DML.
A
MessageMemoryManager
has been implemented in the frontend. It tracks memory usage of frontend messages and provides hint of whether this message would cause memory constraint violation. For any message of size n,min_filter_bytes
, the message won't add to the memory constraint, because the message is too small that it can always be accepted. e.g.SELECT
is usually small relative in size and should not be rejected.max_filter_bytes
, the message won't add to the memory constraint, because the message is too large that it's better to be rejected immediately.MessageMemoryManager::add
.The frontend will follow
MessageMemoryManager
's hint and respond client with error immediately, without processing the message any further.Tests similar to those in #20470 have demonstrated the effectiveness of this PR.
Checklist
Documentation
Release note