refactor: improve SCAN iterator/expiration performance

[BorysTheDev]

Summary: This PR streamlines SCAN’s key iteration and lazy expiration path to reduce overhead during traversal.

Changes:

• Adds CompactKey::IsExpired for a direct TTL-tagged expiration check.
• Extracts the deletion portion of lazy expiry into DbSlice::Expire and exposes TryExpire for raw iterators.
• Updates SCAN to avoid constructing an auto-laundering iterator when traversal is protected from preemption.
• Defers key-string materialization until after type, attribute, size, bucket, and pattern filters pass.

Technical Notes: The existing journal flush guard maintains raw-iterator safety while expired entries are removed during SCAN.

COUNT	TTL	MATCH	Before ms/DB	After ms/DB
10	None	*	359.90	320.85
10	Live	*	430.18	336.99
100	None	*	291.60	252.80
100	Live	*	349.08	266.83
1000	None	*	279.65	241.91
1000	Live	*	330.25	255.68
1000	Live	k*	333.36	262.81

I've asked AI to generate a call stack picture to show the difference in approaches for better understanding

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Optimize SCAN iteration and expiration handling

✨ Enhancement 🕐 20-40 Minutes

---

AI Description

• Adds direct embedded-TTL checks and separates expiration eligibility from deletion.
• Uses guarded raw iterators throughout SCAN filtering to reduce wrapper overhead.
• Defers key materialization until filters pass and avoids unnecessary copies.

Diagram

graph TD
  A["SCAN traversal"] --> B["Raw iterator"] --> C{"Entry erased?"}
  C -- "Yes" --> D["Skip result"]
  C -- "No" --> E["Apply filters"] --> F["Materialize key"] --> G["SCAN results"]

Loading

High-Level Assessment

The approach is appropriate because SCAN already prevents journal-flush preemption, making raw prime iterators safe during traversal. Retaining the higher-level iterator wrapper would provide redundant safety while preserving its conversion overhead, and eager key materialization would continue wasting allocations for filtered entries.

Files changed (4) +51 / -24

Other (4) +51 / -24

compact_object.hAdd an inline embedded-TTL expiration check +4/-0

Add an inline embedded-TTL expiration check

• Adds 'CompactKey::IsExpired' to test the TTL tag and compare its embedded expiration timestamp directly against the current time. This avoids separate expiration-presence and timestamp-access operations on hot paths.

src/core/compact_object.h

db_slice.ccSeparate expiration checks from entry deletion +12/-5

Separate expiration checks from entry deletion

• Refactors 'ExpireIfNeeded' to use 'CompactKey::IsExpired' and delegates actual deletion to a new boolean-returning 'Expire' method. The extracted method preserves replication, notification, deletion hooks, and expiration statistics while reporting whether the entry was erased.

src/server/db_slice.cc

db_slice.hExpose guarded raw-iterator expiration +9/-0

Expose guarded raw-iterator expiration

• Adds 'TryExpire' for callers that can safely retain a raw prime-table iterator by preventing preemption. Declares the internal 'Expire' operation used by both existing expiration handling and the optimized SCAN path.

src/server/db_slice.h

generic_family.ccStreamline SCAN filtering and key materialization +26/-19

Streamline SCAN filtering and key materialization

• Processes SCAN entries directly through 'PrimeIterator' under the existing flush guard and uses 'TryExpire' to discard erased entries. Moves compact-key string conversion after structural filters, writing directly into the result when no matcher exists and moving matched temporary strings otherwise.

src/server/generic_family.cc

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

---

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

---

Tip of the day

💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

---

Powered by Qodo

[augmentcode]

🤖 Augment PR Summary

Summary: This PR streamlines SCAN’s key iteration and lazy expiration path to reduce overhead during traversal.

Changes:

• Adds CompactKey::IsExpired for a direct TTL-tagged expiration check.
• Extracts the deletion portion of lazy expiry into DbSlice::Expire and exposes TryExpire for raw iterators.
• Updates SCAN to avoid constructing an auto-laundering iterator when traversal is protected from preemption.
• Defers key-string materialization until after type, attribute, size, bucket, and pattern filters pass.

Technical Notes: The existing journal flush guard maintains raw-iterator safety while expired entries are removed during SCAN.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

[Copilot]

🟢 Approval recommended

The refactor is localized, preserves prior expiration gating (including replica behavior), and the SCAN traversal preemption constraints remain enforced by the existing guards.

Pull request overview

This PR refactors the SCAN traversal/expiration path to avoid extra iterator wrapping and redundant expiration checks, improving hot-path performance while preserving existing expiration behavior.

Changes:

• Add CompactKey::IsExpired(now_ms) for a fast inline expiration check on TTL-tagged keys.
• Split DbSlice::ExpireIfNeeded logic by extracting DbSlice::Expire(...) and add DbSlice::TryExpire(...) for safe “erase-if-expired” use with raw PrimeIterators.
• Refactor SCAN callback to use TryExpire and a small helper (AppendScanKey) to avoid extra work when matching/serializing keys.

File summaries

File	Description
src/server/generic_family.cc	SCAN callback now uses raw iterators + TryExpire and centralizes key append/match logic.
src/server/db_slice.h	Adds TryExpire and factors out Expire(...) declaration used by expiration paths.
src/server/db_slice.cc	Refactors ExpireIfNeeded to use IsExpired + extracted Expire(...) implementation.
src/core/compact_object.h	Adds CompactKey::IsExpired(now_ms) helper used by expiration logic.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[romange]

Cosmetic comments

[Copilot]

🟢 Approval recommended

No unresolved review issues were identified.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[romange]

LGTM

[Copilot]

🟢 Approval recommended

Only a non-blocking test-coverage nit remains.

Review details

Suppressed comments (1)

src/server/generic_family.cc:670

• This changes SCAN to delete expired entries through a raw PrimeIterator, but the existing SCAN tests cover live-key filtering/cursor mutation and do not exercise this expiration path. Please add a regression test that scans after TTL expiry (including a filtered scan and traversal past deleted entries) and verifies the expired key is removed without skipped/duplicate results; this is the behavior that protects the raw-iterator refactor.

  if (db_slice.TryExpire(op_args.db_cntx, prime_it)) [[unlikely]]
    return false;

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite