Conversation
Rewrite Mds.md from draft to approved state: single wire protocol with
OST (role subsets, session/data/metadata/volume opcode ranges, unified
{res,len,hash} response frame, version+feature handshake), explicit
chunk map + HRW placement, map_epoch with fence-as-watermark, two-phase
snapshots over native CoW, witness voting rules kept off the failure
hot path (counterexamples included), single MDS instance on SQLite in
v1. Protocol.md points to the shared command space.
Implementation of stage 1 showed the earlier claim wrong: create() writes sync_id 0 deliberately and F10 recreate relies on it (a random sync_id on a blank copy reads as split brain at the next open; the current set's sync_id would claim data the copy does not have). Only the .spec v0 acceptance is legacy; also record the SET_STATE -ENOSYS tolerance removal.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the design specification for the Rawstor Metadata Storage Target (MDS/MDT) specialized for block storage, outlining its architecture, data model, protocol changes, and implementation stages. The review feedback highlights a few areas for improvement in the design document, including a signature mismatch in the placement pseudocode, a potential inconsistency scenario if the MDS crashes during migration, and a missing OST-level opcode definition for client-driven snapshots.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| place(volume_id, index): | ||
| slots = HRW_choose(width, level=failure_domain, key) // distinct domains | ||
| for s in slots: ost = HRW_descend_to_leaf(s, key) | ||
| return [(slot_index, ost) ...] |
There was a problem hiding this comment.
The pseudocode for place is missing the topology parameter in its definition, even though the signature on line 282 defines it as place(volume_id, index, topology). Additionally, the variable key is used within the function body but is not defined or passed in. It would be clearer to show how key is derived from stripe_width and the volume/index.
| place(volume_id, index): | |
| slots = HRW_choose(width, level=failure_domain, key) // distinct domains | |
| for s in slots: ost = HRW_descend_to_leaf(s, key) | |
| return [(slot_index, ost) ...] | |
| place(volume_id, index, topology): | |
| key = derive_key(volume_id, index, policy.stripe_width) | |
| slots = HRW_choose(width, level=failure_domain, key, topology) // distinct domains | |
| for s in slots: ost = HRW_descend_to_leaf(s, key, topology) | |
| return [(slot_index, ost) ...] |
| 1. copy the slot to the new OST (resync machinery, source = an IN-SYNC slot) | ||
| 2. MDS reserves E' = map_epoch + 1; record E' as slot.fence on the OLD owner (fsync'd) | ||
| 3. MDS: chunk_map[index] slot -> new ost_id, publish map_epoch = E' | ||
| 4. (later) release the old slot |
There was a problem hiding this comment.
If the MDS crashes after Step 2 (fence recorded on the OLD owner) but before Step 3 (updating the map and publishing the new epoch E'), the system enters an inconsistent state. The OLD owner will reject client writes with STALE because client_map_epoch < slot.fence. However, when the client queries the MDS to refresh its map, the MDS (having restarted and lost the uncommitted state) will return the old map with the old epoch. This will cause the client to loop indefinitely or fail. Consider detailing how the MDS guarantees atomicity or recovers/resumes this multi-node migration sequence (e.g., via a persistent transaction state in SQLite).
| snapshot(volume_id): | ||
| MDS: VOL_SNAPSHOT begin -> reserve snap_id | ||
| client: drain in-flight I/O, FLUSH all IN-SYNC members (point-in-time barrier) | ||
| OST*: each IN-SYNC member -> backend CoW (zfs snapshot zvol@<snap_id> / lvcreate -s) | ||
| MDS: VOL_SNAPSHOT commit -> record snapshots[snap_id] { members = the IN-SYNC set }, | ||
| map_epoch++ | ||
| read: client SET_OBJECT(volume_id, val = snap_id) -> OST serves that version |
There was a problem hiding this comment.
In the client-driven snapshot sequence, the client is responsible for triggering the backend CoW on each IN-SYNC OST member. However, the "Protocol deltas" and "Wire protocol" sections do not define an OST-level opcode for this action (only MDS-level CMD_VOL_* opcodes are listed). To make the design complete, please specify the opcode or mechanism the client uses to instruct an OST to perform the backend CoW (e.g., CMD_OST_SNAPSHOT or an extension of SET_STATE).
1 participant
No description provided.