Skip to content

Remove timeout translation from NpgsqlReadBuffer#6126

Merged
vonzshik merged 1 commit intomainfrom
6122-read-buffer-timeout-translation-fix
Oct 5, 2025
Merged

Remove timeout translation from NpgsqlReadBuffer#6126
vonzshik merged 1 commit intomainfrom
6122-read-buffer-timeout-translation-fix

Conversation

@vonzshik
Copy link
Contributor

@vonzshik vonzshik commented Jun 2, 2025

Fixes #6122

@vonzshik vonzshik force-pushed the 6122-read-buffer-timeout-translation-fix branch from 180f367 to 50939cc Compare September 10, 2025 11:38
@vonzshik vonzshik marked this pull request as ready for review October 1, 2025 12:17
@vonzshik vonzshik requested a review from roji as a code owner October 1, 2025 12:17
ReadBuffer.Timeout = TimeSpan.FromSeconds(command?.CommandTimeout ?? Settings.CommandTimeout);
// But the next time, we call the Prepare, which doesn't set its own timeout
var timeoutSeconds = command?.CommandTimeout ?? Settings.CommandTimeout;
ReadBuffer.Timeout = timeoutSeconds > 0 ? TimeSpan.FromSeconds(timeoutSeconds) : Timeout.InfiniteTimeSpan;
Copy link
Member

Choose a reason for hiding this comment

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

Just confirming: from the user's perspective, we're retaining the meaning of CommandTimeout=0 to mean infinite/no timeout, right? (both in the connection string and in NpgsqlCommand.CommandTimeout). In those contexts, and "immediate timeout" makes no sense in any case.

(It's just slightly confusing that we have two contexts/meanings: one user-facing one where 0 means infinite and another internal one where it means immediate. I get why it's like this and it makes sense - maybe good to make it slightly clearer in comments here and there)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just confirming: from the user's perspective, we're retaining the meaning of CommandTimeout=0 to mean infinite/no timeout, right? (both in the connection string and in NpgsqlCommand.CommandTimeout). In those contexts, and "immediate timeout" makes no sense in any case.

Correct, that's why we pass Timeout.InfiniteTimeSpan if command's timeout is 0, since now instead of NpgsqlReadBuffer doing that for us, we have to do that ourselves at each call. Not ideal but then our COPY operations allow an essentially direct access to read buffer's timeout, and it's only two places, so...

@vonzshik vonzshik merged commit 5fd06df into main Oct 5, 2025
16 checks passed
@vonzshik vonzshik deleted the 6122-read-buffer-timeout-translation-fix branch October 5, 2025 19:12
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.

Setting NpgsqlReadBuffer.Timeout property to Timeout.InfiniteTimeSpan gets overridden in setter

2 participants