-
Notifications
You must be signed in to change notification settings - Fork 88
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
pgconn write buffer size as config option? #137
Comments
First, please ensure you are working on the correct repo. This repo is for the pgconn used with pgx v4. It was merged into the pgx repo for v5. As far as a making the write buffer size configurable, I'm not totally opposed to it, but my first inclination would be to avoid low level configurations like that. It should just work. (like how the Go GC has extremely few settings to tune). I would first be interested in seeing benchmarks that meaningfully changed with a larger buffer size. My assumption is that the write buffer is usually not a significant factor (obviously I could be wrong here). If it can be shown to be an issue, then either automatically adjusting the write buffer size or using a buffer pool like the internal iobufpool package might solve the problem without exposing a new setting. |
Thanks, I've confirmed my team is still using v4 - I've made a note to switch to v5 later but looking the code in question appears to be roughly the same between versions so I did my benchmarks against v4. We're doing bulk inserts using the
pattern, and the size of the overall query can get fairly large. The one I tested with was approximately 16MB in size. (to note, I think typically the overall size would be lower than this, but was looking for a worst case as opposed to average case) I documented the benchmarks here: https://github.com/quantics16/wbufbenchmark Preview:
Let me know if you need more details |
Have you considered using |
I have considered it yes. We have loads that are like the above example, and ones that are much smaller. I figured with We've not yet looked into when to switch to |
Actually, From the pgx benchmarks (v5 but I think v4 is similar):
Even at 2 rows |
Ah interesting - I'll make a note to test this in the new year. Thanks! It does appear that CopyFrom would still have the allocation issue I have above - https://github.com/jackc/pgx/blob/master/copy_from.go#L168C3-L168C3. Would it still be reasonable to consider a change to that write buffer? |
🤷♂️ My current assumption is that the improvement wouldn't be worth the (small) increase in the public pgx interface, but I guess if it could be shown to be a big enough issue we could do it. (Though ideally it would be something where pgx could adaptively resize the buffer rather than expose a setting to the user.) |
I'm looking into reducing, or at least better understanding the memory footprint from pgx that a program I have takes up. The profile points to https://github.com/jackc/pgproto3/blob/master/bind.go#L134 being the main cause of memory allocations, which it looks like uses the wbuf in a pgconn to do the writing (looking at https://github.com/jackc/pgconn/blob/master/pgconn.go#L1110).What I can't figure out is what the wbuf is for. I was looking at e948dc3 where it was added and I see it being appended into, but I don't see where it gets reset. I would have thought to see aSomewhere to 'reuse' the write buffer. What am I missing?Edit: Turns out I needed a rubber duck. I missed that wbuf was never being assigned the value of buf so it'd always be an empty slice.
In this case, my query strings are almost always larger than 1024 bytes, so there is still significant allocation overhead. This seems like something that could be promoted into a configuration variable?
The text was updated successfully, but these errors were encountered: