Refactor some of ASM and slot-stats functions#4
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 865827b49d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /* Skip if: no keys in the slot, it's our slot, or we are importing it. */ | ||
| if (!countKeysInSlot(i) || | ||
| clusterIsMySlot(i) || | ||
| getImportingSlotSource(i)) | ||
| { |
There was a problem hiding this comment.
Preserve keys in unassigned slots during config check
The new deletion predicate treats any slot that isn’t owned or importing as “unowned,” which includes unassigned slots. On startup, if the local node has keys in a slot that is currently unassigned (a scenario the previous logic explicitly handled by claiming the slot), verifyClusterConfigWithData() will now call clusterDeleteKeysInUnownedSlots() first and those keys are deleted before clusterClaimUnassignedSlots() can take ownership. This is a data-loss regression for clusters that load keys for unassigned slots after config desync or node restart; consider excluding unassigned slots from deletion or claim before deletion.
Useful? React with 👍 / 👎.
Benchmark PR from agentic-review-benchmarks#4