After JS schedules new backup compaction creation we want to keep track of the promise_id so we can resolve/reject it after the operation. But the logic is split Rust -> C++ (compaction creation) -> Rust (upload to backup service) so we need to keep a map from backupID to the promise id.
Details
Tested with the later diff but things that touch this code in particular:
- tested that the promise resolved after successful upload
- tested that the promise rejected with a correct message after an error on the Rust side
- tested that the promise rejected with a correct message after an error on the C++ side
The testing code looks roughly like this:
if let Err(err) = rust_op_that_can_fail().await { compaction_upload_promises::resolve(&backup_id, Err(err)); return; } let (future_id, future) = future_manager::new_future::<()>().await; cpp_that_can_fail(future_id); if let Err(err) = future.await { compaction_upload_promises::resolve(&backup_id, Err(err)); return; } compaction_upload_promises::resolve(&backup_id, Ok(()));
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Decided to change the approach a bit:
- instead of having a new function for notifying of compaction creation I instead created a general future manager so that we can pass Rust futures to the C++. This is based on D9925 but simplified
- COMPACTION_UPLOAD_PROMISES introduced in the previous revision of this diff still exists as we need to wait for uploading of the compaction to the backup service before resolving the promise. It was also simplified a bit
native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/BackupOperationsUtilities/BackupOperationsExecutor.cpp | ||
---|---|---|
12 | What does futureID originate from? | |
native/native_rust_library/src/backup/upload_handler.rs | ||
198 | I know that this line wasn't introduced in this duff but it just caught my attention when I was taking a closer look at this diff. What happens if compaction is 1GB of size? Are data loaded eagerly into memory at once or does tokio::fs::read handle it in a smarter lazy way? |
native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/BackupOperationsUtilities/BackupOperationsExecutor.cpp | ||
---|---|---|
12 | The futureID originates from a call to new_future in future_manager.rs. The general flow is let (future_id, future) = future_manager::new_future::<()>().await; // Some operation where we want to wait for the C++ to finish create_main_compaction(&backup_id, future_id); future.await; You can see the real example in the next diff. | |
native/native_rust_library/src/backup/upload_handler.rs | ||
198 | Good point. I don't think it handles it in a smarter way automatically, but it shouldn't be too hard to make it send compactions in batches. I will look into it. |
native/native_rust_library/src/backup/upload_handler.rs | ||
---|---|---|
198 | Was this investigated? |
native/native_rust_library/src/backup/upload_handler.rs | ||
---|---|---|
198 | I had this on my todo list but it wasn't directly related to this diff. It's a bit more problematic so I have created a a task for it (ENG-6976 : Streaming compaction upload) with some investigation results. |