backend/perf: pipelined cluster Redis reads and batched zAddIfPossible [prodHotPush-BPP] - #1518
Merged
piyushKumar-1 merged 1 commit intoSep 14, 2026
Conversation
- mGetClusterRaw sends its per-slot MGETs in a single hedis pipeline (one round trip per cluster node) instead of sequential MGETs or waves of 32 forked MGETs. Replies stay index-aligned; a failed slot or a failed pipeline yields Nothing for the affected keys, as before. - New runPipelined / runPipelinedByKey: run many single-key commands (ZCOUNT, EVAL, ZREM, ...) in one pipeline with results aligned to the input; per-key reply errors or a failed pipeline become Nothing. Batches run through runHedis, so a connection-level failure surfaces as an error instead of an empty result. - New zAddIfPossibleMany runs the zAddIfPossible check-and-add script for many keys in one pipeline and returns per-key results aligned with the input (1 added, 0 at capacity, -1 on a reply error or failed pipeline). The script and its arguments are shared with zAddIfPossible, whose behaviour is unchanged. - Drops the CLUSTER_MGET_ASYNC_ENABLED env flag and the Forkable / L.MonadFlow constraints on mGetClusterRaw, mGetCluster and mGetClusterWithKeys. - Bumps euler-hs to main (a97ac313, nammayatri/euler-hs#76), which relocks hedis to 773cb39c with nammayatri/hedis#6: a pipeline's per-node batches run concurrently when REDIS_CLUSTER_PARALLEL_PIPELINE is true (off by default, read once per process). It also brings the node-pool reset after repeated acquire timeouts that was already on that hedis branch. - Adds a Redis Cluster test group to mobility-core-tests, run only when MOBILITY_CORE_TEST_REDIS_CLUSTER_PORT is set: key order across 600 keys, missing, duplicate and slot-sharing keys, a non-string key, an empty list, 2500 keys past the pipeline flush, mGetClusterWithKeys, runPipelinedByKey and zAddIfPossibleMany. (cherry picked from commit ea37819)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
15 tasks
piyushKumar-1
added a commit
to nammayatri/nammayatri
that referenced
this pull request
Sep 14, 2026
Cherry-picks onto prodHotPush-BPP. 1. backend/feat/continuos-batching-stuff (44818e0) - DriverPoolConfig gets batchingMode (OFF, CONTINUOUS, STAGGERED) and nextBatchScheduleTime; SearchTry and SearchRequestForDriver get batchingMode. - The allocator keeps a per-search-try reserve list of ranked drivers and tops batches up from it; the driver accept-offer flow moves to Domain.Action.UI.DriverAcceptOffer. - DDL migration 0865-continuous-batching.sql. The geohash index migration keeps this branch's 0862 name instead of being renamed to 0866. 2. #16738 (ad8e0a2): pipelined driver pooling reservation and enrichment - processCandidatesChunk: the parallel-request pre-filter runs one pipelined ZCOUNT per chunk instead of a sequential Redis call per candidate. - makeTaggedDriverPool: keeps the two-pass soft/hard admission on zAddIfPossible but reserves in rank order only up to batchSize, through the pipelined zAddIfPossibleMany; drivers outside the batch are never added to a parallel set. The rest of the ranked pool goes to the reserve list in continuous mode, and top-ups reserve when they pop a driver. - Enrichment reads (SRD counters, idle time) go through shared-kernel's pipelined mGetClusterWithKeys. - Behaviour-tracker CounterConfig gets hashTagEntityId, passed through to mkCounterKey. It is True only for the RIDE_CANCELLATION and QUOTE_RESPONSE series that pooling enrichment reads, so a driver's pooling counters and idle keys share one Redis Cluster slot. Those counters reset on deploy and their old keys expire by TTL. - New TransporterConfig.enableDriverPoolEnrichment (Maybe Bool, unset = on) to skip the enrichment reads. - flake.lock pins shared-kernel to d217e6e1, the head of shared-kernel prodHotPush-BPP with nammayatri/shared-kernel#1518 (the cherry-pick of #1516) merged. It also moves euler-hs to a97ac313 and hedis to 773cb39c, which adds REDIS_CLUSTER_PARALLEL_PIPELINE. flake.nix is unchanged. (cherry picked from commit 44818e0) (cherry picked from commit ad8e0a2)
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.
What
Cherry-pick of #1516 (
ea378198) ontoprodHotPush-BPP(c5af9e39), for the driver pooling hot push (nammayatri/nammayatri#16738).mGetClusterRawsends its per-slot MGETs in one hedis pipeline, costing about one round trip per cluster node.mGetClusterandmGetClusterWithKeysbenefit automatically.New
runPipelined/runPipelinedByKey/zAddIfPossibleMany: many single-key commands in one pipeline, with results aligned to the input. Batches run throughrunHedis, so a connection failure surfaces as an error.Removed: the
CLUSTER_MGET_ASYNC_ENABLEDasync-fork path, and theForkable/L.MonadFlowconstraints on the MGET functions.flake.lock: euler-hs6290a7e3→a97ac313, which moves hedis7c8a33b2→773cb39c. That brings:REDIS_CLUSTER_PARALLEL_PIPELINE=true, a pipeline's per-node batches run concurrently. The flag is off by default.7e233136: improved fork/await with timeout.Only the euler-hs and hedis lock entries change.
The code changes are the same patch as
ea378198(identicalgit patch-id); only the surrounding files differ, because this branch is behindmain. The follow-up test fix #1517 (d7123b11) is not needed here: this branch doesn't havewithForkCountersinCoreMetrics, and the test monad's instance matches this branch's 29 methods exactly.Rollout note
Production currently sets
CLUSTER_MGET_ASYNC_ENABLED, which forks per-slot MGETs in waves of 32. This PR replaces that path with a pipeline. WithREDIS_CLUSTER_PARALLEL_PIPELINEunset, a read that spans many masters is sent node by node, which can be slower on a cluster with many masters. SetREDIS_CLUSTER_PARALLEL_PIPELINE=truewhen deploying this.Testing
Same as #1516:
mobility-corebuilds with-Wall -Werror, and the 10 Redis Cluster tests plus the full suite pass onmain. hedis#6 was verified on a local 3-master cluster. This branch relies on CI for the build.