Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add flush request in pipeline #2200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zenkovev
Copy link

Hello, there is a suggestion to add the ability to send a Flush message in Pipeline mode. This feature is available in Pipeline mode in the libpq library, in the PostgreSQL documentation this method is proposed for executing dependent commands, as an alternative to transaction mode for some cases. As for my need, I want to create a test scenario for such case using the library. Also, this feature may close issue #2025.

@jackc
Copy link
Owner

jackc commented Dec 21, 2024

I think adding the ability to send a Flush is a good feature. But GetResultsNotCheckSync is a pretty awkward addition to the API. It would be really nice if that was not needed. If I understand correctly, the reason for this new method is because you may have sent messages that will not trigger a ReadyForQuery such as a Parse or Describe but you still want to read their response. But GetResults is designed to return nil, nil when the pipeline is empty.

Would it be reasonable to track the expected responses from non-ReadyForQuery triggering messages and fold this behavior into GetResults?

@zenkovev
Copy link
Author

Yes, I agree that GetResultsNotCheckSync is not a good feature for the api. Yes, it is needed to receive messages after Flush without sending Sync, since in this mode only Sync triggers ReadyForQuery. From the point of view of the protocol, there are such cases of tracking other messages:

  • Parse -> ParseComplete (or ErrorResponse)
  • Bind -> BindComplete (or ErrorResponse)
  • Execute -> CommandComplete, PortalSuspended (or EmptyQueryResponse, ErrorResponse)
  • Sync -> ReadyForQuery (or ErrorResponse)
  • Describe -> RowDescription, NoData (or ErrorResponse)
  • Close -> CloseComplete (or ErrorResponse)

Also, this adds another level: we only need to track messages before Flush or Sync. In this case, it is possible that the public Flush as a function should be combined with the Flush as a message, so as not to overcomplicate the internal logic. As a possible solution, two counters can be added waiting for messages in the buffer and waiting for sent messages. This variant, at least when considered, looks like a working.

@jackc
Copy link
Owner

jackc commented Dec 24, 2024

This does get tricky. What we're trying to do is is stay synced with the server without using a Sync point. But the difficulty of doing that is part of why Sync points exist.

But perhaps there is an approach that isn't too complicated. The pipeline system doesn't operate at the protocol message level. It operates at a slightly higher level. For example, there is no way to only send a Bind message. That is sent by SendQueryParams and SendQueryPrepared. Likewise, GetResults does not operate at the level of protocol messages. It returns a *ResultReader, *StatementDescription, or *PipelineSync.

Perhaps just as expectedReadyForQueryCount corresponds with *PipelineSync we should have expected* for ResultReader and StatementDescription. Errors might complicate this, but since errors abort everything until the next Sync, it might be as simple as zeroing the other counters. Except it's possible to have multiple Syncs and Flushes pending...

Yuck... this is really hard. I think we would actually need to have a record of everything sent so we could match it with what we expect to receive. That's more complicated than I'd like. But the alternative seems to be something like what you did originally: a way of getting results that doesn't know if there are any results pending or not.

🤷

@zenkovev
Copy link
Author

I thought about this question, as a result, I can suggest the following construction. We can add a queue, the elements of which will contain three fields: the type of message (in the sense of the user, i.e. Prepare, QueryParams, QueryPrepared, etc), a flag indicating whether message was sent to the server, a flag indicating whether there was a Flush or Sync as a message after (that is, we can expect a response from the server). Then, it seems, it will be readable and transparent in terms of processing logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants