Stabilize Windows CI - #3168
Open
mgravell wants to merge 2 commits into
Open
Conversation
Cluster tests set connectTimeout=10000 and only skip once a connection attempt has failed, so with no cluster running each of ~50 tests burned 10 seconds before reporting a 1ms skip. Sentinel was the same shape via SentinelBase.InitializeAsync, which polls for 15 seconds per test and then reports a failure rather than a skip. Add Skip.IfNoServer, backed by a single short TCP connect per endpoint cached for the run, and route the six cluster configuration overrides through TestBase.GetClusterConfiguration so the probe happens before a configuration is handed out. This only reports whether anything is listening: a server that is up but unreachable still fails, as it must. Also, while in SentinelBase: wait on the primary connection rather than only the sentinel connection (the retry loop already used the former as its success condition, but the assert checked the latter, so it could pass having never reached the primary), and give the assert a message. Full suite against a primary/replica/secure/failover-only topology goes from 10m34s with 27 failures to 49s with none attributable to the missing servers; against the full topology the skip count is unchanged at 150, confirming nothing is newly suppressed. CI: the Windows job now starts that smaller topology by default, since the cluster and sentinel instances gossip continuously and that job is a fractional vCPU running Windows running WSL running redis. Dispatch with windows-topology=full for everything. Also allows pinning the apt redis version instead of silently testing whatever is GA, and polls for readiness rather than sleeping a fixed 5 seconds.
The wsl-bash shell runs with -euo pipefail, so `out=$(redis-cli ... PING)` aborts the entire step the first time a server is not yet accepting connections - which is the exact state the poll exists to wait out. The previous code never hit this because every redis-cli call was guarded with `|| echo ...`. Effect on the last run: the step died 41ms after launching the servers, continue-on-error painted it green, and the suite then ran against a Redis that wasn't there - thousands of "not possible to connect" failures across MultiPrimaryTests, RespProtocolTests and friends, ten minutes later and nowhere near the cause. So: tolerate the expected probe failure, and drop continue-on-error in favour of failing this step explicitly when a required port never comes up. One clear "no server on: 6379" beats several thousand connection errors in a later step.
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.
Windows CI is runs on a potato. Maybe half a potato. One of the little ones. Additionally, it runs against an uncontrolled server (
apt-getwhatever is GA). The primary build is Ubuntu - we definitely want to retain Windows checks, but right not it is failing more times than not, with most builds taking 4+ runs, which is not ideal.The potato is a VM running Windows running WSL running Ubuntu running ¿16? Redis instances, many of which all chatter to each-other in spiderwebs (cluster with replicas, etc). It is ambitious and unrealistic to "fix" this.
So!
apt-getChecklist