On Wed, Jul 16, 2025 at 4:35 PM Andres Freund <and...@anarazel.de> wrote: > Why do we care about not hitting the socket? We always operate the socket in > non-blocking mode anyway?
IIUC, that would change pqReadData() from a bounded read to an unbounded one. Then we have to somehow protect against a server that can send data faster than we can process it. > FWIW, I have seen too many folks move away from encrypted connections for > backups due to openssl being the bottlenecks to just accept that we'll never > go beyond the worst performance. Any actually decently performing networking > will need to divorce when socket IO happens and when openssl sees the data > much more heavily. Can't we make our custom BIO do that? > Just relying more and more on extremely tight coupling is a > dead end. Why? This abstraction _must_ leak. Because of PQsocket() at minimum, but also there's a bunch of code complexity around the read/write behavior and EOF differences for a TLS connection. When I look at the code in pqReadData() I am very sad. TLS does not behave like raw TCP. So if there's no way to get rid of the coupling, IMO we might as well use it. > > 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? > > It seems to be a configurable limit, with > SSL_set_default_read_buffer_len(). But the default is to just read up to a > whole frame's worth of data, instead of of reading the length > separately. I.e. an irrelevant amount of memory. Hmm, okay. > > 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. > > Why? The only case where we ought to care about whether pending data exists > inside openssl is if our internal buffer is either empty or doesn't contain > the entire message we're trying to consume. In either of those two cases we > can just consume some data from openssl, without caring how much it precisely > is. I can't tell if you're discussing the fix for this bug or a hypothetical future architecture that makes readahead safe. I don't think PQconnectPoll() behaves the way you're describing; there is no retry-read concept. The order of operations is: - read some available data; don't care how much - parse the message(s) if we have enough data - otherwise tell the caller to poll for read That's an architecture that's already very tightly coupled to the behavior of un-userspace-buffered TCP/UNIX sockets. Which is why I'm trying to have pqReadData() make compatible guarantees for SSL/GSS. --Jacob