Repository navigation
feat: Add read_from and client_az options to the Redis GLIDE client - #7093
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7093 +/- ##
==========================================
+ Coverage 51.79% 51.81% +0.01%
==========================================
Files 458 458
Lines 57715 57735 +20
Branches 8483 8486 +3
==========================================
+ Hits 29896 29915 +19
Misses 25793 25793
- Partials 2026 2027 +1
*This pull request uses carry forward flags. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
@NopeCharon the PR is out-of-date, pls update it. |
|
Merged the latest master into the branch, no conflicts. |
ce7ce13 to
4b075f3
Compare
The GLIDE client added in feast-dev#6870 always read from the primary, because the Feast config had no way to pass GLIDE's read strategy through. Clusters with replicas could not spread online reads across them. Add two RedisOnlineStoreConfig options that map onto GLIDE's own settings: * read_from: primary (default), prefer_replica, az_affinity, az_affinity_replicas_and_primary or all_nodes, translated to glide_sync.ReadFrom. * client_az: the client's availability zone, used by the az_affinity strategies. The config is validated at load time so a setting can never be silently ignored: * read_from and client_az require client: glide, since redis-py does not use them. * A read_from other than primary requires redis_type: redis_cluster. A standalone GLIDE client sends every batch to the primary, so the batched HMGET read would ignore the strategy. * The az_affinity strategies require client_az, which GLIDE also enforces when building the client config. Verified against a Valkey 8 cluster with one primary and one replica: prefer_replica and az_affinity send the batched online_read to the replica, all_nodes spreads it over both nodes, and the default keeps reading from the primary. Fixes feast-dev#7086 Signed-off-by: NopeCharon <[email protected]>
4b075f3 to
38df0e8
Compare
What this PR does / why we need it:
The GLIDE client from #6870 always reads from the primary: the Feast config has no way to pass GLIDE's read strategy through, so clusters with replicas can't spread online reads across them.
This adds two
RedisOnlineStoreConfigoptions that map directly onto GLIDE's own settings:read_from:primary(default),prefer_replica,az_affinity,az_affinity_replicas_and_primaryorall_nodes, translated toglide_sync.ReadFrom.client_az: the client's availability zone, used by theaz_affinitystrategies.I used dedicated options rather than
connection_stringparameters, as the issue suggests. The reason is thatconnection_stringparameters are also passed as kwargs to redis-py, which still handles writes and async reads, and redis-py would reject them.The config is validated at load time, so a setting is never silently ignored:
read_fromandclient_azrequireclient: glide, since redis-py doesn't use them.read_fromother thanprimaryrequiresredis_type: redis_cluster. A standalone GLIDE client sends every batch to the primary whateverread_fromis, so the batchedHMGETread would ignore the strategy. GLIDE's standaloneBatchOptionshas no routing option. I checked this against a real primary + replica: withPREFER_REPLICA, singlehmgetcalls went to the replica butexec(Batch(...))still went to the primary.az_affinitystrategies requireclient_az. GLIDE also enforces this, but only when the client is created.Docs:
docs/reference/online-stores/redis.mdgets a "Reading from replicas" section and two new rows in the configuration reference table.Which issue(s) this PR fixes:
Fixes #7086
Checks
git commit -s)Testing Strategy
Unit tests (
tests/unit/infra/online_store/test_redis.py) cover the default, the three validation rules, and the mapping of everyread_fromvalue ontoglide_sync.ReadFromandclient_az.Manual test: I ran
RedisOnlineStore.online_write_batch+online_readwithclient: glideagainst a Valkey 8 cluster with one primary (availability-zone zone-a) and one replica (zone-b). For each config I countedHMGETcalls per node fromINFO commandstatsover one read of 3 entities:primary)prefer_replicaall_nodesaz_affinity,client_az: zone-baz_affinity,client_az: zone-a(no replica in zone, falls back to other replicas)az_affinity_replicas_and_primary,client_az: zone-aAll reads returned the written values.
Misc