Skip to content

kv: fix ReverseScanAt cross-shard ordering when clampToRoutes=false#411

Merged
bootjp merged 3 commits intofeature/s3-implfrom
copilot/sub-pr-396
Mar 23, 2026
Merged

kv: fix ReverseScanAt cross-shard ordering when clampToRoutes=false#411
bootjp merged 3 commits intofeature/s3-implfrom
copilot/sub-pr-396

Conversation

Copy link
Contributor

Copilot AI commented Mar 23, 2026

reverseScanRoutesAt with clampToRoutes=false (used for S3 manifest scans spanning multiple shards) concatenated per-route results with a shrinking limit, producing out-of-order output and silently dropping keys when shard ranges interleave in descending keyspace.

Changes

  • reverseScanRoutesAt: when clampToRoutes=false, fetch limit results from every route then merge+sort descending — mirroring the approach scanRoutesAt already uses for the ascending case. When clampToRoutes=true, preserve the original short-circuit on len(out) >= limit.
  • mergeAndTrimReverseScanResults: new helper symmetric to mergeAndTrimScanResults but sorting descending (bytes.Compare > 0).
// before: concatenates per-route results with shrinking budget → wrong order
for i := len(routes) - 1; i >= 0 && len(out) < limit; i-- {
    kvs, _ := s.scanRouteAtDirection(ctx, route, ..., limit-len(out), ts, true)
    out = append(out, kvs...)
}

// after: fetch limit from every route, merge+sort descending
kvs, _ := s.scanRouteAtDirection(ctx, route, ..., limit, ts, true)
out = mergeAndTrimReverseScanResults(out, kvs, limit)

Tests

New tests added in kv/shard_store_test.go:

  • TestShardStoreReverseScanAt_DescendingOrderAcrossShards: verifies that a nil-start scan (clampToRoutes=false) across two shards returns all keys merged in descending order.
  • TestShardStoreReverseScanAt_LimitAcrossShards: verifies that the top-N keys are correctly selected across shard boundaries (e.g. limit=4 from 3+3 keys picks the 4 largest).
  • TestShardStoreReverseScanAt_SingleShard: single-shard baseline for descending order.
  • TestShardStoreReverseScanAt_IncludesS3ManifestKeysDescending: mirrors the existing forward-scan manifest test — reverse scan over manifest keys returns descending order.
  • TestMergeAndTrimReverseScanResults / _EmptyInput / _WithinLimit / _ExactLimit: unit tests for the new helper covering merge, descending sort, and limit trimming edge cases.

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Add S3 adapter to elastickv kv: fix ReverseScanAt cross-shard ordering when clampToRoutes=false Mar 23, 2026
Copilot AI requested a review from bootjp March 23, 2026 06:52
@bootjp
Copy link
Owner

bootjp commented Mar 23, 2026

@copilot add test code

@bootjp bootjp marked this pull request as ready for review March 23, 2026 07:07
…ReverseScanResults

Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bootjp/elastickv/sessions/845127a7-c957-4c22-8699-b8584a84033d
Copy link
Contributor Author

Copilot AI commented Mar 23, 2026

@copilot add test code

Added in commit d6fd20f. Eight tests covering:

  • TestShardStoreReverseScanAt_DescendingOrderAcrossShards — nil start (clampToRoutes=false) merges both shards in descending order
  • TestShardStoreReverseScanAt_LimitAcrossShards — top-N selection across shards (e.g. limit=4 from 3+3 keys picks top 4 descending)
  • TestShardStoreReverseScanAt_SingleShard — single-shard baseline
  • TestShardStoreReverseScanAt_IncludesS3ManifestKeysDescending — manifest key reverse scan returns descending order
  • TestMergeAndTrimReverseScanResults / _EmptyInput / _WithinLimit / _ExactLimit — unit tests for the new helper

@bootjp bootjp merged commit bd857e7 into feature/s3-impl Mar 23, 2026
4 checks passed
@bootjp bootjp deleted the copilot/sub-pr-396 branch March 23, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants