Skip to content

Conversation

@NinoFloris
Copy link
Member

No description provided.

Fix style issues

### Synchronous command batching can cause threadpool starvation

To allow Npgsql to write command batches containing large data payloads (generally parameter data) synchronously, without running into PostgreSQL limitations, it runs any commands after the first asynchronously. This introduces sync over async and can cause threadpool starvation at higher loads. In Npgsql versions prior to 8.0 a workaround was in place that would mitigate the effects of this sync over async, allowing most higher loads to run succesfully. This workaround was removed to make room for the far reaching NativeAOT refactor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably add a few examples how threadpool starvation affects users, like delayed query execution time and even timeouts.

* Use [ThreadPool.SetMinThreads()](https://learn.microsoft.com/en-us/dotnet/api/system.threading.threadpool.setminthreads?view=net-8.0) to expand the minimum pool of threads available for blocking.
* Refactor code using command batches to use asynchronous apis.

If these options are not suitable for your project you can consider opening an issue on [https://github.com/npgsql/npgsql](https://github.com/npgsql/npgsql). This helps us prioritize the effort to bring back some mitigation in a patch version.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe direct people to a specific issue (like the one that triggered this breaking change note) to avoid new duplicate issues getting opened etc.


If these options are not suitable for your project you can consider opening an issue on [https://github.com/npgsql/npgsql](https://github.com/npgsql/npgsql). This helps us prioritize the effort to bring back some mitigation in a patch version.

To clarify, Npgsql 8.0 has not dropped support for synchronous command batches completely; the feature is entirely supported for use at low load or for local data exploration.
Copy link
Member

Choose a reason for hiding this comment

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

Is that maybe a bit too harsh? Given the workaround (e.g. SetMinThreads()) it's possible to do this even at higher load, it's just up to the user to preconfigure things etc?

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.

4 participants