On 11-06-15 03:20 PM, Martin Pihlak wrote:

Yes, that sounds like a good idea -- especially considering that COPY
is not the only operation that can cause SSL_write retries.


This is of course still "work in progress", needs cleaning up and
definitely more testing. But at this point before going any further,
I'd really appreciate a quick review from resident libpq gurus.


Martin,

I'm not a libpq guru but I've given your patch a look over.

The idea of storing the original buffer in new members of connection structure for later re-use seems like a good way to approach this.

A few things I noticed (that you might be aware of since you mentioned it needs cleanup)

-The patch doesn't compile with non-ssl builds, the debug at the bottom of PQSendSome isn't in an #ifdef

-I don't think your handling the return code properly.   Consider this case.

pqSendSome(some data)
  sslRetryBuf = some Data
  return 1
pqSendSome(more data)
  it sends all of 'some data'
  returns 0

I think 1 should be returned because all of 'more data' still needs to be sent. I think returning a 0 will break PQsetnonblocking if you call it when there is data in both sslRetryBuf and outputBuffer. We might even want to try sending the data in outputBuffer after we've sent all the data sitting in sslRetryBuf.


If you close the connection with an outstanding sslRetryBuf you need to free it.


Other than running your test program I didn't do any testing with this patch.

Steve

regards,
Martin




Reply via email to