Skip to content

docs: design doc for client-side compression (VDO/dedup) — issue #277 - #398

Open
boddumanohar wants to merge 7 commits into
mainfrom
design-issue-277-client-side-compression
Open

docs: design doc for client-side compression (VDO/dedup) — issue #277#398
boddumanohar wants to merge 7 commits into
mainfrom
design-issue-277-client-side-compression

Conversation

@boddumanohar

@boddumanohar boddumanohar commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

Design document for issue #277 (client-side compression and deduplication). No implementation code yet — this is for design review before starting on the actual CSI driver / operator changes.

Rebased onto latest main. This round is the design doc only (operator/docs/designs/design-issue-277-client-side-compression.md) — the hands-on validation spike log is being iterated separately and will follow in a subsequent PR.

Covers: node-capability detection/auto-install and advertisement, topology-based scheduling gate, StorageClass/CRD changes (compression and deduplication as two independent, separately-switchable parameters — verified to have genuinely different cost profiles), VDO device management (create/reattach/grow), performance characteristics, re-provisioning/failure handling, and a compatibility review against existing CSI driver/operator features (Guardian, VolumeMigration, snapshots/clones, encryption).

Highlights worth reviewer attention

  • Compression and deduplication are independent switches, not one combined toggle. Verified hands-on: deduplication carries a large, fixed per-volume RAM cost (its dedup index); compression is essentially free in comparison. Collapsing them into one flag would force every volume to pay dedup's cost just to get compression's much cheaper benefit.
  • A real correctness gap was found and a fix validated: clone/snapshot-restore volumes carry byte-duplicate LVM/VDO metadata, which silently shadows the source volume's data rather than failing loudly. Confirmed live on a real cluster; the fix (UUID regeneration + VG/LV rename) was verified to resolve it.
  • upsertStorageClass is create-only, not create-or-update — enabling compression/dedup on an existing Pool silently has no effect on its already-created StorageClass. Flagged as an open item needing real resolution (loud signal to the admin), not just documentation.
  • Kernel/OS support is narrower than "just install a package": RHEL9/Rocky9 needs a fragile out-of-tree kmod tied to exact kernel builds; kernels ≥6.9 get native support for free; several of simplyblock's other officially-supported OSes need materially more engineering than a simple package install.
  • Several sections are marked explicitly as not yet verified (crash/interrupt recovery during volume creation, migration cutover completion, growth at the minimum size floor) — flagged rather than glossed over.

Test plan

  • Design review by team
  • No code changes in this PR — implementation to follow as a separate PR once the design is agreed

@boddumanohar
boddumanohar marked this pull request as draft August 5, 2026 11:54
Design document covering node-capability detection/install, topology-based
scheduling gate, StorageClass/CRD changes, VDO device management
(create/reattach/grow, independent compression/deduplication switches),
performance characteristics, and a compatibility review against existing
CSI driver/operator features (Guardian, VolumeMigration, snapshots/clones,
encryption). Rebased onto latest main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@boddumanohar
boddumanohar force-pushed the design-issue-277-client-side-compression branch from b89fa3b to 8a74d82 Compare August 6, 2026 07:41
@boddumanohar boddumanohar changed the title docs: design doc for client-side compression (VDO) — issue #277 docs: design doc for client-side compression (VDO/dedup) — issue #277 Aug 6, 2026
@boddumanohar
boddumanohar marked this pull request as ready for review August 6, 2026 07:53
boddumanohar and others added 4 commits August 7, 2026 12:02
Tested against the real implementation (PR #402), not just raw LVM
commands: two real PVCs with clientCompression/clientDeduplication on the
same node, distinct checksummed data, node rebooted. Both VDO instances
reattached cleanly via fresh NodeStageVolume calls (kubelet's own
bookkeeping resets on reboot too) -- kvdo module usage count exactly 2, both
VDOOperatingMode normal, both checksums matched exactly.

Caveat found and documented: the two NodeStageVolume LVM command sequences
happened to complete sequentially rather than genuinely overlapping, so
LVM's internal command locking under truly concurrent vgchange/pvscan calls
remains unexercised -- narrowed the open item accordingly rather than
closing it outright.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ation

Tested both clone paths against PR #402's real ResolveClonedVDO: a direct
PVC-to-PVC clone and a snapshot restore, both scheduled onto the same node
as their still-live source (the specific co-location scenario this finding
warns about). Both correctly resolved via vgimportclone + lvrename, mounted
cleanly with data matching the source exactly, and coexisted with the
source and each other with independent VG identities and no
cross-contamination.

Also corrected the "Detection" section: the implementation ended up simpler
than originally planned -- detection is unconditional and purely
device-identity-based, not gated on VolumeContentSource, so no separate
content-source plumbing was needed.

Found and fixed one bug along the way: the collision-detection log message
was picking up pvs's stderr WARNING: lines merged into its output instead of
just the VG name -- harmless in this run, but fixed to parse cleanly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Deliberately reproduced against PR #402's real implementation: forcibly
disconnected a VDO volume's NVMe-oF subsystem at the host level while the
node stayed up, then deleted the pod. This exposed two real bugs (both now
fixed on that branch): DeactivateVDO had no fallback for an unreachable
device, and once added, the fallback's device-name matching didn't account
for device-mapper's dash-escaping and matched nothing. With both fixed,
cleanup is now fully automatic -- confirmed by reproducing the whole
sequence a second time.

Also documented an unplanned but valuable side observation: for ~19s after
disconnect, cached reads/writes silently appeared to succeed before the
real I/O failure surfaced, at which point VDO correctly fenced itself into
read-only mode and ext4 independently aborted its journal -- both layers
protected data correctly with no wiring needed from this design.

Narrowed the remaining open item: the node-NotReady-and-rejoin path is
still unverified (this test kept the node itself healthy throughout, only
the storage connection was severed).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
XFS-on-VDO: provisioned a real VDO volume with fsType=xfs (every prior spike
used ext4). mkfs.xfs ran with no stripe-alignment flags, confirming the
xfsStripeOptions skip fires correctly; mounted cleanly, compression/dedup
stayed enabled, data round-tripped, and reattach-on-recreate worked
identically to ext4. No bugs found.

Crash-consistency of async: forced vdo_write_policy=async explicitly, wrote
one fsync()'d file and one non-fsync()'d file, then genuinely crashed the
node via sysrq (immediate reboot, zero filesystem sync) -- confirmed via a
new boot timestamp, not a graceful reboot that would have told us nothing.
The fsync()'d file survived with an exact checksum match; the non-fsync()'d
file was lost entirely, the correct POSIX outcome. Resolves the design
doc's open safety question: async correctly honors flush/FUA durability
end-to-end through NVMe-oF to the simplyblock backend.

Note on method: triggering a real kernel reboot needs CAP_SYS_BOOT, which
neither kubectl debug node's sysadmin profile nor an ordinary privileged
pod reliably grants -- required direct VM access to actually execute.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant