Full-sync streaming compression - review vs replication base - #22
Open
roshkhatri wants to merge 56 commits into
Open
Full-sync streaming compression - review vs replication base#22roshkhatri wants to merge 56 commits into
roshkhatri wants to merge 56 commits into
Conversation
Add streaming LZ4-backed RDB compression with rio decorators, stream envelope handling, integration changes, and the follow-up fixes and config cleanup needed on top of unstable. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- Remove dead code: rdbIsValidMagic() and unused #include <string.h> in rdb.h - Remove redundant first RIO_FLAG_SKIP_RDB_CHECKSUM set in rdbSaveInternal - Remove unrelated changes: config_parse_depth, USE_FAST_FLOAT, write-make-settings - Validate full 8-byte VKCS envelope in aof.c rdbFileUsesStreamingCompression - Add SAFETY comment for rdbRioHasCorruptCompressedInput cast invariant - Rename all snake_case identifiers to camelCase per Valkey conventions: types (compression_algo_t -> compressionAlgo, stream_compressor_t -> streamCompressor, compress_rio_t -> compressRio, etc.), functions (stream_writer_create -> streamWriterCreate, compress_rio_finish -> compressRioFinish, write_vkcs_envelope -> writeVkcsEnvelope, etc.), and static variables (compression_lz4_codec_impl -> compressionLz4CodecImpl) - Drop _t suffix from all types to match Valkey convention - Fix typo: streamWriterIsErrord -> streamWriterIsErrored - Replace silent dummy buffer allocation with assert(needed > 0) in streamWriterEnsureOutBuf Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
* Address streaming RDB compression review * Skip RDB CRC for streaming compression * Remove brittle 32-bit compression unit test --------- Co-authored-by: Sarthak Aggarwal <sarthagg@amazon.com> Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
--------- Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- rdbSaveInternal: the old comment claimed per-string LZF was disabled 'when algo != LZF', but the actual gate is RIO_FLAG_STREAMING_COMPRESSION on the wrapper rio. Standalone rios (DUMP, AOF rewrite, diskless) keep using LZF regardless of algo. - rdbInputStreamPrepare: flag the synchronous probe IO so a future non-blocking caller (replication) doesn't accidentally block the loop. - rdbRioHasCorruptCompressedInput: the cast is sound today only because one producer sets RIO_FLAG_STREAMING_DECOMPRESSION. Replace the 'SAFETY' assertion with a note telling the next person to add a type discriminator before adding a second producer. - rdbSaveRawString: minor wording cleanup on the per-string LZF gate. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Match the surrounding Valkey style: drop comments that restate the code, drop the Ownership/Threading/Returns boilerplate from headers, collapse repeated 'capacity-shortage is retriable' notes into one explanation per function. Behavior is unchanged. Net -285 lines; integration tests (21/21) and build are clean. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
--------- Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Drop the clamp detail (an internal streamReaderCreate concern) and keep only the caller-facing contract that buffer_size must be nonzero. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
The loaderr path exits the process, so resetting reading_config_file there is dead code. It was a leftover from the reverted config_parse_depth change and is not present upstream. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Collapse the save-notice if/else into one serverLog that always names the active algorithm (none/lzf/lz4), instead of branching the message. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Replace em dashes in the compression comments with ASCII punctuation and add a short note above streamWriterCreate describing the writer/reader push/pull model and the finish-before-free requirement. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- Simplify the streaming APIs: caller-owned structs with Init/Free and descriptive self-parameter names; codec used only for the algorithm vtable. - Store the compression algorithm id directly in the VKCS envelope and drop the separate codec enum plus its translation functions. - Make the rio decorator opaque: track connection-backed streams with a flag instead of a transport-type enum. - Route LZ4 contexts through zmalloc and assert on caller errors; keep runtime errors for corrupt or external input. - Document the feed functions' streaming contract, mark the retriable capacity-shortage returns, and drop a redundant probe re-zero. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
valkey-check-rdb reported offsets via rioTell() on every read, and rdbLoadProgressCallback computed the transport offset via rioTell() on every read even when no progress event was due. Read processed_bytes directly for the offset, and only compute the transport position when a progress event is actually reported. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Replace the separate rdb-compression-algo config with a single rdbcompression no|yes|lz4-stream. 'yes' keeps the legacy per-string LZF path; 'lz4-stream' selects whole-file streaming LZ4 RDB saves. Update valkey.conf and the RDB compression, check-rdb, and replication AOF sync tests to use the new config value. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
--------- Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- use explicit VCS wire IDs (VCS_CODEC_LZ4, VCS_STREAM_RDB) instead of serializing implementation enum values into the envelope - drop the redundant envelope checksum flag; the LZ4 frame header is the single source of truth for checksum presence, byte 5 is now reserved - make rdbchecksum and DEBUG SET-SKIP-CHECKSUM-VALIDATION control only codec checksum verification for VCS input via LZ4F skipChecksums; framing, decompression, parsing, and exact-end validation stay active - carry the physical format classified by rdbLoad() in rdbSaveInfo and use it for the AOF base reuse decision, removing the reopen/read classifier and failing closed for VCS or unknown formats - route the VCS rejection through restartAOFWithSyncRdb() cleanup so rdb-del-sync-files removes the downloaded RDB Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
--------- Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
streaming compression rio pr api simplification Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
…ession-rio-pr Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> # Conflicts: # src/valkey-check-rdb.c
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
roshkhatri
force-pushed
the
repl-streaming-compression-fullsync-v2
branch
from
July 30, 2026 17:07
eb23371 to
d8abd39
Compare
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
roshkhatri
force-pushed
the
repl-streaming-compression-fullsync-v2
branch
from
July 30, 2026 17:42
d8abd39 to
11a2fc6
Compare
roshkhatri
force-pushed
the
replication-streaming-compression-pr
branch
5 times, most recently
from
August 4, 2026 02:13
bd80272 to
fc58806
Compare
roshkhatri
force-pushed
the
replication-streaming-compression-pr
branch
from
August 5, 2026 07:58
fc58806 to
1e7389a
Compare
roshkhatri
force-pushed
the
replication-streaming-compression-pr
branch
3 times, most recently
from
August 13, 2026 20:21
9814a6d to
a7a8cba
Compare
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
2 participants
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.
Full-sync RDB payload compression (disk-based, diskless, dual-channel) as one commit over the squashed replication branch (
dbe758135), for a clean review diff. Ported to the new inline streamWriter/streamReader API (compression_rio is gone on this base).Compression is capability-negotiated: a replica advertises
REPLCONF capa compressionwhen itsrepl-compressionislz4-stream; the primary compresses a full sync only when every replica in the attaching cohort advertised it (cohort-AND). If any attaching replica is not capable, the payload is sent plaintext to all of them. $EOF framing stays plaintext; the frame checksum replaces the RDB CRC64. A link dropped mid-frame is recoverable truncation (resync), not corruption.Verified: build clean, unit tests green (truncation + in-frame corruption),
repl-fullsync-compression14/14,repl-compressiongreen, compressed CI matrix green.Counterpart of PR #20 / upstream valkey-io#4075; sibling of PR #21.