Skip orphan multi/exec keys in SaveIndexExtension instead of aborting

[mohanrajendran]

Problem

IndexSchema::SaveIndexExtension assumes every key in multi_mutations_keys_ also exists in tracked_mutated_records_. That assumption is not guaranteed at an RDB save boundary: the main thread updates the queue while writer threads consume the mutation map.

This patch addresses a production ElastiCache crash where a forked RDB writer aborted during replication full sync. The primary process stayed up, but repeated child aborts caused full-sync failures. The initial failure can occur at the first save boundary; a save/reload/save cycle is not required.

sequenceDiagram
    participant M as Main thread
    participant Q as Multi exec queue
    participant MAP as Mutation map
    participant W as Writer thread
    participant S as RDB save

    M->>MAP: Normal write K
    M->>W: Schedule K
    Note over W: Writer is paused
    M->>Q: MULTI EXEC write K
    M->>MAP: Merge K
    Note over Q: Queue length is below threshold
    W->>MAP: Consume K
    Note over Q,MAP: Queue has K and map has no K
    S->>Q: Walk queued K
    S->>MAP: Check K
    S-->>S: RDB writer aborts

Loading

Fix

Skip queued keys that no longer have a mutation-map entry, and write the retained-key count so the RDB stream remains self-consistent. A skipped key's index membership is re-derived from the serialized key list or backfill on load; the load path needs no change.

Tests

• IndexSchemaFriendTest.OrphanMultiKeySkippedOnSave verifies that SaveIndexExtension succeeds when the queue contains a map-less key.
• TestMutationQueue.test_multi_exec_orphan_key_saved_on_first_save reproduces the production runtime sequence without a reload and verifies that the first SAVE succeeds.
• TestMutationQueue.test_multi_exec_orphan_key_skipped_still_searchable covers the save/reload/save path and verifies that the data remains searchable after another restart.
• Focused integration run: 2 passed, 503 deselected.
• Full Debug integration run: the C++ harness passed all 47 tests, and the orphan-save integration tests passed.

Note

Draining the multi/exec queue in the pre-fork callback could reduce the chance of forming this inconsistency at snapshot time. The save-time guard is still required for an RDB that already contains the inconsistency.

[github-actions]

Reviewers for this PR

• First Pass Reviewer: @chinguyen21 — Please do your best to do a detailed review on the PR and get a response on your feedback. Once the first pass is done, notify the maintainer assigned to this PR to follow up on the final review and getting the PR merged. You can reach out to the people owning the relevant code paths for more help on the review.
• Maintainer Reviewer: @yairgott — Once the first review is done, please follow up with a final review and help to merge the change in.

_{Assigned automatically to the least-assigned members of the reviewer pools in .github/reviewer-pools.json. Use /reviewer or /remove-reviewer to adjust.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 36cdec15-ddc8-418f-a188-f6eec638a661

📥 Commits

Reviewing files that changed from the base of the PR and between c8601e0 and 0327121.

📒 Files selected for processing (4)

• integration/test_saverestore.py
• src/index_schema.cc
• src/index_schema.h
• testing/index_schema_test.cc

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The RDB save path now skips orphaned multi/exec keys instead of aborting. It records skipped keys, serializes live entries, and adds unit and integration tests for save, restore, readability, and searchability.

Changes

Orphaned multi/exec save handling

Layer / File(s)	Summary
Save-path orphan filtering src/index_schema.cc	The RDB save path skips multi/exec keys without tracked mutation records. It logs the skipped keys, increments rdb_save_multi_exec_orphans_skipped, and serializes only live keys.
Orphan state unit validation src/index_schema.h, testing/index_schema_test.cc	The friend test declaration and unit test construct an orphaned queue state and verify that SaveIndexExtension succeeds.
Save and restore integration coverage integration/test_saverestore.py	Integration tests verify skipped-key counters, readable rewritten RDB files, and searchable records. Writer-thread settings move to startup arguments.

Suggested reviewers: allenss-amazon

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 03271

RDB saves now tolerate consumed multi/exec queue keys without aborting, while preserving readable snapshots and searchable records. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: skipping orphan multi/exec keys during SaveIndexExtension instead of aborting.
Description check	✅ Passed	The description directly explains the production failure, the save-time fix, the retained-key count behavior, and the regression tests covered by the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Aksha1812]

/label bug