The purpose-prefixed labels (hanzo-build-*, hanzo-deploy-*, hanzo-docs-build-*)
are retired in favour of the bare <os>-<arch> every runner already advertises.
They were not a capability, only a habit, and the habit cost builds: papers asked
for hanzo-build-linux-amd64, no online runner offered it, and two idle
linux-amd64 runners sat beside the queue while the build simply never ran. The
lux tree lost three days to lux-linux-amd64 the same way.
What a job is FOR is the workflow's name. runs-on is the machine it needs.
Co-authored-by: Hanzo Dev <[email protected]>
First real runs on this fleet turned up three things the workflows had been
getting for free from GitHub's hosted image.
golangci-lint had no setup-go step. The linter shells out to `go` to load
packages, and the job image does not carry a toolchain on PATH, so it exited on
"go command required, not found" without reading a file. It sets up Go from
go.mod now, like every other job here.
doctests ran setup-go before checkout, so go.mod was not on disk when the action
looked for it and the module cache degraded to a permanent miss. Checkout goes
first.
The e2e proxy builder was pinned to golang:1.24-alpine while go.mod requires
1.26.4. The official golang images set GOTOOLCHAIN=local, which makes the image
the authority and refuses a newer directive, so `go mod download` stopped the
compose build and took both docker.start and docker.e2e.start with it. The base
moves up to 1.26-alpine; the version stays declared in exactly one place, go.mod,
with the image as its floor.
govulncheck is reporting three standard-library advisories against this module.
That is a real finding and it stays visible.
Every leg of test-redis-ce brings up a real Redis container, so a red leg can
mean the environment rather than the code. workflow_dispatch makes confirming
which one a button instead of an empty commit.
The runner labels here were already right, but the files sat in
.github/workflows, the second entry in WORKFLOW_DIRS. Second is only read while
the first is absent, so every one of them was one .hanzo/workflows commit from
going silent. Six move across; four do not.
Moved: build.yml, doctests.yaml, golangci-lint.yml, govulncheck.yml,
spellcheck.yml, test-e2e.yml. The runners carry a docker daemon, so the
compose-backed suites (make docker.start, make docker.e2e.start) and the
`services:` block in doctests work as written. .github/actions/run-tests stays
where it is -- it is a composite action, not a workflow, and the `./` path
resolves from the repo root regardless of which directory the caller lives in.
Two adjustments inside the files that moved:
build.yml drops the benchmark job and samples the matrix at three Redis
versions instead of five. `make bench` measures without comparing to a
baseline, so it produced no pass/fail signal for thirty runner-slots per push
on a pool every org shares. What the sampling gives up is a break specific to
Redis 8.6 or 8.2, or to the 1.25.x pin now covered by oldstable/stable. The
Codecov upload goes with it: CODECOV_TOKEN is not a secret this forge holds,
so coverage is produced and no longer reported to a dashboard.
govulncheck.yml asked setup-go for "1.26.4.x", which is not a version it can
resolve. It reads go.mod now, which is where the toolchain version is already
decided.
Dropped, with what each cost:
codeql-analysis.yml uploaded SARIF to GitHub code scanning. There is no
such service here, and govulncheck covers the
vulnerability half against the Go advisory database.
release-drafter.yml assembled GitHub Release drafts from merged pull
requests. Release notes are written by hand.
Its .github/release-drafter-config.yml goes too;
nothing else reads it.
stale-issues.yml aged out issues and pull requests through the GitHub
issues API. Triage is manual.
test-redis-enterprise.yml needed the licensed redislabs/redis Enterprise
image and a clone of RedisLabs/redis-ee-docker. It was
already `if: false` upstream, so it gated nothing;
Redis Enterprise remains untested by CI.
The tree carried two compose files. Docker Compose v2 resolves by a fixed
precedence -- compose.yaml, compose.yml, docker-compose.yaml,
docker-compose.yml -- so compose.yml won every time and docker-compose.yml,
the one upstream merges actually keep current, was never read:
warning Found multiple config files with supported names:
.../compose.yml, .../docker-compose.yml
warning Using /Users/z/work/hanzo/kv-go/compose.yml
compose.yml was a 106-line snapshot taken before several upstream updates.
It pinned CLIENT_LIBS_TEST_IMAGE at 8.4.0 while the Makefile asks for 8.8.0,
and it was missing three services and a whole profile. Two things were
therefore broken and quiet about it:
* `make docker.e2e.start` runs `docker compose --profile e2e up`. There is
no e2e profile in the stale file, so it started nothing;
.github/workflows/test-e2e.yml then sat waiting for cae-resp-proxy on
:18100 until the timeout.
* `make docker.start` (--profile all) never started osscluster-tls, so
tls_cluster_test.go -- whose own comment says the ports come from the
compose file -- had no server on 5430.
Deleting the stale copy and giving the live one the canonical name (this
workspace uses compose.yml, never docker-compose.yml) leaves exactly one
compose file. `docker compose --profile e2e config --services` now returns
redis, cae-resp-proxy and proxy-fault-injector; --profile all also brings up
osscluster-tls. The three comments that named the old path now name the new
one. go build ./... clean.
Co-authored-by: Hanzo Dev <[email protected]>
When a connection is both not usable and has a bad conn error,
releaseConn would call reconnect twice — first establishing a new
connection, then immediately closing it to create another. Add early
return after the first reconnect to avoid the wasted connection.
* fix(test): make waitForSentinelClusterStable robust to disconnected replicas
CI on master sporadically fails the universal_test.go spec
"should connect to failover servers on slaves when readonly Options is ok"
with `Expected master to equal slave`. The flake reproduces only when the
random spec order puts the sentinel-failover Describe block before the
universal_test spec.
Root cause is in waitForSentinelClusterStable's post-conditions, not in
production code:
1. It only checks `len(replicas) >= 2`. Production
(parseReplicaAddrs in sentinel.go) filters out replicas flagged
`s_down`, `o_down`, or `disconnected`. Post-failover, replicas
often linger as `disconnected` for several seconds after Sentinel
agrees on the master/replica counts, so the stabilization check
can pass while production sees zero usable replicas.
2. It then waits a fixed 10s. With no link between that sleep and
the actual precondition (a FailoverClient(ReadOnly:true) finding
a routable replica), the test gambles that 10s is enough on every
CI host. When it isn't, RandomReplicaAddr silently returns the
master and the spec asserts on the wrong role.
Two changes:
- Add countUsableReplicas, mirroring parseReplicaAddrs's filter, and
require each sentinel to report at least 2 usable (non-down,
non-disconnected) replicas with consistent counts across sentinels.
- Replace the 10s sleep with a bounded Eventually that opens the same
kind of client the failing spec uses (UniversalClient + MasterName +
ReadOnly:true) and confirms ROLE returns "slave". This is the
deterministic equivalent of waiting "long enough" — it returns as
soon as the actual precondition holds, and bounds at 30s if the
cluster never stabilizes.
No production code changes; the flake is entirely in test harness
post-conditions. The new probe uses only public API and existing
package-level fixtures (sentinelName, sentinelAddrs).
* fix(test): harden sentinel ReadOnly spec against silent master fallback
CI on master sporadically fails the universal_test.go spec
"should connect to failover servers on slaves when readonly Options is ok"
with `Expected master to equal slave`. Two random-spec-ordering bug
paths combine to produce this:
1. sentinel_test.go has three failover-causing specs across three
Describe blocks ("Sentinel", "NewFailoverClusterClient", and
"Sentinel Failover with Conns"). Only the third runs Ordered/Serial
with an AfterAll that calls waitForSentinelClusterStable. The
first two can land right before the universal_test spec under any
random spec ordering and leave the cluster mid-recovery.
2. FailoverClient(ReadOnly:true) routes via RandomReplicaAddr, which
silently falls back to the master when Sentinel reports zero
usable replicas. parseReplicaAddrs in sentinel.go filters out
replicas flagged s_down, o_down, or disconnected — and freshly
promoted/demoted replicas often linger as "disconnected" for
several seconds after Sentinel agrees on master/replica counts.
Two test-only changes (no production code touched):
a) waitForSentinelClusterStable: previously only checked
len(replicas) >= 2, which can pass while every replica is
filtered out by production code. Now mirrors parseReplicaAddrs's
filter via countUsableReplicas and requires each sentinel to
report >= 2 usable replicas with consistent counts across
sentinels. The fixed 10s sleep is replaced with a bounded
Eventually that opens the same UniversalClient(ReadOnly:true)
the failing spec uses and confirms ROLE returns "slave" — the
deterministic equivalent of "wait long enough".
b) universal_test.go spec: defense in depth. Since the failover
specs in (1) don't all share an AfterAll, the spec itself
retries client creation + ROLE in a bounded Eventually. Each
iteration closes the old client and creates a new one so
RandomReplicaAddr is re-evaluated against current Sentinel
state. Bounded at 30s so a genuinely broken setup still fails.
The (a) fix keeps the post-failover state clean for any downstream
test; the (b) fix keeps this specific spec correct regardless of
ordering. Together they make the spec deterministic across all 1414!
possible random spec orderings.
* fix(test): make ReadOnly SET assertion atomic with the ROLE probe
Previous attempt fixed the ROLE check ("Expected master to equal slave")
by retrying client creation inside Eventually until ROLE returned
"slave". CI surfaced a second failure mode in the same spec:
[FAILED] Expected an error to have occurred. Got: <nil>: nil
universal_test.go:110
The spec's intent was: ReadOnly client lands on a replica, and a write
against that replica returns READONLY. The new failure means the write
succeeded — there's a second path back to the master:
SET on replica → READONLY error
→ shouldRetry returns true for READONLY (error.go)
→ conn discarded, retried via masterReplicaDialer
→ RandomReplicaAddr called again, may silently fall back to master
when no usable replica is reported (same root cause as the original
flake — Sentinel sees 2 replicas but both are "disconnected")
→ SET hits the master, succeeds, err is nil
→ Expect(err).To(HaveOccurred()) fails
Two changes to make the spec atomic and bypass-proof:
- Set MaxRetries: -1 on the test client. A READONLY reply now
surfaces as the first/only SET error instead of triggering a
retry-dial that may pick the master. This isolates what the test
is actually asserting: a single round-trip to whatever node the
dialer picked.
- Move the SET assertion inside the Eventually closure together
with the ROLE check. If either step lands on the master (because
RandomReplicaAddr fell back at dial time), the closure returns an
error, Eventually closes the client and retries with a fresh one
so the dialer re-queries Sentinel. The full assertion is therefore
atomic — both ROLE and SET observe the same dial.
Error messages from the closure name the precise failure mode so a
genuinely broken setup (timeout after 30s) reports something more
useful than "expected slave, got master".
No production code changes; the retry-on-READONLY behaviour is correct
for normal failover scenarios and stays untouched.
---------
Co-authored-by: ofekshenawa <[email protected]>
* fix: do not close sentinel when replicas list is empty
When a sentinel-monitored Redis master has no replicas (master-only
setup), replicaAddrss() was calling closeSentinel() on receiving an
empty replica list without error. This destroyed the sentinel
connection (and its pub/sub failover subscription), forcing a full
sentinel rediscovery on every subsequent operation — creating a
continuous rediscovery loop that flooded logs and added latency.
The empty-replica case is a valid, stable state for master-only
deployments. The sentinel connection must be preserved for ongoing
master discovery and failover monitoring via pub/sub.
The fix returns early with an empty slice instead of tearing down
the sentinel. A test is added to verify the sentinel survives
repeated ReplicaAddrs calls when no replicas exist.
Fixes: sentinel rediscovery loop on master-only clusters
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: preserve UseDisconnectedReplicas fallback path
Address review feedback: the early return for zero replicas must only
apply when useDisconnected=false (the normal path). When
useDisconnected=true, fall through to the discovery loop which
correctly passes useDisconnected to parseReplicaAddrs — since
getReplicaAddrs hardcodes false, the cached sentinel path can never
find disconnected replicas.
Also remove unused SetSentinelForTest helper.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* test: use SENTINEL MONITOR to create a true zero-replica master
Address review feedback: the previous test used sentinelName which
has two replicas in the test environment, so getReplicaAddrs returned
a non-empty list and the zero-replica code path was never exercised.
Now the test registers a temporary master-only name via SENTINEL
MONITOR (pointing to the existing master port but with a new name
and no replicas), verifies that ReplicaAddrs returns empty without
closing the sentinel, and cleans up via SENTINEL REMOVE.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* test: skip zero-replica sentinel test when infrastructure unavailable
Add a connectivity check at the start of the test so it is skipped
(not failed) in environments where sentinel processes are not running.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* test: use standalone Redis (no replicas) for zero-replica sentinel test
Point SENTINEL MONITOR at the standalone Redis instance (redisPort)
instead of sentinelMasterPort, which already has two replicas. This
prevents Sentinel from discovering replicas via INFO REPLICATION and
ensures the test reliably exercises the zero-replica code path.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* ci: re-trigger checks
* ci: re-trigger checks (flaky PubSub ClientSetName and conn_state tests)
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Nedyalko Dyakov <[email protected]>
Add a dedicated GitHub Actions workflow to run govulncheck.
- scans all Go modules in the repository
- collects results across modules instead of stopping on first failure
- fails explicitly if no modules are discovered
- ensures scans run only after successful directory change
- uses a fixed govulncheck version for reproducibility
- runs on Go 1.26.x to avoid failures caused by known vulnerabilities in older toolchains
Co-authored-by: Nedyalko Dyakov <[email protected]>
In ClusterClient.SSubscribe, the per-PubSub newConn closure picks the
target node by hash slot of channels[0]. PubSub.conn() was only passing
c.channels (regular SUBSCRIBE) into that closure — sharded SSubscribe
channels stored in c.schannels were silently dropped.
For an SSubscribe-only PubSub (the common ClusterClient.SSubscribe case)
this meant the channel list was empty on reconnect, so the closure fell
through to nodes.Random(). The reconnected SSUBSCRIBE went to the wrong
shard, the MOVED reply was never read by the write-only resubscribe
path, and the PubSub looked healthy (Ping succeeds, Receive returns)
while delivering no messages. On an N-shard cluster the failure mode hit
(N-1)/N reconnects.
Add c.schannels to the channel list built in PubSub.conn(). c.patterns
is intentionally NOT added: patterns are not slot-addressable, and
including them would pin PSubscribe-only PubSubs to a single
pattern-hash node and lose the existing random-node behaviour.
Adds TestPubSubConn_PassesPersistedChannelsToNewConn covering five cases
(subscribe-only reconnect, ssubscribe-only reconnect, initial
ssubscribe, mixed, and ordering of newChannels) via a stub newConn that
captures the channel list. The cluster routing layer itself is not
under test — the regression is whether the channel list reaches the
closure intact.
Fixes#3806
In TLS-only clusters started with `--port 0 --tls-port <port>`, CLUSTER
SLOTS reports the regular port as 0. newClusterState used to pass that
value verbatim into clusterNode.Client.opt.Addr, which the PubSub path
dials directly with no MOVED/seed fallback, producing
`dial tcp <ip>:0: connection refused`.
Add a replaceZeroPort helper symmetric to replaceLoopbackHost: when a
slot node's port is "0", substitute the origin endpoint's port — by
definition reachable, since it is the port we used to fetch the slot
map. NodeAddress is left untouched so maintenance-notification matching
continues to see the server-reported value.
Adds TestNewClusterState_PortZero covering the bug scenario plus the
non-zero passthrough and the loopback+:0 composition.
Fixes#3726
In MasterAddr, multiple goroutines probe sentinels concurrently. The first
to succeed enters sync.Once and saves its SentinelClient via setSentinel.
Other goroutines that also succeeded but lost the once.Do race doesn't
closed its SentinelClient — a resource leak.
After once.Do returns, close the sentinelCli if it's not the winner.
sync.Once guarantees all goroutines see c.sentinel written by the winner.
* fix(vset): vlinks return type
* fix(vset): fix test
* fix(vset): fix test
* fix(vset): remove last layer check from tests
* fix(vset): fix lint & issue
* fix(vset): fix n -> innerLen check
* test(vset): add mocked-response parser tests for VLINKS
Cover StringSliceSliceCmd.readReply (VLINKS) and
VectorScoreSliceSliceCmd.readReply (VLINKS WITHSCORES) with both
RESP2 and RESP3 wire fixtures, including empty layers, nil
elements, odd-length-array errors, and malformed input.
Also switch NewStringSliceSliceCmd to args ...any to match the
sibling NewVectorScoreSliceSliceCmd signature.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---------
Co-authored-by: Nedyalko Dyakov <[email protected]>
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Co-authored-by: Nedyalko Dyakov <[email protected]>
Root cause: a race condition between two independent goroutines
that detect failover completion:
1. The test's polling loop queries sentinel.Master() directly,
which returns as soon as the sentinel server reports the new
master.
2. The ConnectionClosed metric callback is triggered asynchronously
via the pubsub "+switch-master" message. The pubsub listener
runs in a separate goroutine closes stale connections and emits
the CloseReasonFailover metric.
When the test's polling loop breaks before the pubsub listener has
processed the "+switch-master" message, the metric callback does
fire eventually, but after the assertion has already failed.
Fix: replace the direct Expect assertion with Eventually.
Go 1.20+ auto-seeds the global PRNG and Go 1.24 uses lock-free
ChaCha8 internally, making the custom mutex-protected wrapper
unnecessary. This simplifies the codebase and improves concurrent
performance.
* feat: impl array data structure
* add integration tests for array commands
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* fix: move CmdTypeUint and CmdTypeUintSlice to end of iota sequence
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* test: add boundary tests for large uint64 index values
Covers ARSet, ARGet, ARGetRange, ARMSet, ARMGet, ARDel,
ARDelRange, ARScan, ARSeek, ARNext, and AROp.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* refactor: replace generic AROp with typed methods and improve test coverage
- Split AROp into specific methods: AROpSum, AROpMin, AROpMax, AROpAnd,
AROpOr, AROpXor, AROpMatch, AROpUsed with proper return types
- Pre-allocate slices in ARScan, ARGrep, ARGrepWithValues, ARLastItems
- Add tests: ARInfoFull, negative AND/OR/XOR results, ARGrep nil args
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* mark ArrayCmdable API as experimental
Co-Authored-By: Claude Opus 4.7 <[email protected]>
* Use `any` instead of `interface{}` and document index range constraint on ArrayCmdable interface.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
---------
Co-authored-by: Claude Opus 4.7 <[email protected]>
* Adds support for multiple aggregators in TS range commands
* Validates against invalid aggregators
---------
Co-authored-by: Nedyalko Dyakov <[email protected]>
* feat(command): add increx
* add one additional test
* update increx
* use custom 8.8 image with new increx
* address pr comments
* always use by in float
* reading xautoclaim's 3rd return value as list of deletedIDs
* adding test for xautoclaim that returns deletedIDs
* updating expected comments. running doctests make test locally works fine ?
* Adding missing clone deletedIDs
* switch to creating a new command instead
* output needed updating
* forgot to add the XAutoClaimWithDelete command to StreamCmdable
* Fix naming inconsistency in XAutoClaimWithDeleted
* trigger CI
* trigger CI
* trigger CI
* trigger CI
---------
Co-authored-by: Benoit Verreault <[email protected]>
Co-authored-by: Nedyalko Dyakov <[email protected]>
zeekayandHanzo Dev
