Memoize Connection#rootless? correctly - #608
Open
tas50 wants to merge 1 commit into
Open
Conversation
`@rootless ||= (info['Rootless'] == true)` cannot cache a false result, and
#info is not memoized, so every call against a non-rootless daemon issued a
fresh /info request:
5x Docker.rootless? -> 5 /info requests
Test for the ivar with defined? instead. Every daemon that is not rootless is
now one request rather than one per call:
5x Docker.rootless? -> 1 /info request
Also adds a guard for #podman?, which uses the same `||=` idiom but is already
backed by the memoized #version, so it makes a single /version request. The spec
pins that behaviour so it does not regress into the same trap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@rootless ||= (info["Rootless"] == true)cannot cache a false result —||=re-evaluates whenever the memo isfalseornil.#infois not memoized either, so every call against a non-rootless daemon issued a fresh/inforequest.Measured against a real daemon before the fix:
After:
The fix
Test for the ivar with
defined?rather than relying on truthiness.Also
Adds a guard spec for
#podman?. It uses the same||=idiom, but is backed by the memoized#version, so it already makes only one/versionrequest. The spec pins that so it cannot regress into the same trap. No behaviour change there.Verification
The new
only queries /info oncespec was confirmed to fail before the change:connection_spec goes from 13 examples to 17, with no new failures.
Note on CI
The one failing example on this branch (
its(:resource)) is pre-existing on master — I confirmed master alone reports13 examples, 1 failurefor the same spec. It is fixed by #605, and CI here will stay red until that lands.