Skip to content

Conversation

timvisee
Copy link
Member

Fix snapshot restore with new shard key format introduced in #5838.

The new shard key storage format is now enabled by default in development builds through a feature flag.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you formatted your code locally using cargo +nightly fmt --all command prior to submission?
  3. Have you checked your code using cargo clippy --all --all-features command?

@timvisee timvisee requested review from generall, ffuugoo and coszio March 19, 2025 12:35
Comment on lines -187 to +189
let shard_key_mapping: ShardKeyMapping = read_json(&mapping_path)?;
shard_key_mapping
.values()
.flat_map(|v| v.iter())
.copied()
.collect()
// Use wrapper type to support both formats
let shard_key_mapping: ShardKeyMappingWrapper = read_json(&mapping_path)?;
shard_key_mapping.shard_ids()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we failed to recover the new shard format, which resulted in the CI errors here.

@timvisee timvisee force-pushed the fix-new-shard-key-storage-snapshot-restore branch from 3037f49 to 7f1adf4 Compare March 19, 2025 12:38

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@timvisee timvisee merged commit 6aa24ee into dev Mar 19, 2025
17 checks passed
@timvisee timvisee deleted the fix-new-shard-key-storage-snapshot-restore branch March 19, 2025 16:57
timvisee added a commit that referenced this pull request Mar 21, 2025
* Use new shard key storage wrapper type when recovering snapshot

* Fix compiler warning
@timvisee timvisee mentioned this pull request Mar 21, 2025
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.

3 participants