-
Notifications
You must be signed in to change notification settings - Fork 820
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
Using synchronized(this)
instead of InternalLock
when writing to sockets
#3245
Conversation
Could you please share CPU profiles? |
syncronized(this)
instead of InternalLock
when writing to socketssynchronized(this)
instead of InternalLock
when writing to sockets
I believe the synchronization is not required there since we already synchronize via At the same time, if we go with "regular synchronized", then it would become loom-incompatible as WDYT of creating an unsynchronized |
I think it will be the best option but I have no idea how to allow user to tune its max and initial sizes |
I'm wondering if there are other problems like this ? |
We have |
I guess we could run into similar issues if we add lock-based synchronization in methods like |
If the value of what is |
Do you mean |
I have no idea what -- I've just tested, and macOS 14.4.1 yield getSendBufferSize() of 146988 when executed with Java 8, 11, 17, and 23. So if we go with |
My calculations were off in the previous comment. If a default buffer size is 150KiB, then 100 connections would consume 15MiB which is more-or-less reasonable to have by default. |
By the way, the backend uses 8192 buffer size: https://github.com/postgres/postgres/blob/3ddbac368c205fce1f293de1fe60c1b479800746/src/backend/libpq/pqcomm.c#L118-L119 See https://www.postgresql.org/message-id/0cdc5485-cb3c-5e16-4a46-e3b2f7a41322%40ya.ru However, they mention the performance increase is not significant for 1Gbps links: https://www.postgresql.org/message-id/20190728202141.GA25242%40hjp.at |
Times of 1Gbit LANs are gone, contemporary server LANs are 10-100Gbit. Our kernel settings for read/write buffers:
But for batched inserts/updates we use |
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
Closing in favor of #3248 |
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes two issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections Fixes pgjdbc#3245
…utStream, increase the send buffer size This fixes the following issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections 3) The previous implementation of GSSOutputStream could produce oversize packets that would be rejected by the backend. Fixes pgjdbc#3245 See https://github.com/postgres/postgres/blob/acecd6746cdc2df5ba8dcc2c2307c6560c7c2492/src/backend/libpq/be-secure-gssapi.c#L348
…utStream, increase the send buffer size This fixes the following issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections 3) The previous implementation of GSSOutputStream could produce oversize packets that would be rejected by the backend. Fixes pgjdbc#3245 See https://github.com/postgres/postgres/blob/acecd6746cdc2df5ba8dcc2c2307c6560c7c2492/src/backend/libpq/be-secure-gssapi.c#L348
…utStream, increase the send buffer size This fixes the following issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections 3) The previous implementation of GSSOutputStream could produce oversize packets that would be rejected by the backend. Fixes pgjdbc#3245 See https://github.com/postgres/postgres/blob/acecd6746cdc2df5ba8dcc2c2307c6560c7c2492/src/backend/libpq/be-secure-gssapi.c#L348
…utStream, increase the send buffer size This fixes the following issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections 3) The previous implementation of GSSOutputStream could produce oversize packets that would be rejected by the backend. Fixes pgjdbc#3245 See https://github.com/postgres/postgres/blob/acecd6746cdc2df5ba8dcc2c2307c6560c7c2492/src/backend/libpq/be-secure-gssapi.c#L348
…utStream, increase the send buffer size This fixes the following issues: 1) Java 21 uses ReentrantLock for BufferedOutputStream, however, we do not need synchronization as we synchronize at the level of QueryExecutor.execute 2) The default buffer size of 8192 was small for the modern connections 3) The previous implementation of GSSOutputStream could produce oversize packets that would be rejected by the backend. Fixes pgjdbc#3245 See https://github.com/postgres/postgres/blob/acecd6746cdc2df5ba8dcc2c2307c6560c7c2492/src/backend/libpq/be-secure-gssapi.c#L348
Currently when inserting with rewritten batches a lot of CPU is spent on
lock()
/unlock()
calls ofBufferedOutputStream
on writing of each value.Proposed changes switch internal implementation of
BufferedOutputStream
to usesyncronized(this)
instead ofInternalLock
If synchronization is not needed then a better approach would be using of custom implementation.