feat(commands): add FLASH.COPY / FLASH.RENAME / FLASH.RENAMENX#24
Open
mbocevski wants to merge 2 commits into
Open
feat(commands): add FLASH.COPY / FLASH.RENAME / FLASH.RENAMENX#24mbocevski wants to merge 2 commits into
mbocevski wants to merge 2 commits into
Conversation
Atomic key-relocation primitives that close the v1.1.x gap forcing
wrappers' Bucket C into non-atomic GET+SET+DEL fallback for cross-tier
moves. All three commands work uniformly across the four flash-tier
types (FlashString, FlashHash, FlashList, FlashZSet).
Atomicity from the client's view comes from single-tick command
execution: the entire operation runs to completion within one
event-loop iteration, no client observes an intermediate state. The
Cold-read in src materialisation blocks the event loop synchronously
— matches the existing accepted pattern in lset/linsert/lrem.
Algorithm (v1):
1. Open src, type-check (one of four FlashType; else error/no-op).
2. Materialise src's value: clone for Hot, synchronous NVMe read
for Cold (same helper shape as convert.rs::cold_read).
3. Read TTL from src's typed object.
4. If dst exists with a different type, delete it first
(ModuleTypeSetValue is type-strict; native RENAME is type-blind).
5. set_value at dst with a new typed object (Tier::Hot, src's TTL).
Auto-demotion will demote to Cold later if policy warrants.
6. Apply TTL to dst.
7. For RENAME/RENAMENX: delete src. For COPY: leave src in place.
Semantics match native COPY / RENAME / RENAMENX:
- FLASH.COPY src dst [REPLACE]
→ 1 on copy, 0 if dst exists without REPLACE, 0 if src missing
or non-flash (no error)
- FLASH.RENAME src dst
→ +OK; errors if src missing or non-flash; replaces dst regardless
- FLASH.RENAMENX src dst
→ 1 on rename, 0 if dst exists; errors if src missing
Replication: replicate_verbatim (one command to replicas + AOF).
Notifications: flash.copy on COPY; flash.rename_from + flash.rename_to
on RENAME/RENAMENX. Cluster CROSSSLOT delegated to Valkey core via
key-spec declaration (1, 2, 1).
What this v1 does NOT do (deferred):
- Pointer-swap optimisation for same-tier same-type (Cold metadata
migration without data movement). Spec proposed it; v1 instead
materialises through Hot for simplicity. Follow-up perf task.
- Custom WAL record type. v1 uses standard Put/Delete records;
atomicity comes from single-tick execution, not WAL coordination.
Tests: 2 unit tests cover REPLACE keyword parsing (case insensitive,
unknown keyword rejected). 27 integration tests against a live module
cover RENAME across all 4 types, COPY across all 4 types, TTL
preservation, cross-type replacement, RENAMENX both branches (succeed
when dst absent, return 0 when dst present including native dst),
COPY with and without REPLACE, missing-src and native-src returning 0
(not error) for COPY but raising for RENAME/RENAMENX, and arity
errors.
4 tasks
CI lint job runs ruff format --check on the whole repo. The newly added test_flash_copy_rename.py was already format-clean; this commit fixes test_flash_exists.py and test_flash_mget_mset.py inherited from upstream branches in the stack.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
FLASH.COPY,FLASH.RENAME, andFLASH.RENAMENX— atomic key-relocation primitives that replace the wrappers' Bucket C non-atomic GET+SET+DEL fallback for cross-tier moves.All three commands work uniformly across the four flash-tier types. Cross-type rename (e.g. string overwriting an existing hash at dst) is type-blind, matching native
RENAMEsemantics. Atomicity from the client's view comes from single-tick command execution — no client observes an intermediate state. TTL is preserved from src to dst. Cluster-mode CROSSSLOT enforcement is delegated to Valkey core via the standard(1, 2, 1)key spec.Algorithm (v1)
lset/linsert/lrem).ModuleTypeSetValueis type-strict; native RENAME is type-blind. This was a real bug found during testing: cross-type RENAME (string→hash) errored with "Existing key has wrong Valkey type" without the explicit delete-first.set_valueat dst with a new typed object (Tier::Hot, src's TTL).What this v1 does NOT do (deferred)
New keyspace notifications:
flash.copy,flash.rename_from,flash.rename_to.Test plan
tests/test_flash_copy_rename.py— RENAME across all 4 types, COPY across all 4 types, TTL preservation, cross-type replacement (the bug case from §4 above), RENAMENX both branches (succeed when dst absent, return 0 when dst present including native dst), COPY with and without REPLACE, missing-src and native-src returning 0 for COPY but raising for RENAME/RENAMENX, arity errors[Unreleased]Added