Skip to content

refactor: make WAL synchronization configurable - #1226

Merged
longbinlai merged 4 commits into
alibaba:mainfrom
shirly121:codex/wal-sync-mode
Nov 12, 2026
Merged

longbinlai merged 4 commits into
alibaba:mainfrom
shirly121:codex/wal-sync-mode

Conversation

@shirly121

@shirly121 shirly121 commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

What do these changes do?

Make WAL synchronization a database-level, open-time setting with STANDARD (default) and FULL modes. On macOS, STANDARD uses fsync to reduce per-commit overhead, while FULL requests 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

  • C++ WAL synchronization, transaction, and execution-slot test suites passed.
  • Python configuration and initialization tests: 49 passed, 6 skipped.
  • Node.js WAL and initialization tests: 30 passed, 4 skipped.
  • Java end-to-end writes and recovery passed with both STANDARD and FULL server policies.
  • C++/Python/Java formatting checks and JavaScript syntax checks passed.
  • API references regenerated from source comments with Doxygen 1.13.2 and pydoc-markdown 4.8.2; a second generation left all 28 reference files unchanged.

Related issue number

Fixes #1220

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

@shirly121
shirly121 requested a review from longbinlai November 11, 2026 10:43

@neug-agent neug-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread bin/rt_server.cc
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>());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里 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 解析的覆盖。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@longbinlai longbinlai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@longbinlai
longbinlai merged commit d0cf49f into alibaba:main Nov 12, 2026
6 checks passed
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.

Make WAL sync mode configurable and default to standard fsync on macOS

3 participants