Redis resolves the keys it must invalidate in getKeysFromCommand(): the
module getkeys API, then getkeys_proc, then the legacy range. Key specs are
never consulted, so a module command that declares movable keys without
registering a getkeys callback is tracked by its legacy positions alone,
usually none. cscDeriveMeta indexed the spec positions instead, and the
entry then sits stale for its whole life because no invalidation ever
matches it.
Deriving no extraction means the command is never cached, so the guarantee
no longer depends on remembering each module command in a dont_cache
correction. The cacheable-command list is unchanged; the pinned
fingerprint moves because ts.nrange and ts.nrevrange now derive no
extraction.
Refs #4097
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
A null attribute (|-1) is not valid RESP3, but ReadAttributeLen consumes
the header and reports proto.Nil. peekCommandInfoReplyType passed that
error on, and the COMMAND readers treat a Nil error as "the value was
already drained", so the wrapped reply stayed on the wire and the
connection went back to the pool out of step with the stream.
Unwrap it as an empty attribute instead.
Refs #4097
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
HGetEXWithArgs and HSetEXWithArgs panicked with a nil pointer
dereference when the options were nil. Treat nil like empty options,
as SDiffCard and SUnionCard do, so the commands match HGetEX and
HSetEX.
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
* test(cluster): skip unknown-endpoint spec before 8.4
On Redis 8.0 and 8.2 the cluster nodes segfault in a module timer
shortly after cluster-preferred-endpoint-type is set to
unknown-endpoint. The spec restores the setting quickly, so it only
fails when the timer fires inside that window, but then every later
cluster spec hits a dead cluster.
* test(cluster): guard unknown-endpoint teardown on skip
* fix(auth): make empty-password AUTH opt-in
#4161 made a Username with an empty Password send AUTH user "". Redis
OSS accepts it (the nopass default user takes any password), but Redis
Enterprise and Redis Cloud databases with unauthenticated access can
reject it with WRONGPASS, so configs with a username and no password
stopped connecting.
Restore the earlier default (no AUTH without a password) and add
Options.AuthEmptyPassword (auth_empty_password=true) to opt in, on all
options types and credential providers at connect time. Public Hello
again sends AUTH only with a password. The client-side cache namespace
uses the ACL user the connection runs as.
Refs #4161
* fix(auth): keep CSC namespace per username
A Username that sends no AUTH runs as the default user, or as a
TLS-certificate user. Mapping it to the default namespace let clients
with different certificates share cache entries. It now gets its own
namespace per Username ("noauth <name>", which no ACL user can match).
Test: delete the ACL user before closing the admin client, so the user
does not leak into the ACL specs.
* fix(auth): split sentinel empty-password opt-in
Sentinel and data-node credentials are independent, so one flag for
both could force AUTH user "" onto Redis Enterprise data nodes to
authenticate a nopass sentinel user. FailoverOptions and
UniversalOptions gain SentinelAuthEmptyPassword
(sentinel_auth_empty_password=true); AuthEmptyPassword now covers the
master/replica connections only.
HOTKEYS GET reports the session CPU time as total-cpu-time-user-ms and
total-cpu-time-sys-ms, which is how src/hotkeys.c and the command docs name
them. readReply looked up used-cpu-user-ms and used-cpu-sys-ms instead, so
UsedCPUUser and UsedCPUSys stayed zero for every reply.
* fix(cluster): accept a null host in CLUSTER SLOTS
With cluster-preferred-endpoint-type unknown-endpoint the server sends a
null host. ClusterSlotsCmd returned redis.Nil and left the rest of the
reply unread on the connection. Read the null as an empty host and let
newClusterState reuse the origin host, as redis.conf describes.
* test(cluster): expect the origin host for an empty slot host
An empty host in CLUSTER SLOTS, which a node sends when it does not know
its own IP, now also takes the origin host, as redis-py does. Update the
loopback spec that asserted the old ":1234" address.
* chore(cluster): say why a null CLUSTER SLOTS host is accepted
Explain the Nil guard so it is not removed later as a typo.
* test(cluster): cover unknown-endpoint end to end
Set cluster-preferred-endpoint-type unknown-endpoint on the 6-node test
cluster, then check that a ClusterClient loads the topology, runs
SET/GET on two shards and follows a MOVED redirect.
* test(cluster): say the MOVED check relies on a loopback cluster
With unknown-endpoint, Redis sends MOVED and ASK with an empty host, and
the client dials ":port" on the local machine. The spec follows the
MOVED only because the test cluster is on loopback. Say so in the spec
comment and the It text, and leave taking the origin host for the
redirect target to a follow-up.
* fix(script): keep client-computed SHA-1 in Script.Load
* fix(script): export ErrScriptDigestMismatch
Wrap the Script.Load mismatch error so errors.Is works, skip the
comparison while the SCRIPT LOAD reply is still queued in a pipeline
(Val is empty until Exec), and fold the tests into
script_server_sha_unit_test.go.
* fix(script): compare Load digest case-insensitively
JSONGetWithArgs appended the paths inside the options != nil block, so
with nil options it sent JSON.GET key without the paths and returned
the whole document instead of the requested paths.
* fix(sentinel): set up streaming creds on failover
* docs: add release note for failover streaming creds
* fix(pool): run OnRemove hooks in Filter
* test(sentinel): check failover pipeline pool hook
ModuleLoadexConfig.toArgs put ARGS before every argument, but Redis
passes everything after the first ARGS to the module, so a module
loaded with Args [a b] got the arguments [a ARGS b]. Send ARGS once,
followed by all the arguments, and fix the test that expected the
repeated ARGS.
FT.CREATE stops reading type-specific field options at SORTABLE, UNF
and NOINDEX. A field with Sortable and Weight, Separator,
CaseSensitive, PhoneticMatcher, WithSuffixtrie, IndexEmpty or
IndexMissing was rejected by the server. Emit SORTABLE, UNF and NOINDEX
after the other options.
Redis does not queue WATCH or MULTI sent inside MULTI and does not
abort the transaction either, so EXEC returns one result fewer than
the client wrote. The tx readers only checked that the EXEC reply was
an array, then assigned results to commands by position and blocked
on the missing one until ReadTimeout.
Fail the transaction when the count differs from the queued commands
so the connection is dropped and no result lands on the wrong command.
FailoverOptions.clusterOptions() did not copy ConnMaxLifetimeJitter,
so NewFailoverClusterClient ignored it. UniversalOptions.Failover()
also dropped FailingTimeoutSeconds, so NewUniversalClient with
MasterName plus RouteByLatency, RouteRandomly or IsClusterMode
ignored both options.
* fix(autopipeline): reroute cluster FD recovery
A cluster full-duplex node child replayed a dropped session's tail only
on a fresh connection to the same node, and failed its carry and queue
once lease retries ran out. If topology change had closed or replaced
that master, every accepted command on it failed instead of reaching
the slot's new owner (codex P2 on #4002, listed as a known limitation).
Add an internal owner seam, AutoPipelineOptions.fdReroute: an engine
with an owner runs that work through the owner, in order, instead.
- connection error: the replayable tail (same budget partition and
sent-NoRetry split as the replay) goes through the owner;
- lease retries spent: the carry, then the queue, go through the owner
instead of failing.
The cluster FD router sets it to the ClusterClient pipeline, which
routes by slot against live topology. Standalone FD is unchanged.
* fix(autopipeline): keep retry budget on FD reroute
fdReroute now takes the remaining retry bound. rerouteReqs goes through
flushCarryBudgeted, so each group passes its remaining budget and a
command never runs more than budget+1 times; the cluster child uses
cc.processPipelineRetries with that bound in place of MaxRedirects.
- The Close-time flush (flushReqs) also goes through the owner, so a
child closed by topology GC does not flush through its dropped node.
- runReroute returns a per-command transport error even when the owner
returns an earlier Redis error (redis.Nil), so the flush still aborts.
- flushReqsVia reports per-command metrics before completing, as the
reader does. This also adds them to the standalone Close-time flush.
* test(autopipeline): fold reroute tests into FD tests
* docs(autopipeline): note nested hooks on FD recovery
* fix(cmd): don't modify the caller's geo queries
geoLocationArgs incremented withLen on the GeoRadiusQuery that the
caller passed, and GeoLocationCmd read it back to parse the reply. A
second GeoRadius call with the same query parsed the reply with a
doubled withLen and failed with "got 2 elements in the array, wanted
3", and sharing one query between goroutines was a data race. Count
the WITH flags when the reply is read, as GeoSearchLocationCmd does,
and drop the withLen field.
geoSearchArgs also wrote the default "km" unit back to the
GeoSearchQuery. Keep the default in a local variable instead.
* fix(cmd): copy the geo query into the command
GeoLocationCmd and GeoSearchLocationCmd kept the caller's query and read
its WITH flags when the reply arrived. With a pipeline or autopipelining,
a change to the query between queuing and reading parsed the reply with
flags that weren't sent. Keep a copy of the query in the command, so the
reply is parsed with the flags of the command, and make Clone a plain
copy.
* fix(cluster): verify TLS against each node host
Signed-off-by: Pragalva Sapkota <sapkotapragalva@gmail.com>
* test(cluster): update TLS URL expectations
Signed-off-by: Pragalva Sapkota <sapkotapragalva@gmail.com>
* test(cluster): fix skip-verify TLS expectation
* fix(cluster): make per-node TLS verification opt-in
Keep ServerName pinned to the first URL host by default, so clusters
that share one certificate and announce nodes by IP keep working.
Add tls_verify_node_host=true to clear ServerName and verify each
node against the host it is dialed at.
Refs #4129
Signed-off-by: Pragalva Sapkota <sapkotapragalva@gmail.com>
---------
Signed-off-by: Pragalva Sapkota <sapkotapragalva@gmail.com>
"race limit the number of connections" asserted IdleConns == 0 and
TotalConns == PoolSize after 1000 concurrent Gets with MinIdleConns 50.
A Get that misses while min-idle dials are in flight dials an excess
non-pooled conn, and the late min-idle conns stay idle, so total and
idle both overshoot by the same amount. PoolSize is a soft cap on
pooled conns (see .claude/specs/pool.md), so this is allowed.
Assert the contract instead: no Get errors, in-use conns == PoolSize,
pooled conns <= PoolSize. Also close the spec's pool and nil-guard the
shared AfterEach so the spec can run focused.
* fix: carry streaming credentials manager to Conn and Tx
Conn(), EphemeralConn(), and Watch build derived clients that call
initConn on cold connections. initConn uses streamingCredentialsManager
when StreamingCredentialsProvider is set; without the parent's manager
those paths nil-pointer on Listener (issue #4154).
Copy the manager the same way clone() already does for WithTimeout.
Disclosure: drafted with Grok assistance; I reviewed and tested.
* test: check EphemeralConn manager identity and drive cold initConn
Apply review suggestions on #4155: assert EphemeralConn shares the
parent's streamingCredentialsManager, and drive the cold initConn path of
Conn, EphemeralConn and Tx against an unresponsive net.Pipe so the test
panics without the fix and returns a handshake error with it.
Disclosure: drafted with AI assistance (Grok); I reviewed and ran it.
HPTTL right after HEXPIRE asserted 10000ms +/- 2ms, which fails on
slow CI runners (seen 9991). TTL only counts down, so assert
9000 < ttl <= 10000 instead.
* fix(search): read per-doc RESP2 values in FT.SEARCH
On RESP2, FT.SEARCH puts the WITHSCORES, WITHPAYLOADS and WITHSORTKEYS
values right after each document ID, even with NOCONTENT, and sends a
nil payload for a document without one. parseFTSearch stopped at the
ID when NoContent was set and skipped a nil payload without consuming
it, so a score, sort key or nil was read as the next document ID: Docs
held twice the entries, or the call failed with "invalid document ID
format". Read every requested value before the fields.
* fix(search): step past nil RESP2 sort keys
FT.SEARCH WITHSORTKEYS sends a nil sort key for a document that lacks
the SORTBY field, and for every document when there is no SORTBY.
parseFTSearch only stepped past string sort keys, so the nil was read
as the fields or the next document ID and the call failed with
"invalid document ID format". Step past the slot like the payload.
* test(search): cover nil RESP2 sort key before a doc
* test(search): compare RESP2 and RESP3 payloads
* fix(conn): validate URL retry and buffer values
Reject negative URL settings so a mistyped positive value cannot silently
become a default retry delay or an unintended buffer size. An explicit
zero retry timeout selects no delay; omitted values keep their defaults.
Closes#4132
Refs #4063
* docs(conn): clarify URL retry timeout rules
* docs(conn): note URL validation compatibility
* fix(cmd): allow = in CLIENT INFO values
Redis accepts "=" in a client name, a user name and lib-name, and
go-redis puts Options.IdentitySuffix into lib-name. parseClientInfo
split each field on every "=" and rejected a field with more than one,
so ClientInfo and ACLLog failed with "unexpected client info data".
Split each field at the first "=" only.
* Update client_info_internal_test.go
---------
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
* fix(cmd): accept RESP3 doubles in Cmd.Float64
With RESP3, the default protocol, ReadReply returns a double reply
(for example from ZSCORE) as float64. toFloat64 and toFloat32 only
handled int64 and string, so Cmd.Float64, Float32, Float64Slice and
Float32Slice failed with "unexpected type=float64". Handle float64 the
same way toBool handles the RESP3 boolean.
* fix(cmd): keep Float32 range error on RESP3
float32(val) turns a RESP3 double outside the float32 range into an
infinity without an error, while the RESP2 bulk string path returns the
ParseFloat range error. Return the same error for the RESP3 double. An
infinity sent by the server is still returned as is.
---------
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
* fix(pubsub): avoid panics on Channel conflict and Ring empty subscribe
* fix(pubsub): avoid data race on Channel conflict warn
Conflict-path logging used getContext(), which reads c.cmd while
the other Channel Receive loop may write it — fails CI under -race.
Closes nothing; Refs #3761
* fix(pubsub): prioritize closed state over stickyErr on conn retrieval
* fix(pubsub): override stickyErr on Close for channel exit
Match SingleConnPool.Close by setting stickyErr to ErrClosed so
failed Ring PubSub Channel receivers always observe the closed
exit signal. Add .gitattributes to keep Go sources on LF.
* fix(pubsub): address review feedback on sticky error channel handling and docs
- Remove dead stickyErr write in Close()
- Short-circuit Channel() and ChannelWithSubscriptions() when stickyErr is set to return an already-closed channel and log a warning
- Add warning log in failedPubSub for visibility
- Update doc comments for Subscribe/PSubscribe/SSubscribe and Channel methods
- Remove repo-wide .gitattributes
Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
---------
Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
* fix(command): read past error elements in IntSliceCmd
TS.MADD and CMS.INCRBY reply with an error element for a rejected item
and keep answering for the items after it. Returning on that element
left the tail of the array unread, and in a pipeline the next command
read it as its own reply.
* docs(command): note per-item errors on TSMAdd/CMSIncrBy
Also move the error-element tests next to the nil-element cases.
---------
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
* docs(readme): fix the supported-versions list
The Supported versions section lists Redis 8.0, 8.2, 8.4, 8.8 and 8.10, but
that is not what CI builds against. `.github/workflows/build.yml` runs both
`test-stack` (:48-54) and `test-redis-ce` (:109-115) over
8.12.x, 8.10.x, 8.6.x, 8.4.x, 8.2.x and 8.0.x.
So two versions that are tested are missing from the list (8.6, and 8.12 as a
pre-release) while 8.8 is listed although it is no longer exercised — its
removal from the matrix is recorded in RELEASE-NOTES.md:440 ("8.8 was dropped
from the CI matrix", #3940).
Rewrite the list to match the matrix. The "last three releases" sentence is
left as-is: it states the policy, not a fact CI can confirm.
* docs(readme): avoid unknown word in the 8.12 support note
* docs(readme): preserve supported Redis versions
---------
Co-authored-by: ump45nose <ump45nose@users.noreply.github.com>
* feat(options): parse more URL connection options
ParseURL, ParseClusterURL and ParseFailoverURL accepted different
subsets of the connection settings, so an option one parser takes was
an unexpected option in another. Accept dialer_retries,
dialer_retry_timeout, context_timeout_enabled, read_buffer_size,
write_buffer_size, disable_identity and identity_suffix wherever the
options struct has the field. ParseFailoverURL also accepts the
snake_case identity names; the camelCase ones keep working.
* Update options_test.go
---------
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
