On Wed, Jul 16, 2025 at 2:34 PM Andres Freund <and...@anarazel.de> wrote: > > Based on my understanding of [1], readahead makes this overall problem > > much worse by opportunistically slurping bytes off the wire and doing > > absolutely nothing with them until you call SSL_read() enough times to > > finally get to them. > > Right - but it also substantially reduces the number of syscalls :(. I'm not > sure that it's wise to close that door permanently.
I think it's very likely that OpenSSL's implementation of readahead is fundamentally incompatible with our async implementation. For one, the documented way to call SSL_read() without accidentally hitting the socket is via SSL_pending(), which readahead is documented to break. For two, if you're worried about how much data we could potentially have to hold during a "drain all pending" operation, I think readahead changes the upper bound from "the size of one TLS record" to "SO_RCVBUF", doesn't it? > Without it openssl will do one read for the record length, and another read > for the record contents. I.e. it'll often double the number of syscalls. That is unfortunate... but if we're talking about a correctness/performance tradeoff then I'm somewhat less sympathetic to the performance side. > Luckily there seems to be SSL_has_pending(): > > > SSL_has_pending() returns 1 if s has buffered data (whether processed or > > unprocessed) and 0 otherwise IIUC, we can't use SSL_has_pending() either. I need to know how exactly many bytes to drain out of the userspace buffer without introducing more bytes into it. > To me the pattern in our code seems bogus, even without readahead, no? > > getCopyDataMessage() can return 0 because our internal buffer is empty, > without ever reading from the socket or openssl. But if the SSL record > contained two messages (or parts of two messages), all the required data may > be in openssl. In which case the pqWait() that pqGetCopyData3() does will wait > forever. I think the "cheating" in pqSocketCheck() that I mentioned upthread ensures that pqWait() will see the OpenSSL data and return immediately. (That's a major architectural smell IMO, but it is there. Just not for GSS, which might explain some hangs discussed on the list a while back.) > I.e. the extra read actually ensures that we're not doing a pqWait() without > knowing whether we need one. I don't think so, because (without my WIP patch) you still can't guarantee that the single call to pqReadData() got all of the readahead data. And with my WIP patch, you don't need the extra call, because the previous call to pqReadData() will have ensured that there's no leftover SSL buffer to begin with. Am I missing something? Thanks! --Jacob
