[thweetkomputer]

Context

Grouped String segment operations and typed Stream writes hold the worker's store-state mutex while reading pages and preparing private changes. A cold page read or a large range/trim can therefore delay unrelated keys on that worker even though per-key intent already serializes conflicting commands.

Behavior before and after

Existing grouped String SETRANGE/APPEND/SETBIT and typed Stream mutations now release store state during page reads and private planning. GETRANGE/GETBIT also read their immutable grouped view outside store state. Unrelated keys can proceed while the target key remains serialized, including under retained EXEC/Lua intents. Large synchronous preparation yields cooperatively; ordinary single-page String edits and small Stream appends do not gain a yield per page.

Implementation

• Pin Bycorf to upstream merge commit 0f7b4c0730d515422531528ecacb65457afdd939 from perf(net): add opt-in buffered stream reads bycorf#15.
• Retain key intent, database admission and immutable grouped metadata across preparation. Reacquire store state and validate population, authority and logical identity before publication or successful mutation no-ops.
• Keep Stream page/input admission with the prepared plan until commit. Reuse checked page loading, physical lifetime pins and GC relocation refresh.
• Preserve native candidate/loading and whole-image callback contracts. Publication continues through the existing atomic grouped writer; no durable format changes.

Design decisions and alternatives

This uses the existing unlocked preparation model for Hash/Set/List/Sorted Set instead of adding a new lock. It shortens the store-state critical section without weakening same-key or transaction ordering. Encoding/publication and small compact-demotion work may still require store state; this is not a claim that every String/Stream operation is lock-free.

Documentation and comments

Updated the storage, grouped collection and Stream architecture claims to include these preparation paths. Nearby comments explain retained ownership, no-op validation, reservation lifetime and cooperative work budgets.

Test plan

The 43-case local validation below used GCC 13 Debug on ARM64 and Bycorf 8caaf75cb131b591922af51a19861f92e2ffc893 through a local source override:

./scripts/configure_debug.sh -DLAVIK_BYCORF_SOURCE_DIR=/home/ubuntu/lavik/bycorf -DFETCHCONTENT_SOURCE_DIR_GOOGLETEST=/home/ubuntu/lavik/build_debug/_deps/googletest-src -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache
cmake --build build_debug --target lavik lavik_compact_collection_write_e2e_test lavik_grouped_ordered_write_e2e_test --parallel 8
GTEST_FILTER='GroupedStringWriteE2e.*:BatchWindows/GroupedStringCrashE2e.*' ctest --test-dir build_debug -R '^lavik_grouped_ordered_write_e2e$' --output-on-failure --verbose
GTEST_FILTER='*GroupedStreamWriteConcurrencyE2e*:*StreamNoOpsAndErrorsKeepTtlAndWatch*:*StreamPreparationFailureKeepsExecUsableAndRecovers*' ctest --test-dir build_debug -R '^lavik_compact_collection_write_e2e$' -V --output-log /tmp/lavik-grouped-locks-stream-new-final.log
GTEST_FILTER='GroupedStreamE2e.*:BatchWindows/GroupedStreamCrashE2e.*-GroupedStreamE2e.LargeRdbRoundTripKeepsMessagesAndDeletedPendingBounded' ctest --test-dir build_debug -R '^lavik_grouped_ordered_write_e2e$' -V --output-log /tmp/lavik-grouped-locks-stream-existing.log
pre-commit run
git diff --cached --check

All 43 selected cases passed without skips: 10 String, 12 new Stream concurrency/semantics/failure, and 21 existing Stream cases. Coverage includes single-worker unrelated-key progress with same-key serialization, EXEC/Lua, TTL/WATCH/no-ops, failed preparation, recovery with three workers, a Stream above 512 MiB, and nine String/Stream crash windows. The isolated EXEC test was also rerun after adding missing array-reply decoding to the test client.

The final Bycorf pin is upstream merge commit 0f7b4c0730d515422531528ecacb65457afdd939. Its source tree is identical to tested PR head c4ec8208a15279e9484a07f74bdec51cfe8e6b5f (both tree IDs are d2a27871d788aeaf6ff8f089ce650d5bf66e1976; git diff is empty). That runtime passed all five local checks and all eight upstream CI test configurations plus formatting. This PR CI is rerunning to validate the final integration.

The unrelated large Stream RDB roundtrip was excluded from this targeted run. The full compatibility suite and original performance workload have not been run here.

Risk assessment

The main risks are retaining borrowed index state across suspension, releasing memory admission before publication, or changing no-op/error behavior. The implementation owns retained metadata and plans, validates after reacquisition, and tests same-key serialization alongside unrelated-key progress, TTL/WATCH, EXEC/Lua, preparation failure and recovery.

Rollback plan

Revert this PR. No storage migration is required.

Reviewer guide

Start with String's segment preparation and the typed Stream dispatch in compact_api.cpp, then review Stream plan admission transfer and the deterministic concurrency/failure tests. This PR is based on main after #338 and is independent of #339, so the lock change can be benchmarked separately.

Follow-up work

Rerun the original 80-connection large-value benchmark, especially SETRANGE and XADD with exact MAXLEN trimming. Functional concurrency checks demonstrate removal of the lock obstruction; they do not establish an end-to-end throughput or p99.9 improvement.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 00c0918e-3c8c-4af7-be96-00f5bd60fa77

📥 Commits

Reviewing files that changed from the base of the PR and between 045df0b and 684dfa9.

📒 Files selected for processing: 10

• bycorf
• docs/architecture/04-storage-and-recovery.md
• docs/architecture/09-grouped-collections.md
• docs/architecture/12-streams.md
• src/storage/engine/compact_api.cpp
• src/storage/engine/grouped/stream.cpp
• src/storage/engine/grouped/string.cpp
• src/storage/engine/impl.h
• tests/compact_collection_write_e2e_test.cpp
• tests/grouped/ordered_write_e2e_test.cpp

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}