Sitelet https://github.com/npgsql/npgsql/issues/4180
Skip to content

Exclusive connection openers get starved in multiplexing over-capacity mode #4180

Description

@NinoFloris

Today over capacity mode is just looping over all connectors - idle or busy - without duly guarding against them being in use by, or concurrently starting, some binding scope like a transaction.

Activity

  1. vonzshik commented on Nov 30, 2021

    @vonzshik
    Contributor

    I took another look at this: shouldn't this lines guard specifically against running a multiplexing query on an idle connector?

    // We may be in a race condition with the connector read loop, which may be currently returning
    // the connector to the Idle channel (because it has completed all commands).
    // Increment the in-flight count to make sure the connector isn't returned as idle.
    var newInFlight = Interlocked.Increment(ref connector.CommandsInFlightCount);
    if (newInFlight == 1)
    {
    // The connector's in-flight was 0, so it was idle - abort over-capacity read
    // and retry the normal flow.
    Interlocked.Decrement(ref connector.CommandsInFlightCount);
    spinwait.SpinOnce();
    continue;
    }

    (Also, we probably shouldn't grab a connector with 0 in-flight queries, like you've done in #4179)

  2. NinoFloris commented on Nov 30, 2021

    @NinoFloris
    MemberAuthor

    You're probably right (though the impact of all this code is really hard to follow across the codebase so I'm not 100% sure).

    Either way it'll be really inefficient if the write loop encounters a lot of binding scope connectors. They're not idle (so cannot be rented by the earlier TryGetIdleConnector) but they're not available for multiplexing either as they'll always fail this check.

    As it stands we could even make the multiplexing loop spinloop forever by holding all connectors in a binding scope.

  3. modified the milestones: Backlog, 7.0.0 on Dec 31, 2021
  4. roji commented on Dec 31, 2021

    @roji
    Member

    @NinoFloris @vonzshik I remember thinking about this stuff and also doing some stress-testing over over-capacity mode, though it's true we generally don't exercise it much (we obviously never do over-capacity in the TechEmpower benchmarks). I think this code should be OK, but it's worth taking another look at some point.

    Either way it'll be really inefficient if the write loop encounters a lot of binding scope connectors. They're not idle (so cannot be rented by the earlier TryGetIdleConnector) but they're not available for multiplexing either as they'll always fail this check.

    This is true... I remember considering another data structure only for unbound connectors, but it wasn't trivial.

    Am putting this in 7.0 to think about.

  5. vonzshik commented on Dec 31, 2021

    @vonzshik
    Contributor

    I was thinking about making a separate pool for the case whenever a connection tries to bind a connector.

  6. roji commented on Dec 31, 2021

    @roji
    Member

    Interesting idea - I like it. I don't remember any more what seemed complex about separating bound and unbound connectors, but it's definitely worth revisiting.

  7. roji commented on Sep 11, 2022

    @roji
    Member

    General direction by @NinoFloris... In multiplexing, when someone gets to RentAsync, that means that there was no idle connection. At that point, we can pick some connection and flag it as reserved for this usage. This makes us refrain from pushing more over-capacity commands into it, which will eventually make it available. When that happens, the connection gets pushed into the idle connector channel.

  8. changed the title [-]Guard against concurrent use in multiplexing over capacity mode and connection binding scopes[/-] [+]Exclusive connection openers get starved in multiplexing over-capacity mode[/+] on Sep 25, 2022
  9. modified the milestones: 7.0.0, 8.0.0 on Nov 3, 2022
  10. modified the milestones: 8.0.0, 9.0.0 on Nov 7, 2023
  11. modified the milestones: 9.0.0, 10.0.0 on Oct 21, 2024
  12. added theissue type on Jun 14, 2025
  13. modified the milestones: 10.0.0, 11.0.0 on Nov 2, 2025
  14. NinoFloris commented on Mar 14, 2026

    @NinoFloris
    MemberAuthor

    Multiplexing was removed in #6457

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions