Adjust hash partition mapping in Eloq submodules#121
Conversation
WalkthroughUpdated submodule pointers for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
557b0e7 to
e2726a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
storage/eloq/store_handler (1)
1-1: Rollout note: 10‑bit partition remap may affect data placement and checkpoints.If partition IDs now use the lower 10 hash bits, confirm:
- No persisted identifiers or on-disk layouts depend on the old mapping.
- Rebalancer/checkpoint logic can handle the remap without data loss or double processing.
- Cross‑service messages/events that embed partition IDs remain compatible.
Consider a staged rollout with metrics and a quick rollback path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
storage/eloq/store_handler(1 hunks)storage/eloq/tx_service(1 hunks)
🔇 Additional comments (3)
storage/eloq/store_handler (2)
1-1: LGTM on the pointer update.Assuming the above checks pass, this submodule bump is good to merge.
1-1: Submodule bump to 443fce6: verify remote commit, 10‑bit partition assumptions, and C++20 ranges support.Looks fine, but please verify:
- The referenced SHAs exist in the submodule remotes.
- Parent repo doesn’t assume a different partition bit width/mask.
- CI/build uses C++20 (ranges) for the updated store_handler.
Run this script from the repo root:
storage/eloq/tx_service (1)
1-1: Submodule bump LGTM; please verify cross-service safety of the new hash-partition mapping.
- Since store_handler now uses the lower 10 hash bits for partition IDs, confirm every producer/consumer that computes/relies on partition IDs (tx_service, ingest/workers, any client SDKs) uses the same mapping to avoid write/read skew.
- If partition IDs are persisted (DB rows, Kafka keys, object prefixes, or checkpoints), plan a zero‑downtime rollout: either feature‑flag the new mapping, dual‑compute during the transition, or coordinate a strict deploy order so mixed versions don’t coexist.
- Ensure CI toolchains are C++20‑ready for the std::ranges checks in store_handler.
Run these quick checks from repo root:
e2726a0 to
0aa6333
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
storage/eloq/store_handler(1 hunks)storage/eloq/tx_service(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- storage/eloq/store_handler
| @@ -1 +1 @@ | |||
| Subproject commit 48e5c862f36a0e5096c7e1bae42a1790b7c0fba7 | |||
| Subproject commit 16f3cf1d6aabb763b1e8d2a71f0b06385a4631e3 | |||
There was a problem hiding this comment.
🧩 Analysis chain
Submodule commit hash mismatch detected.
The submodule commit shown (16f3cf1d6aabb763b1e8d2a71f0b06385a4631e3) doesn't match either the old commit (48e5c862f36a0e5096c7e1bae42a1790b7c0fba7) or the expected new commit (f0ab43d1f52a06276493d86fdaa58f766de416af) mentioned in the AI summary. This inconsistency suggests the submodule may not be pointing to the intended commit.
Please verify the correct target commit for the tx_service submodule update:
🏁 Script executed:
#!/bin/bash
# Description: Verify the submodule commit and check if it contains the expected changes
# Check current submodule status
git submodule status storage/eloq/tx_service
# Show the submodule commit details
cd storage/eloq/tx_service
git log --oneline -5
git show --stat HEAD
# Check if this commit contains hash-partition related changes
git log --oneline --grep="hash.partition\|partition.*hash" -10
git log --oneline --grep="blank.*line\|sync" -5Length of output: 1616
Fix tx_service submodule pointer (storage/eloq/tx_service).
- Superproject records: 16f3cf1d6aabb763b1e8d2a71f0b06385a4631e3.
- Local submodule HEAD: 0aa6333 (commit "update"; touches storage/eloq/store_handler and storage/eloq/tx_service).
- Neither matches expected new commit f0ab43d1f52a06276493d86fdaa58f766de416af or old commit 48e5c862f36a0e5096c7e1bae42a1790b7c0fba7.
- No commits matching the "hash-partition" grep were found in recent history.
Set the submodule to the intended commit and update the superproject pointer (fetch & checkout the target inside storage/eloq/tx_service, then git add/commit the submodule change).
🤖 Prompt for AI Agents
In storage/eloq/tx_service around line 1, the submodule pointer in the
superproject does not match the intended commit; fetch the target commit
f0ab43d1f52a06276493d86fdaa58f766de416af into the submodule and update the
superproject pointer: cd into storage/eloq/tx_service, run git fetch --all (or
fetch the correct remote), git checkout f0ab43d1f52a06276493d86fdaa58f766de416af
(or create a local branch at that commit), then cd back to the superproject, git
add storage/eloq/tx_service and git commit -m "Update storage/eloq/tx_service
submodule to f0ab43d1", and push the change; if the commit is not available from
the submodule remote, verify the correct remote/url or request the missing
commit from the author.
Summary
eloq/store_handlerto use the lower 10 hash bits for partition IDs and adoptstd::rangessort checks.gitignoreand fix the pool resize cast for clarityeloq/tx_serviceto drop a stray blank line in the hash-partition data sync implementationSummary by CodeRabbit