Re: libpq: Process buffered SSL read bytes to support records >8kB on async API - Mailing list pgsql-hackers
From | Andres Freund |
---|---|
Subject | Re: libpq: Process buffered SSL read bytes to support records >8kB on async API |
Date | |
Msg-id | kl6inl3iyycxqpiedmnnzaz2ytid5dw6knwwqk5plg6ej34oi6@fbyu4ll7ebzn Whole thread Raw |
In response to | Re: libpq: Process buffered SSL read bytes to support records >8kB on async API (Jacob Champion <jacob.champion@enterprisedb.com>) |
Responses |
Re: libpq: Process buffered SSL read bytes to support records >8kB on async API
|
List | pgsql-hackers |
Hi, On 2025-07-15 15:31:25 -0700, Jacob Champion wrote: > So I think pqReadData() needs to make the same guarantees for SSL/GSS > that it does for plain TCP: when it returns, either 1) there are no > bytes left pending or 2) the socket itself is marked readable. > Otherwise I think we'll continue to chase weird corner cases. It's not really the same issue, as at least my reproducer requires modifying libpq (and thus clearly isn't a bug), but it thought it might still be interesting. For one, I'm not sure this can only be triggered with the modifications. If one modifies libpq to use openssl readahead (which does result in speedups, because otherwise openssl think it's useful to do lots of 5 byte reads from the socket), I see occasional hangs in libpq. diff --git i/src/interfaces/libpq/fe-secure-openssl.c w/src/interfaces/libpq/fe-secure-openssl.c index 51dd7b9fec0..a8deb28594c 100644 --- i/src/interfaces/libpq/fe-secure-openssl.c +++ w/src/interfaces/libpq/fe-secure-openssl.c @@ -864,6 +864,9 @@ initialize_SSL(PGconn *conn) */ SSL_CTX_set_mode(SSL_context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + SSL_CTX_set_read_ahead(SSL_context, 1); + SSL_CTX_set_default_read_buffer_len(SSL_context, 64*1024); + /* * If the root cert file exists, load it so we can perform certificate * verification. If sslmode is "verify-full" we will also do further ninja src/bin/psql/psql && src/bin/psql/psql -Xq -h localhost -c '\copy pg_class to stdout' hangs reliably for me (if it's going to a file it's less reliable, so there's a timing aspect here). At that point psql is not interruptible, which isn't great either... Backtrace: #0 __internal_syscall_cancel (a1=a1@entry=140726731206072, a2=a2@entry=1, a3=-1, a4=a4@entry=0, a5=a5@entry=0, a6=a6@entry=0,nr=7) at ./nptl/cancellation.c:44 #1 0x00007f1e25e3f6ad in __syscall_cancel (a1=a1@entry=140726731206072, a2=a2@entry=1, a3=<optimized out>, a4=a4@entry=0,a5=a5@entry=0, a6=a6@entry=0, nr=7) at ./nptl/cancellation.c:75 #2 0x00007f1e25eb39c6 in __GI___poll (fds=fds@entry=0x7ffd7ed2edb8, nfds=nfds@entry=1, timeout=<optimized out>) at ../sysdeps/unix/sysv/linux/poll.c:29 #3 0x00007f1e26248dea in poll (__fds=0x7ffd7ed2edb8, __nfds=1, __timeout=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/poll2.h:44 #4 PQsocketPoll (sock=<optimized out>, forRead=<optimized out>, forWrite=<optimized out>, end_time=<optimized out>) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-misc.c:1175 #5 0x00007f1e262491a8 in pqSocketCheck (conn=conn@entry=0x55f69b7e68f0, forRead=1, forWrite=0, end_time=-1) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-misc.c:1114 #6 0x00007f1e262492b3 in pqWaitTimed (forRead=<optimized out>, forWrite=<optimized out>, conn=0x55f69b7e68f0, end_time=<optimizedout>) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-misc.c:1039 #7 0x00007f1e262492f1 in pqWait (forRead=forRead@entry=1, forWrite=forWrite@entry=0, conn=conn@entry=0x55f69b7e68f0) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-misc.c:1021 #8 0x00007f1e26234c46 in pqGetCopyData3 (conn=conn@entry=0x55f69b7e68f0, buffer=buffer@entry=0x7ffd7ed2efb8, async=async@entry=0) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-protocol3.c:1846 #9 0x00007f1e26243d98 in PQgetCopyData (conn=conn@entry=0x55f69b7e68f0, buffer=buffer@entry=0x7ffd7ed2efb8, async=async@entry=0) at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-exec.c:2840 #10 0x000055f65d28f9c6 in handleCopyOut (conn=0x55f69b7e68f0, copystream=copystream@entry=0x7f1e25f985c0 <_IO_2_1_stdout_>,res=res@entry=0x7ffd7ed2f008) at ../../../../../home/andres/src/postgresql/src/bin/psql/copy.c:442 #11 0x000055f65d27b07d in HandleCopyResult (resultp=resultp@entry=0x7ffd7ed2f158, copystream=0x7f1e25f985c0 <_IO_2_1_stdout_>) at ../../../../../home/andres/src/postgresql/src/bin/psql/common.c:950 #12 0x000055f65d27dab8 in ExecQueryAndProcessResults (query=query@entry=0x55f69b8b3470 "COPY pg_attribute TO STDOUT ", elapsed_msec=elapsed_msec@entry=0x7ffd7ed2f1b8, svpt_gone_p=svpt_gone_p@entry=0x7ffd7ed2f1b7, is_watch=is_watch@entry=false,min_rows=min_rows@entry=0, opt=opt@entry=0x0, printQueryFout=0x0) at ../../../../../home/andres/src/postgresql/src/bin/psql/common.c:1934 #13 0x000055f65d27cdc9 in SendQuery (query=0x55f69b8b3470 "COPY pg_attribute TO STDOUT ") at ../../../../../home/andres/src/postgresql/src/bin/psql/common.c:1212 #14 0x000055f65d28f6dd in do_copy (args=args@entry=0x55f69b8b1f10 "pg_attribute to stdout") at ../../../../../home/andres/src/postgresql/src/bin/psql/copy.c:370 #15 0x000055f65d299f24 in exec_command_copy (scan_state=scan_state@entry=0x55f69b8a71f0, active_branch=active_branch@entry=true) at ../../../../../home/andres/src/postgresql/src/bin/psql/command.c:951 #16 0x000055f65d2a2065 in exec_command (cmd=cmd@entry=0x55f69b8b3310 "copy", scan_state=scan_state@entry=0x55f69b8a71f0,cstack=cstack@entry=0x55f69b887ba0, query_buf=query_buf@entry=0x0, previous_buf=previous_buf@entry=0x0) at ../../../../../home/andres/src/postgresql/src/bin/psql/command.c:338 #17 0x000055f65d2a2b59 in HandleSlashCmds (scan_state=scan_state@entry=0x55f69b8a71f0, cstack=cstack@entry=0x55f69b887ba0,query_buf=query_buf@entry=0x0, previous_buf=previous_buf@entry=0x0) at ../../../../../home/andres/src/postgresql/src/bin/psql/command.c:241 #18 0x000055f65d29305f in main (argc=<optimized out>, argv=<optimized out>) at ../../../../../home/andres/src/postgresql/src/bin/psql/startup.c:409 It does seem ... weird ... that pqGetCopyData3() just processes whatever is already in the libpq buffer and then waits for the socket to become readable, before ever calling pqReadData(). What guarantees, even without the change above, that a) there's no pending data inside openssl b) that we're not waiting for a write? If I make pqGetCopyData3() call pqReadData() before the pqWait() and continue if it returns 1, the hang vanishes. The same if (pqWait(true, false, conn) || pqReadData(conn) < 0) return -2; pattern (without a preceding pqReadData()) exists in several other places :( > What I'm not sure about is unforeseen consequences -- could fixing > this expose other latent bugs? If we're concerned about that, we might > have to tier the backports to make sure everything remains stable. > Here are the three bugs discussed so far, with the fix for 3 possibly > subsuming 1 and 2 but also affecting more code: > > 1) pqSocketCheck() checks for pending SSL bytes but not GSS bytes > 2) PQconsumeInput() does not drain all pending bytes > 3) pqReadData() does not drain all pending bytes What are the limits for the maximum amount of data this could make us buffer in addition to what we are buffering right now? It's not entirely obvious to me that a loop around pqReadData() as long as there is pending data couldn't make us buffer a lot of data. Do you have a WIP patch? Greetings, Andres Freund
pgsql-hackers by date: