Expose BackupEngine C API additions: StopBackup and rate limiters by rajsuvariya-stripe · Pull Request #14722 · facebook/rocksdb

rajsuvariya-stripe · 2026-05-08T13:48:29Z

Summary

Extends RocksDB's C API with two BackupEngine capabilities needed by
language bindings (e.g. Rust via librocksdb-sys) that consume the C API:

StopBackup: Add rocksdb_backup_engine_stop_backup() to allow cancelling
an in-progress backup.
Rate limiters: Add
rocksdb_backup_engine_options_set_backup_rate_limiter() and
rocksdb_backup_engine_options_set_restore_rate_limiter() to expose the
shared_ptr<RateLimiter> fields on BackupEngineOptions. The existing
uint64_t setters only throttle writes; these expose the richer RateLimiter
object that supports read+write throttling (e.g. kAllIo mode).

Test plan

New tests in db/c_test.c cover StopBackup and rate limiter
setter/getter roundtrips, plus opening a real backup engine with rate
limiters set and running a backup end-to-end
make check passes with no regressions

Add rocksdb_backup_engine_stop_backup() to c.h and db/c.cc so that language bindings using the C API (e.g. Rust via librocksdb-sys) can cancel an in-progress backup without needing a custom C++ shim. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> Committed-By-Agent: claude

Add rocksdb_backup_engine_options_set_backup_rate_limiter() and rocksdb_backup_engine_options_set_restore_rate_limiter() to c.h and db/c.cc so that language bindings (e.g. Rust via librocksdb-sys) can pass a full RateLimiter object — including kAllIo mode for read+write throttling — without needing a custom C++ shim. The existing backup_rate_limit / restore_rate_limit uint64_t setters only throttle writes; these new functions expose the richer shared_ptr<RateLimiter> fields that override the uint64_t limits. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> Committed-By-Agent: claude

hemal-stripe · 2026-05-11T21:04:05Z

+}
+
+void rocksdb_create_backup_options_set_flush_before_backup(
+    rocksdb_create_backup_options_t* options, unsigned char v) {


why not use a bool here instead of the unsigned char v?

Good catch — <stdbool.h> is included and bool does appear in a few places (e.g. rocksdb_multi_get_entity_cf, rocksdb_load_latest_options, rocksdb_write_buffer_manager_*). However, those look like inconsistencies
introduced in newer additions rather than an intentional shift, because:

The file explicitly documents the convention at line 40: "Bools have the type unsigned char (0 == false; rest == true)"

The overwhelming majority of the file — including all existing backup engine option setters (set_share_table_files, set_sync, set_destroy_old_data, set_backup_log_files) — uses unsigned char

Staying consistent with the immediate family of backup functions and the documented convention seems like the right call here. That said, if you'd prefer we use bool to match the newer additions and move the file
toward bool uniformity, I'm happy to make that change.

i am fine as long as code owners are fine with the format.

hemal-stripe · 2026-05-11T21:13:02Z

+    rocksdb_create_backup_options_t* options, void* callback_arg,
+    void (*callback)(void* arg)) {


Curious - how do we plan to use the callback_arg. The C++ api will call the callback_function without any arguments (std::function<void()>) - so at best we can track the number of times the callback function was called -- but that can be achieved other ways as well instead of passing callback args to this translation layer.

The C++ callback is std::function<void()> with no arguments. The C API bridges this by capturing callback_arg in a lambda that calls the C function pointer with that arg: [callback, callback_arg]]() { callback(callback_arg); }. This is the standard C idiom for stateful callbacks (same pattern as pthread_create, qsort_r, etc.) — it lets callers pass context without requiring global state. Without callback_arg, a Rust or C caller would have to use a global variable to track progress state.

hemal-stripe · 2026-05-11T21:18:46Z

+    rocksdb_env_destroy(benv);
+    CheckNoError(err);
+
+    int progress_count = 0;


progress_count is just incremented through the callback function, but I don't see validations if it was ever incremented or called into.

Fixed — added assert(progress_count > 0) after the backup call. Also set callback_trigger_interval_size = 1 when opening the backup engine so the callback fires even on the small test database (the default 4MB threshold would never be reached in a unit test).

hemal-stripe · 2026-05-11T21:19:45Z

+    {
+      rocksdb_ratelimiter_t* limiter =
+          rocksdb_ratelimiter_create_with_mode(1024 * 1024, 100 * 1000, 10, 2,
+                                               0);
+      rocksdb_backup_engine_options_set_backup_rate_limiter(bdo, limiter);
+      rocksdb_backup_engine_options_set_restore_rate_limiter(bdo, limiter);
+      rocksdb_ratelimiter_destroy(limiter);
+    }
+


can this test evolved to see if there's going to be any error in actually running the backup with provided rate limiter?

Fixed in the latest commit. For the rate limiter test, added a block that opens a real backup engine with both backup_rate_limiter and restore_rate_limiter set, runs a backup against the live DB, and asserts no error is returned.

hemal-stripe · 2026-05-11T21:20:53Z

+extern ROCKSDB_LIBRARY_API void rocksdb_backup_engine_stop_backup(
+    rocksdb_backup_engine_t* be);


would like to see a test for this method too.

Fixed in the latest commit. For stop_backup, added a call to rocksdb_backup_engine_stop_backup in the backup_with_progress_callback test after the backup completes (safe to call even when no backup is in progress, exercises the code path).

hemal-stripe · 2026-05-14T16:58:54Z

Hey @jaykorean - Can you help review this PR? Thanks!

jaykorean · 2026-05-14T18:22:35Z

@hemal-stripe I'm sorry, I currently don't have bandwidth at the moment. Added my teammates to help out.

github-actions · 2026-05-14T19:19:28Z

✅ clang-tidy: No findings on changed lines

Completed in 3414.9s.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]> Committed-By-Agent: claude

rajsuvariya-stripe · 2026-05-18T09:01:35Z

@pdillinger @archang19 I have fixed the lint issues and removed the progress callback for now (I will raise a separate PR for that). Can you please review this PR now?

rajsuvariya-stripe and others added 2 commits May 8, 2026 19:16

meta-cla Bot added the CLA Signed label May 8, 2026

This was referenced May 8, 2026

Expose BackupEngine::StopBackup() in the C API #14700

Closed

Expose backup_rate_limiter and restore_rate_limiter in the C API #14705

Closed

Expose CreateBackupOptions with progress callback in the C API #14714

Closed

rajsuvariya-stripe marked this pull request as ready for review May 8, 2026 13:50

hemal-stripe reviewed May 11, 2026

View reviewed changes

Comment thread db/c_test.c Outdated

hemal-stripe reviewed May 11, 2026

View reviewed changes

jaykorean requested review from archang19 and pdillinger May 14, 2026 18:22

rajsuvariya-stripe changed the title ~~Expose BackupEngine C API additions: StopBackup, rate limiters, and CreateBackupOptions with progress callback~~ Expose BackupEngine C API additions: StopBackup, rate limiters, and CreateBackupOptions May 15, 2026

rajsuvariya-stripe changed the title ~~Expose BackupEngine C API additions: StopBackup, rate limiters, and CreateBackupOptions~~ Expose BackupEngine C API additions: StopBackup and rate limiters May 15, 2026

Improve rate limiter test: open engine and run backup end-to-end

a23a485

Co-Authored-By: Claude Sonnet 4.6 <[email protected]> Committed-By-Agent: claude

rajsuvariya-stripe force-pushed the rajsuvariya/backup-c-api-combined branch from 27eed2a to a23a485 Compare May 15, 2026 12:50

		rocksdb_create_backup_options_t* options, void* callback_arg,
		void (callback)(void arg)) {

		extern ROCKSDB_LIBRARY_API void rocksdb_backup_engine_stop_backup(
		rocksdb_backup_engine_t* be);

Conversation

rajsuvariya-stripe commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajsuvariya-stripe May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hemal-stripe commented May 14, 2026

Uh oh!

jaykorean commented May 14, 2026

Uh oh!

github-actions Bot commented May 14, 2026

✅ clang-tidy: No findings on changed lines

Uh oh!

rajsuvariya-stripe commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rajsuvariya-stripe commented May 8, 2026 •

edited

Loading

rajsuvariya-stripe May 14, 2026 •

edited

Loading