Repository: moby/moby
File: client/client.go (Go)

Code before:
// checkVersion manually triggers API version negotiation (if configured).
// This allows for version-dependent code to use the same version as will
// be negotiated when making the actual requests, and for which cases
// we cannot do the negotiation lazily.
func (cli *Client) checkVersion(ctx context.Context) error {
	if !cli.manualOverride && cli.negotiateVersion && !cli.negotiated {
		// Acquire the negotiateLock to ensure exclusive access to versionNegotiator.
		// This prevents concurrent negotiations.
		cli.negotiateLock.Lock()
		defer cli.negotiateLock.Unlock()

		if cli.versionNegotiator == nil {
			// The current context is passed to the Ping method to respect any deadlines or cancellations.
			cli.versionNegotiator = sync.OnceValue(func() error {
				ping, err := cli.Ping(ctx)
				if err != nil {
					return err
				}

				cli.negotiateAPIVersionPing(ping)

				return nil
			})
		}

		err := cli.versionNegotiator()

		// If an error occurred during version negotiation, unset the versionNegotiator
		// to allow for subsequent attempts. This ensures that future calls to checkVersion
		// can retry the negotiation if necessary.
		if err != nil {
			cli.versionNegotiator = nil
		}

		return err
	}
	return nil
}

// getAPIPath returns the versioned request path to call the API.
// It appends the query parameters to the path if they are not empty.
func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) string {
	var apiPath string
	_ = cli.checkVersion(ctx)
	if cli.version != "" {
		v := strings.TrimPrefix(cli.version, "v")
		apiPath = path.Join(cli.basePath, "/v"+v, p)
	} else {
		apiPath = path.Join(cli.basePath, p)
	}
	return (&url.URL{Path: apiPath, RawQuery: query.Encode()}).String()

Review comment (performance):
`cli.versionNegotiator()` is only ever called inside a `cli.negotiateLock` critical section, meaning at most one goroutine can call it at a time. So there is no need for the synchronization behavior of `sync.OnceValue`. And there is no need to remember the error returned by `cli.Ping()` because, if the ping is successful, `cli.negotiated` gets set to `true` by `cli.negotiateAPIVersionPing()`. It looks to me like `sync.OnceValue` is not useful in this situation; adding complexity without providing any benefit in return.

Code after:
// be negotiated when making the actual requests, and for which cases
// we cannot do the negotiation lazily.
func (cli *Client) checkVersion(ctx context.Context) error {
	if !cli.manualOverride && cli.negotiateVersion && !cli.negotiated.Load() {
		// Ensure exclusive write access to version and negotiated fields
		cli.negotiateLock.Lock()
		defer cli.negotiateLock.Unlock()

		// May have been set during last execution of critical zone
		if cli.negotiated.Load() {
			return nil
		}

		ping, err := cli.Ping(ctx)
		if err != nil {
			return err
		}
		cli.negotiateAPIVersionPing(ping)
	}
	return nil
}

// getAPIPath returns the versioned request path to call the API.
// It appends the query parameters to the path if they are not empty.
func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) string {
	var apiPath string
	_ = cli.checkVersion(ctx)
	if cli.version != "" {
		v := strings.TrimPrefix(cli.version, "v")
		apiPath = path.Join(cli.basePath, "/v"+v, p)
	} else {
		apiPath = path.Join(cli.basePath, p)
	}
	return (&url.URL{Path: apiPath, RawQuery: query.Encode()}).String()
}

// ClientVersion returns the API version used by this client.
func (cli *Client) ClientVersion() string {
	return cli.version
}

// NegotiateAPIVersion queries the API and updates the version to match the API
// version. NegotiateAPIVersion downgrades the client's API version to match the
// APIVersion if the ping version is lower than the default version. If the API
// version reported by the server is higher than the maximum version supported
// by the client, it uses the client's maximum version.
//
// If a manual override is in place, either through the "DOCKER_API_VERSION"
// ([EnvOverrideAPIVersion]) environment variable, or if the client is initialized
// with a fixed version ([WithVersion]), no negotiation is performed.
//