[pgpool-general: 7969] Re: Hang with Skunk because Flush not honoured

[Oleg Oshmyan]

> I have implemented "simple" flush message tracking. It remebers there is a pending flush request when a flush message is sent from client. When a response message from backend is received, the response is flushed to frontend. The limitation is that it only flushes last 1 reply message.
> 
> What do you think?


Thank you.

For the specific case of current Skunk, I believe this should indeed be enough of a fix, provided that the earlier mitigation measure (always flushing RowDescription) remains in use a well, and provided that you also flush the response buffer (if nonempty) immediately upon receiving Flush; otherwise it is possible that Pgpool-II receives the Flush message from the client after it has already received the ParseComplete etc. message from the server and it still gets stuck.

However, as you note yourself, this is not a complete solution, and other clients may still get stuck.

I can offer another heuristic that trades some more efficiency for some more partial reliability: when you receive Flush, you could start flushing *every* response message for that client (except possibly known-good responses that *must* be followed by flushable messages in the protocol, like in my initial suggestion) and keep doing it until you receive anything else from the same client.


>> But until that is implemented, shouldn’t SimpleForwardToFrontend rather default to flushing *all* responses and have a list of specific exceptions that are allowed to be buffered because the protocol guarantees that they are followed by flushable messages?
> 
> I think the change is too risky for major applications such as JDBC in
> term of performance degration.

Surely this should not pose a risk for performance if you are careful to bufferize all messages that come in large batches? At the same time, this would reduce the risk of completely breaking another client by bufferizing something that should not be.

Personally, I think performance is definitely important but correctness/reliability should be even more important. If Pgpool-II were to optimize for a common case at the expense of slowing down a rare case, that would be understandable, but currently it renders rare (supposedly) cases completely unusable.

Does the PostgreSQL protocol even allow bufferizing anything other than extended-query responses? The Flush command is only allowed within the extended query flow, so does that not mean that all other responses *cannot* be delayed?

I apologize if this goes off topic or oversteps boundaries, but why does Pgpool-II have user-space bufferization of responses at all? Is it not enough to rely on the operating system's network stack to bufferize data and combine into packets as it sees fit? It knows better than anyone what the network connection and buffer state is, and it has timeouts to ensure data is never stuck in a buffer indefinitely like is happening in Pgpool-II currently. If you want to hint to the OS that you have more data coming, you can use MSG_MORE/TCP_CORK. (For example, the Linux default with TCP_CORK is to wait at most 200 ms for the application to signal the end of data by unsetting TCP_CORK.)

Moreover, could Pgpool-II not simply forward whole TCP packets it receives from the upstream server as TCP packets downstream? If that is not possible due to message parsing, then Pgpool-II could detect when the OS says there is no more data ready to be read from the upstream connection (i.e. that you would need to wait for more data packets to arrive) and *then* flush the downstream buffer. The upstream PostgreSQL server has already packed messages into TCP packets as it deemed efficient and permitted by the application-level protocol, so directly replaying flushes downstream would not lose any network efficiency compared to a direct connection between the client and the upstream server.


-- 
Best regards,
Oleg Oshmyan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.pgpool.net/pipermail/pgpool-general/attachments/20220110/4749c173/attachment.htm>