Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log support in Rust code #3930

Closed
8 tasks done
Tracked by #5137
touilleMan opened this issue Jan 19, 2023 · 2 comments
Closed
8 tasks done
Tracked by #5137

Log support in Rust code #3930

touilleMan opened this issue Jan 19, 2023 · 2 comments
Assignees
Labels
I-Rust Impact: Rust-related stuff
Milestone

Comments

@touilleMan
Copy link
Member

touilleMan commented Jan 19, 2023

Log support requirement

If we want to use both tracing crate and log crate, we could use tracing-log (included by default in tracing-subscriber)

Converting print into log entry

  • Replace eprintln in core_fs::ManifestStorage::close_connection to log::warn
  • Replace println in texts_fixtures::testbed::ensure_testbed_server_is_started to log::info
  • Add log::info at the end of local_db::BackgroundSqliteExecutor::serve
  • Replace eprintln in local_db::SqliteExecutor::exec to log::warn
  • Replace eprintln in local_db::LocalDatabase::exec_job to log::warn
@touilleMan
Copy link
Member Author

Regarding pyo3-log, deadlock might occurs when a log is done while the GIL is already held

This is not as bad as it seems given it only occurs when the log is done from a spawned thread while the initial thread holding the GIL waits on it.
In our bindings we should never need to do that (given we send the work in the tokio thread pool before doing any work).

Another solution to avoid this is to dedicate a background thread to the log (the log functions would then only push a message for this thread, which would itself do the GIL lock). The additional benefit of this approach is it avoids GIL congestion (and hence performance drop) in the Rust threads.

@mmmarcos
Copy link
Contributor

What remained to be done was extracted into specific issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-Rust Impact: Rust-related stuff
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants