Repository navigation
refactor: make WAL synchronization configurable - #1226
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the configurable WAL synchronization change.
No open issues 🎉
What changed in this PR
Makes NeuG’s WAL synchronization policy configurable at database open time, addressing #1220. macOS defaults to standard fsync, with FULL available for stronger device-cache flushing; Linux and Windows retain their existing behavior.
Changes:
- Adds case-insensitive configuration through C++, Python, Node.js, and the standalone server, consistently applied across WAL writer lifecycles.
- Implements fallback for unsupported full synchronization while preserving synchronization-error handling.
- Documents durability tradeoffs and adds configuration, recovery, checkpoint, and client-integration coverage.
| File | Description |
|---|---|
tools/python_bind/tests/test_wal_sync.py |
Tests parsing, validation, and crash recovery across checkpoints. |
tools/python_bind/tests/test_java_driver.py |
Exercises Java writes and recovery under both policies. |
tools/python_bind/src/py_database.h |
Passes the policy into native database configuration. |
tools/python_bind/src/py_database.cc |
Accepts the native Python configuration keyword. |
tools/python_bind/neug/database.py |
Exposes, validates, and documents wal_sync_mode. |
tools/nodejs_bind/tests/test_wal_sync.js |
Tests defaults, parsing, attachment, and recovery. |
tools/nodejs_bind/src/node_database.cc |
Parses and reports the native WAL policy. |
tools/nodejs_bind/lib/database.js |
Exposes and validates walSyncMode. |
tools/java_driver/src/test/java/com/alibaba/neug/driver/JavaDriverE2ETest.java |
Adds a committed-write integration test. |
tests/transaction/test_wal_sync.cc |
Tests policy selection, fallback, and error propagation. |
tests/transaction/test_wal_replay.cc |
Covers both policies across writer and service lifecycles. |
tests/transaction/CMakeLists.txt |
Registers the synchronization test suite. |
src/transaction/wal/wal.cc |
Forwards the policy through the writer factory. |
src/transaction/wal/local_wal_writer.cc |
Selects standard or full synchronization on macOS. |
src/main/wal_writer_set.cc |
Applies the policy when creating writers. |
src/main/neug_db.cc |
Validates and propagates the open-time policy. |
include/neug/transaction/wal/wal.h |
Extends factory interfaces with the policy parameter. |
include/neug/transaction/wal/wal_sync.h |
Adds synchronization selection and unsupported-operation fallback. |
include/neug/transaction/wal/local_wal_writer.h |
Stores the local writer’s synchronization policy. |
include/neug/main/wal_writer_set.h |
Retains the policy for writer creation. |
include/neug/config.h.in |
Defines modes, case-insensitive parsing, and the default. |
doc/source/transaction/checkpoint.md |
Explains policy scope, durability tradeoffs, and the changed default. |
doc/source/reference/python_api/database.md |
Documents the Python option. |
doc/source/reference/nodejs_api/database.md |
Documents the Node.js option. |
doc/source/reference/java_api/index.md |
Documents server-side policy selection for Java writes. |
doc/source/reference/cpp_api/neug_db.md |
Shows C++ policy configuration. |
bin/rt_server.cc |
Adds the --wal-sync-mode command-line option. |
Review effort: Balanced
There was a problem hiding this comment.
This is by an automated agent, not a human.
Reviewed commit 4c5aafe. No actionable correctness findings.
Checked configuration validation and propagation through C++, Python, Node.js and the server; WAL writer creation, service activation and checkpoint rotation; unsupported FULL fallback and preservation of synchronization failures. The changed macOS default and its durability tradeoff are documented.
Validation: all current PR CI checks are successful; local Python AST parsing, JavaScript syntax checks and whitespace checks passed. I did not run a fresh native build or the runtime suites locally. Linux CI does not exercise the actual macOS F_FULLFSYNC path, and process-crash recovery tests do not establish power-loss durability.
| config.memory_level = memory_level; | ||
| config.auto_load_extension = vm["auto-load-extension"].as<bool>(); | ||
| config.wal_sync_mode = | ||
| neug::ParseWalSyncMode(vm["wal-sync-mode"].as<std::string>()); |
There was a problem hiding this comment.
这里 ParseWalSyncMode 会抛 std::invalid_argument,但整个 main() 没有 try/catch:./rt_server --wal-sync-mode bogus 会直接 std::terminate → abort,运维看到的是 "terminate called after throwing an instance of std::invalid_argument",而不是一个正常的用法错误。建议 catch 后打印 what()("wal_sync_mode must be STANDARD or FULL")并以非零码 exit。
另外这个 flag 目前没有测试覆盖——Java e2e 测试走的是 Python db.serve(),不经过 rt_server 的命令行解析;改的时候可以顺手补一条对 flag 解析的覆盖。
There was a problem hiding this comment.
Fixed in 5172924. Added a top-level try/catch to rt_server main(): standard exceptions now print their original what() message and return EXIT_FAILURE, with a fallback message for unknown exceptions. This also covers existing command-line parsing errors.
Added a subprocess regression test to the existing tests/storage/test_recovery.cc (storage_test, already run by CI). It invokes rt_server directly with invalid WAL mode, thread count, HTTP port, memory level, and an unknown option, checking normal exit with code 1 and the expected error text. The build and regression test passed, and formatting checks passed.
What do these changes do?
Make WAL synchronization a database-level, open-time setting with
STANDARD(default) andFULLmodes. On macOS,STANDARDuses fsync to reduce per-commit overhead, whileFULLrequests the stronger device-cache flush. Unsupported FULL operations fall back to standard synchronization; actual synchronization errors retain the existing append-failure handling. Linux and Windows keep their existing synchronization behavior for both modes.Expose case-insensitive configuration through C++, Python (
wal_sync_mode), Node.js (walSyncMode), and the standalone server (--wal-sync-mode). Apply the selected policy to all WAL writers, including service activation and checkpoint rotation. Java client writes inherit the server database policy.Document the configuration and add coverage for defaults, mixed-case values, invalid input, fallback and error propagation, WAL recovery, checkpoint rotation, and Node.js/Java integration. Keep the regressions in the existing CI-selected C++ transaction, Python initialization/persistence, and Node.js initialization test files.
Validation
Related issue number
Fixes #1220