Create a C shim for setting TransactionDB write policy

[eringao-stripe]

TransactionDB supports different write policies (e.g. WritePrepared), but this functionality is not currently accessible via the C API. Creating a C shim to expose the functionality for setting the write policy.

[hemal-stripe]

LGTM

[xingbowang]

/claude-review

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D107654915.

[github-actions]

⚠️ clang-tidy: 9 warning(s) on changed lines

Completed in 1466.1s.

Summary by check

Check	Count
cert-err58-cpp	4
clang-analyzer-optin.core.EnumCastOutOfRange	4
cppcoreguidelines-special-member-functions	1
Total	9

Details

db/blob/db_blob_direct_write_test.cc (1 warning(s))

db/blob/db_blob_direct_write_test.cc:127:7: warning: class 'ActiveBlobVisibilityWritableFile' defines a non-default destructor but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

include/rocksdb/file_system.h (4 warning(s))

include/rocksdb/file_system.h:195:10: warning: The value '3' provided to the cast expression is not in the valid range of values for 'FileOpenContract' [clang-analyzer-optin.core.EnumCastOutOfRange]
include/rocksdb/file_system.h:195:10: warning: The value '3' provided to the cast expression is not in the valid range of values for 'FileOpenContract' [clang-analyzer-optin.core.EnumCastOutOfRange]
include/rocksdb/file_system.h:195:10: warning: The value '3' provided to the cast expression is not in the valid range of values for 'FileOpenContract' [clang-analyzer-optin.core.EnumCastOutOfRange]
include/rocksdb/file_system.h:195:10: warning: The value '3' provided to the cast expression is not in the valid range of values for 'FileOpenContract' [clang-analyzer-optin.core.EnumCastOutOfRange]

tools/ldb_cmd.cc (4 warning(s))

tools/ldb_cmd.cc:5469:19: warning: initialization of 'kInputFile' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
tools/ldb_cmd.cc:5470:19: warning: initialization of 'kResultFile' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
tools/ldb_cmd.cc:5537:51: warning: initialization of 'ARG_JOB_DIR' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
tools/ldb_cmd.cc:5590:50: warning: initialization of 'ARG_JOB_DIR' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]

[xingbowang]

Please fix the format

[github-actions]

✅ Claude Code Review

Requested by @xingbowang

---

Summary

Clean, well-structured PR that follows established C API conventions. No high-severity issues found. The implementation correctly exposes TxnDBWritePolicy to C consumers with proper enum mapping and a good integration test.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Test coverage only exercises WRITE_PREPARED — db/c_test.c

• Issue: The test only covers rocksdb_txndb_write_policy_write_prepared. Neither WRITE_COMMITTED (the default, already implicitly tested elsewhere) nor WRITE_UNPREPARED are explicitly exercised through the new API. Since the purpose of this test is to verify the setter works for all exposed enum values, at minimum WRITE_UNPREPARED should also be tested.
• Suggested fix: Add a second test phase (transactiondb_set_write_policy_unprepared) that opens a TransactionDB with rocksdb_txndb_write_policy_write_unprepared and performs a basic put/commit/get cycle. This is low effort and validates the full enum range.

🟢 LOW / NIT

L1. using statement alphabetical ordering — db/c.cc

• Issue: The new using ROCKSDB_NAMESPACE::TxnDBWritePolicy; is inserted between TransactionDBOptions and TransactionLogIterator (after line 148). Alphabetically, TxnDBWritePolicy sorts after TransactionOptions, so it should be placed between TransactionOptions (line 150) and WaitForCompactOptions (line 151).
• Suggested fix: Move the using statement to after using ROCKSDB_NAMESPACE::TransactionOptions;.

L2. Experimental policies not documented in C header — include/rocksdb/c.h

• Issue: The C++ header (transaction_db.h:30-31) marks WRITE_PREPARED and WRITE_UNPREPARED as "EXPERIMENTAL: not as mature, well validated, nor as compatible with other features as WRITE_COMMITTED." The C enum exposes these without any such annotation.
• Suggested fix: Add a brief comment above or within the enum:

  enum {
    rocksdb_txndb_write_policy_write_committed = 0,
    /* EXPERIMENTAL: less mature than write_committed */
    rocksdb_txndb_write_policy_write_prepared = 1,
    rocksdb_txndb_write_policy_write_unprepared = 2
  };

L3. No corresponding getter function — include/rocksdb/c.h

• Issue: A rocksdb_transactiondb_options_get_write_policy() getter is not provided. While no rocksdb_transactiondb_options_get_* functions currently exist (so this is consistent), a getter would improve API completeness for configuration introspection.
• Suggested fix: Consider adding a getter in a follow-up PR. Not blocking.

Cross-Component Analysis

Context	Code executes?	Assumptions hold?	Action needed?
WritePreparedTxnDB	YES (tested)	YES	None
WriteUnpreparedTxnDB	YES (not tested)	YES	Add test (M1)
Invalid enum values	YES (falls to default)	YES — defaults to WRITE_COMMITTED safely	Consistent with C API convention
ReadOnly DB	NO	N/A	None
User-defined timestamps	N/A at setter	Checked at Open() time	None

The write_policy setter follows the same unchecked static_cast pattern used by all other C API enum setters (rocksdb_options_set_compaction_style, rocksdb_block_based_options_set_checksum, etc.). Invalid values safely default to WRITE_COMMITTED via the default: case in pessimistic_transaction_db.cc:321. No convention-breaking validation is warranted.

Positive Observations

• Enum values correctly match the C++ enum (WRITE_COMMITTED=0, WRITE_PREPARED=1, WRITE_UNPREPARED=2)
• Function naming is consistent with existing rocksdb_transactiondb_options_set_* pattern
• Test follows established c_test.c conventions (StartPhase, unique dbname, proper cleanup)
• Resource management in the test is correct — no leaks
• Placement in c.h is in the correct section (TransactionDB options)

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[eringao-stripe]

@xingbowang done! :-)

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D107654915.

[meta-codesync]

@xingbowang merged this pull request in 2b9e8dc.