feat: separate threads for importer-offline, so perf profiling is easier

[dinhani-cw]

No description provided.

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves multithreading and asynchronous operations which require careful consideration to ensure thread safety and correct handling of asynchronous tasks. The changes are moderate in size but high in complexity due to the concurrency aspects.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The threads storage_thread and importer_thread are created but there is no explicit handling of potential panics within these threads. Unhandled panics in threads can cause the program to terminate or behave unpredictably.
	Thread Safety: The PR introduces new threads that share mutable state or resources. It's crucial to ensure that these shared resources are managed correctly to avoid issues like data races or deadlocks.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/bin/importer_offline.rs
suggestion	Consider using thread-safe wrappers or synchronization mechanisms (like Mutex, Arc, etc.) for shared resources between threads to prevent data races and ensure thread safety. [important]
relevant line	let storage_thread = thread::Builder::new().name("loader".into());

---

relevant file	src/bin/importer_offline.rs
suggestion	Implement error handling for thread spawning and join operations to manage and report errors effectively, enhancing the robustness of the application. [important]
relevant line	let _ = importer_join.join();

---

relevant file	src/bin/importer_offline.rs
suggestion	To improve code readability and maintainability, consider refactoring the thread creation and setup code into a separate function or module. This can help isolate the concurrency logic and make the main function cleaner. [medium]
relevant line	let storage_thread = thread::Builder::new().name("loader".into());

---

relevant file	src/bin/importer_offline.rs
suggestion	Add logging at key points in the thread execution to aid in debugging and performance monitoring, especially before and after long-running operations or potential points of failure. [medium]
relevant line	storage_tokio.block_on(execute_external_rpc_storage_loader(

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Bug	Handle errors from thread spawning to ensure thread creation success. It's recommended to handle the potential error from the thread::Builder::spawn method, as it returns a Result which should be checked to ensure the thread was successfully spawned. src/bin/importer_offline.rs [87-99] -let _ = storage_thread.spawn(move || { +storage_thread.spawn(move || { let _tokio_guard = storage_tokio.enter(); ... -}); +})?;
	Properly handle potential errors from thread joining to ensure thread completion. Ensure that the join method on importer_join is properly handled for errors, as it can return a Result which should be checked. src/bin/importer_offline.rs [116] -let _ = importer_join.join(); +importer_join.join()?;
Enhancement	Correct the spelling of the variable for clarity and maintainability. The variable config.paralellism appears to be a typo. Consider renaming it to config.parallelism for clarity and correctness. src/bin/importer_offline.rs [94] -config.paralellism, +config.parallelism,
Performance	Optimize the use of cancellation tokens by reducing unnecessary cloning. Instead of cloning the cancellation token multiple times, consider restructuring the code to use a more efficient synchronization or state management strategy. src/bin/importer_offline.rs [86-103] -let storage_cancellation = cancellation.clone(); -let importer_cancellation = cancellation.clone(); +let (storage_cancellation, importer_cancellation) = (cancellation.clone(), cancellation);
Best practice	Use more descriptive thread names for better clarity and maintenance. Consider using a more descriptive thread name for better debugging and maintenance. For example, instead of "loader", use "external_rpc_storage_loader". src/bin/importer_offline.rs [84] -let storage_thread = thread::Builder::new().name("loader".into()); +let storage_thread = thread::Builder::new().name("external_rpc_storage_loader".into());

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.