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

sql/pgwire: buffer messages during COPY TO and startup #99761

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Mar 28, 2023

fixes #97314
backport fixes #99546


sql: buffer COPY OUT data

Rather than sending each COPY result row one-by-one, now the data will
get buffered, then flushed when the buffer size limit is reached or when
sending ReadyForQuery.

This fixes an issue that was causing the ruby-pg test to hang, since it assumes
the data will be buffered.


pgwire: buffer startup messages when creating connection

This avoids sending each ParameterStatus one-by-one.


sql: refactor CopyIn handling

This makes it so we don't need to manually send a CommandComplete.
Instead, when the CopyInResult is closed, CommandComplete will be sent,
similar to how it works for other message types.

Release note: None

@rafiss rafiss added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Mar 28, 2023
@rafiss rafiss requested review from otan and a team March 28, 2023 05:05
@rafiss rafiss requested a review from a team as a code owner March 28, 2023 05:05
@blathers-crl
Copy link

blathers-crl bot commented Mar 28, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fancy

@rafiss rafiss force-pushed the buffer-copy-data branch 2 times, most recently from e9fd3a1 to f281dce Compare March 28, 2023 19:18
Rather than sending each COPY result row one-by-one, now the data will
get buffered, then flushed when the buffer size limit is reached or when
sending ReadyForQuery.

Release note: None
This avoids sending each ParameterStatus one-by-one.

Release note: None
This makes it so we don't need to manually send a CommandComplete.
Instead, when the CopyInResult is closed, CommandComplete will be sent,
similar to how it works for other message types.

Release note: None
@rafiss
Copy link
Collaborator Author

rafiss commented Mar 29, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 29, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 29, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 29, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 30, 2023

Build succeeded:

@blathers-crl
Copy link

blathers-crl bot commented Mar 30, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error setting reviewers, but backport branch blathers/backport-release-23.1-99761 is ready: POST https://api.github.com/repos/cockroachdb/cockroach/pulls/100029/requested_reviewers: 422 Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the cockroachdb/cockroach repository. []

Backport to branch 23.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@otan otan removed request for a team March 30, 2023 06:38
@otan
Copy link
Contributor

otan commented Mar 30, 2023

blathers backport 23.1

@blathers-crl
Copy link

blathers-crl bot commented Mar 30, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating backport branch refs/heads/blathers/backport-release-23.1-99761: POST https://api.github.com/repos/cockroachdb/cockroach/git/refs: 422 Reference already exists []

Backport to branch 23.1 failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@rafiss rafiss deleted the buffer-copy-data branch March 30, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachtest: ruby-pg failed roachtest: ruby-pg failed
3 participants