- Notifications
You must be signed in to change notification settings - Fork37
Fix postgres query msg size estimate#599
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Pull Request Overview
This PR enhances the size estimation for PostgreSQL query messages to improve the accuracy of the backpressure mechanism in the batch sender. The previous implementation only accounted for the SQL query string length, while the new implementation includes struct overhead, schema/table names, column names with their overheads, and query arguments with type-specific size estimations.
- Updated
query.Size()method to provide comprehensive memory size estimation - Added helper function
estimateArgSize()to calculate size of query arguments based on their types - Added comprehensive unit tests covering nil queries, empty queries, simple queries, and queries with parameters
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/wal/processor/postgres/postgres_query_msg.go | Enhanced Size() method with detailed memory overhead calculations and added estimateArgSize() helper function |
| pkg/wal/processor/postgres/psotgres_query_msg_test.go | Added comprehensive test suite for query Size() method with multiple test cases |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Merging this branch willincrease overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer tocode statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
f360431 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Description
This PR updates the postgres query msg size estimate so that the backpressure mechanism in the batch sender can work more accurately. Previous implementation was not factoring in the size of the query column values, which especially for large tables, can create a big gap between the estimate and the real allocated size.
Related Issue(s)
Type of Change
Please select the relevant option(s):
Testing