Adjust hash partition mapping in Eloq submodules#199
Adjust hash partition mapping in Eloq submodules#199thweetkomputer merged 1 commit intoeloqdata:mainfrom
Conversation
WalkthroughUpdated git submodule pointers for eloq modules at Changes
Sequence Diagram(s)No sequence diagrams included — changes are metadata-only (submodule pointer updates) and do not modify control flow. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/mongo/db/modules/eloq/tx_service (1)
1-1: Submodule bump only—confirm compatibility with new 10‑bit hash partition mapping.Looks fine as a pointer update. Please verify the paired
eloq/store_handlerbump (lower 10 hash bits) doesn’t conflict with any assumptions in thistx_servicerev:
- No hard‑coded partition count/bit layout (e.g., masks > 0x3FF, ranges not equal to 1024).
- No persisted/on‑disk or cross‑service protocol fields storing prior partition IDs; if so, gate mapping by version or provide migration/backfill.
- Rollout: ensure mixed‑version clusters don’t route inconsistently during deploy; add a brief upgrade note if needed.
- Toolchain:
std::rangesusage in store_handler implies C++20; confirm CI toolchain/stdlib is compatible here as well.If all checks pass, I’m good with this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mongo/db/modules/eloq/store_handler(1 hunks)src/mongo/db/modules/eloq/tx_service(1 hunks)
🔇 Additional comments (1)
src/mongo/db/modules/eloq/store_handler (1)
1-1: Submodule bump to 6f5a167 — verify partition-remap risk & CI
- Status: store_handler submodule updated to 6f5a1678f7bcb9dc561372e7a733518f6e6671eb.
- Findings (quick scan):
- Explicit 1024 usage / loop over partitions: src/mongo/db/modules/eloq/store_handler/dynamo_scanner.cpp (scan_res_.reserve(1024) ln327; future_vec.reserve(1024) ln338; for (partition < 1024) ln340).
- HASH_PARTITION_COUNT used: src/mongo/db/modules/eloq/store_handler/data_store_service_scanner.cpp (partition_scanners_.reserve(HASH_PARTITION_COUNT); loop < HASH_PARTITION_COUNT) ln410–412.
- std::ranges usage present: src/mongo/db/modules/eloq/store_handler/eloq_data_store_service/eloq_store_data_store.cpp (std::ranges::is_sorted ln199; std::ranges::sort ln208).
- Key/row construction (possible persistent formats) to inspect for encoded partition bits: data_store_service_client.cpp (key/kv_range_record/segment_record ln1122,1158,1256), rocksdb_data_store_common.cpp (tmp_key append ln1085/1111), bigtable_handler.cpp (row_key/row_key_prefix ln1757,1979).
- Scan did not produce definitive evidence that persistent keys encode the previous partition mapping; masking/shifting assumptions (e.g., &0x3ff, >>10/20) were not conclusively found in this pass.
- Action items:
- Manually confirm no persistent key/metadata encodes the old partition mapping; if it does, gate the change and prepare a migration/backfill plan.
- Add CI/unit test to assert uniformity of the hash low-10-bits (sample outputs) and fail on bias.
- Ensure std::ranges usage compiles on all CI toolchains (add compile/test job if missing).
- Grep for masking/shifting assumptions (e.g., &0x3ff, >>10, >>20) across repo and update callers or gate rollout.
ab3592f to
9cc20dc
Compare
9cc20dc to
be12993
Compare
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