Page MenuHomePhabricator

[services] Backup - Rust lib - Add initialize
ClosedPublic

Authored by karol on Aug 18 2022, 4:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jun 18, 11:29 AM
Unknown Object (File)
Tue, Jun 18, 11:29 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM
Unknown Object (File)
Tue, Jun 18, 11:28 AM

Details

Summary

Depends on D4884

Adding initializing function in rust to be called from the c++.

The goal here is to:

  • spawn a thread in rust that will listen on a thread-safe queue (initialize)
  • schedule some data on this queue (process)
  • gracefully exit the thread and release all resources (terminate)

Later on, in the spawned thread, we will connect to the blob service through the gRPC, but first things first.

I'm probably going to remove all logging before landing, I'm leaving it as is for now.

Test Plan

test plan in D4870

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 18 2022, 4:32 AM
Harbormaster failed remote builds in B11435: Diff 15730!
karol edited the test plan for this revision. (Show Details)
karol added reviewers: tomek, varun, max.
karol edited the summary of this revision. (Show Details)

Requesting review for the same reason as in https://phab.comm.dev/D4861#140658

varun requested changes to this revision.Aug 18 2022, 12:44 PM
varun added inline comments.
services/backup/rust_lib/src/lib.rs
12–13 ↗(On Diff #15735)

this can just be replaced with use lazy_static::lazy_static;

19 ↗(On Diff #15735)

Why make the Runtime optional?

24 ↗(On Diff #15735)

Do we need an Arc around the Mutex for memory management?

32 ↗(On Diff #15735)

Why do we need to add extern "C" to this fn? Isn't it going to be exposed in the extern "Rust block of the ffi module anyway?

This revision now requires changes to proceed.Aug 18 2022, 12:44 PM
services/backup/rust_lib/src/lib.rs
12–13 ↗(On Diff #15735)

thx

19 ↗(On Diff #15735)

Hmm, yes, we could basically make the runtime global, right?

At first, I made it optional so I could drop and recreate it in order to be sure to terminate the threads, but since I join the handle manually, I think I don't need to do that.

24 ↗(On Diff #15735)

oh, right, thanks

32 ↗(On Diff #15735)

You're right I should use ffi. Long story short is that I was trying to get async functions to work and for asyncs I had to use extern so I made all functions extern to be consistent, then moved away from asyncs but forgot to move back to ffi.

I messed up the git, sorry, I'm fixing this

This revision is now accepted and ready to land.Aug 22 2022, 8:57 AM