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

Make *client.Client safe for concurrent use #43738

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Commits on Jun 23, 2022

  1. client: test using public Client constructor

    Signed-off-by: Cory Snider <csnider@mirantis.com>
    corhere committed Jun 23, 2022
    Configuration menu
    Copy the full SHA
    6ccceb6 View commit details
    Browse the repository at this point in the history

Commits on Jun 24, 2022

  1. client: make safe for concurrent use

    The API version is the only stateful thing in the client. It has
    historically had no locking or synchronization, making the client de
    facto unsafe for concurrent use. Making the client concurrency-safe is
    not exactly a trivial undertaking, however: the client uses the API
    version when building requests and handling the responses, so care must
    be taken to ensure that the responses to any requests be handled with
    the semantics of the same API version that was used when building the
    request, even if the client's API version is changed while the request
    is in-flight. Merely converting all accesses to the version field to
    atomic loads and stores would not be sufficient. The client also has an
    optional feature to lazily auto-negotiate the API version on the first
    API request, so care also needs to be taken to suppress redundant
    auto-negotiation attempts while an auto-negotiation is in progress.
    
    Introduce the concept of a "versioned client" to the client internals
    which encapsulates a point-in-time snapshot of the client's configured
    API version. Refactor the client methods to perform all version-
    dependent operations through a versioned-client instance to ensure that
    they always operate on an immutable snapshot of the version. Move
    version auto-negotiation into the versioned-client constructor and
    synchronize accesses to the version state to prevent data races and
    redundant auto-negotation attempts. Exclusive access to the version
    state is held by the goroutine throughout the entire duration of the
    auto-negotiation process, blocking the construction of new versioned
    clients or starting any concurrent negotiation processes until the
    auto-negotiation process completes.
    
    With auto-negotiation now performed explicitly in the versioned-client
    constructor rather than implicitly in the high-level HTTP request helper
    methods, it is now possible to implement Ping() in terms of those high-
    level helpers. Simplify the Ping implementation through the use of an
    "unversioned" versioned client (i.e. with an empty version) and the
    aforementioned high-level helpers.
    
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    corhere committed Jun 24, 2022
    Configuration menu
    Copy the full SHA
    632e667 View commit details
    Browse the repository at this point in the history

Commits on Jul 4, 2022

  1. client: retry version negotiation after error

    When a client is configured for API version negotiation, it would try
    exactly once to ping the server, then unconditionally configure the
    client API version and set the negotiated flag. Any failure to ping the
    server would result in the client "negotiating" the fallback API version
    v1.24, even if the request never reached the daemon. Transient network
    issues or simply calling a client method with an already-canceled
    context would be sufficient to cause this to happen.
    
    Modify the client's negotiation process to retry negotiation on each
    subsequent request until it receives a ping response from the daemon.
    Only responses with HTTP status codes in the range [1, 500] are
    considered to be ping responses from the daemon as HTTP status codes
    >= 501 (e.g. 502 Gateway Timeout, 503 Service Unavailable) could be
    returned by intermediate hops.
    
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    corhere committed Jul 4, 2022
    Configuration menu
    Copy the full SHA
    2d4806a View commit details
    Browse the repository at this point in the history