fix: don't buffer client request before checking maintenance mode#1259
fix: don't buffer client request before checking maintenance mode#1259levkk wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| // This will pause the client until the maintenance mode is off | ||
| // without checking any timeouts or reading data off of the socket |
There was a problem hiding this comment.
the buffer future is created once during select! in loop and in order to it to be recreated we should proceed to the next iteration of the loop.
that means we can get the following flow:
- client connects and stays idle
- no maintenance mode so we don't wait and go straight to buffer read
- we enable maintenance
- the idle client runs some command and we execute them without checks for maintenance
yes, other branches in select! could recreate the future, but that's not reliable.
So, moving the check upfront could it make it worse, because the check actually moved to the connection time whilst previously it was at execution time, when we actually got some commands.
I think we really need integration tests here to verify the behavior.
There was a problem hiding this comment.
Oh wow. That's such a footgun...really good catch.
| // so if the client disconnects while we wait, we won't send | ||
| // its pending query to the database. |
There was a problem hiding this comment.
I guess the socket data will still be in the kernel buffer and we'll still read and try to execute as soon as we read proper spliced part. The query will be executed then and we'll probably get an error when we write.
that's why another pr for the issue was trying to always read the stream to catch the EOF to understand if the client is disconnected during maintenance.
2 participants
pub(crate)