node-mysql2 icon indicating copy to clipboard operation
node-mysql2 copied to clipboard

Fix backpressure when using TLS

Open vobarian opened this issue 3 years ago • 1 comments

This PR fixes two related issues when using TLS:

  • Using LOAD DATA LOCAL, backpressure is not respected, causing out of memory errors if the data stream supplied by infileStreamFactory is faster than the upload to the database server
  • Using connection.query().stream(), backpressure is not respected if data is returned by the server faster than the client is handling it

Both issues result from the fact that the backpressure mechanisms of pause(), resume(), emit('pause'), on('drain'), etc., were being applied to the original stream object, which have no effect after that stream has been wrapped inside a TLSSocket, according to my experiments. This can lead to an out of memory error if the buffer grows too large.

See: backpressure not handled #1134 Prior fix, which did not cover TLS, because the startTLS function was replacing the write method: handle backpressure when loading data from file #1167

Note: There is still the legacy TLS support at the bottom of connection.js which I did not touch. The docs state TLSSocket was added in Node.js v0.11.4 which was released in 2013. It is so old I did not feel like looking into it.

vobarian avatar Apr 15 '22 19:04 vobarian

I seem to be in need of this. Any time @sidorares ? Found your sponsorship page. Wish I could do more but I won't stop it (sorry you'll have to wait to see that I'm not lying)

terrisgit avatar Sep 15 '22 16:09 terrisgit

LGTM

could you rebase on top of master @vobarian ? That should trigger workflows that were added after creation of this PR

sidorares avatar Nov 29 '22 07:11 sidorares