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

feat(katana-tasks): task manager #2318

Merged
merged 4 commits into from
Aug 20, 2024
Merged

feat(katana-tasks): task manager #2318

merged 4 commits into from
Aug 20, 2024

Conversation

kariy
Copy link
Member

@kariy kariy commented Aug 20, 2024

introduce a TaskManager struct to manage tasks that can be instrumented and cancelled for graceful shutdown. there is also a task builder associated with the task manager where a task can be configured to be:

  • instrumented (currently a no-op but will soon be integrated with the metrics server)
  • cancelled
  • graceful shutdown when a critical task panicked

the idea is to spawn all main (critical) tasks (ie block production, messaging, servers, ... ) and be able to cancel the tasks for graceful shutdown (eg when a main task panicked).

Summary by CodeRabbit

  • New Features

    • Upgraded to a more recent version of the Tokio asynchronous runtime for enhanced performance and capabilities.
    • Introduced a TaskManager for efficient management and cancellation of asynchronous tasks.
    • Added a TaskBuilder and CriticalTaskBuilder for flexible task configuration and error handling.
  • Bug Fixes

    • Improved dependency management by providing warnings for unused dependencies.
  • Tests

    • Implemented unit tests to validate task management functionalities, ensuring reliability and robustness.

Copy link

coderabbitai bot commented Aug 20, 2024

Walkthrough

Ohayo, sensei! The recent changes enhance the asynchronous task management capabilities of the application. Key updates include upgrading the tokio crate and introducing new modules for better structuring. The new TaskManager and task handling systems improve the management, spawning, and cancellation of tasks, while ensuring robustness through error handling and dependency management practices.

Changes

File(s) Change Summary
Cargo.toml Updated tokio version from 1.32.0 to 1.39.2, added tokio-metrics, tokio-util, and included tracing in the workspace.
crates/.../tasks/src/lib.rs Added conditional compilation for unused dependencies, declared new modules manager and task.
crates/.../tasks/src/manager.rs Introduced TaskManager for managing asynchronous tasks, including methods for spawning and cancellation with panic handling.
crates/.../tasks/src/task.rs Added TaskBuilder, CriticalTaskBuilder, and TaskResult enum for task creation and management, improving error handling.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TaskManager
    participant TaskBuilder
    participant CriticalTaskBuilder

    User->>TaskManager: Request to spawn task
    TaskManager->>TaskBuilder: Create new task
    TaskBuilder->>TaskManager: Configure task parameters
    TaskManager->>TaskManager: Spawn task
    TaskManager->>User: Task is active

    User->>TaskManager: Request cancellation
    TaskManager->>TaskManager: Handle cancellation
    TaskManager->>User: Task cancelled or completed
Loading
sequenceDiagram
    participant User
    participant CriticalTaskBuilder

    User->>CriticalTaskBuilder: Request to spawn critical task
    CriticalTaskBuilder->>CriticalTaskBuilder: Configure critical parameters
    CriticalTaskBuilder->>User: Task is now running
    User->>CriticalTaskBuilder: Await completion or cancellation
    CriticalTaskBuilder->>User: Task completed or error handled
Loading

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4f76fd7 and dccc7f1.

Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing (5)
  • Cargo.toml (1 hunks)
  • crates/katana/tasks/Cargo.toml (1 hunks)
  • crates/katana/tasks/src/lib.rs (2 hunks)
  • crates/katana/tasks/src/manager.rs (1 hunks)
  • crates/katana/tasks/src/task.rs (1 hunks)
Additional comments not posted (8)
crates/katana/tasks/Cargo.toml (1)

13-15: Dependencies look good, sensei!

The addition of tokio-metrics and tokio-util with the "rt" feature, along with marking tracing as a workspace dependency, enhances the project's capabilities for metrics and runtime utilities.

Ensure these dependencies are used effectively in the codebase.

Run the following script to verify the usage of the new dependencies:

Verification successful

Ohayo, sensei! The new dependencies are being used effectively.

The tokio-metrics and tokio-util dependencies are actively utilized in the codebase, confirming their integration and usage:

  • tokio_util::bytes::Bytes in crates/torii/core/src/processors/metadata_update.rs
  • tokio_metrics::TaskMonitor in crates/katana/tasks/src/task.rs
  • tokio_util::sync::CancellationToken and tokio_util::task::TaskTracker in crates/katana/tasks/src/manager.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of new dependencies in the codebase.

# Test: Search for usage of `tokio-metrics` and `tokio-util`. Expect: Occurrences of their usage in the codebase.
rg --type rust 'tokio_metrics|tokio_util'

Length of output: 335

crates/katana/tasks/src/task.rs (2)

13-27: TaskResult enum is well-defined, sensei!

The TaskResult enum provides a clear distinction between completed and cancelled tasks, enhancing task management clarity.


145-160: PanickedTaskError implementation is clear, sensei!

The PanickedTaskError struct and its Display implementation provide a robust way to handle and display errors from panicked tasks.

crates/katana/tasks/src/manager.rs (3)

12-25: TaskManager struct is well-structured, sensei!

The TaskManager provides a comprehensive framework for managing tasks, including cancellation and tracking. The use of CancellationToken and TaskTracker is effective for task control.


101-174: Tests for TaskManager are comprehensive, sensei!

The test cases cover various scenarios, including normal tasks, graceful shutdowns, and critical task handling. This ensures the robustness of the task management system.


27-99: TaskManager methods are robust, sensei!

The methods for task spawning, waiting, and shutdown are well-implemented, ensuring effective task management. The use of tokio::select! for cancellation logic is a good choice.

Ensure the task management logic is thoroughly tested.

Run the following script to verify the task management logic:

Verification successful

Ohayo, sensei! Task management logic is well-tested.

The TaskManager methods are thoroughly tested with scenarios covering normal task execution, graceful shutdown, and handling of critical tasks. This ensures robust task management as described.

  • Test cases found in crates/katana/tasks/src/manager.rs cover various task management scenarios.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the task management logic in the codebase.

# Test: Search for test cases related to task management. Expect: Occurrences of task management tests.
rg --type rust 'TaskManager' --context 5

Length of output: 7150

crates/katana/tasks/src/lib.rs (1)

Line range hint 1-16: Modular structure is well-organized, sensei!

The introduction of manager and task modules enhances the modularity and maintainability of the codebase. The use of pub use statements effectively exposes the necessary functionalities.

Ensure the modules are used effectively throughout the codebase.

Run the following script to verify the module usage:

Cargo.toml (1)

202-202: Tokio version update looks good, sensei!

Updating tokio to version 1.39.2 may bring performance improvements and new features. Ensure compatibility with the rest of the codebase.

Verify that the updated version is compatible with existing code.

Run the following script to verify compatibility:


Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

crates/katana/tasks/src/task.rs Show resolved Hide resolved
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 83.61582% with 29 lines in your changes missing coverage. Please review.

Project coverage is 67.31%. Comparing base (4f76fd7) to head (dccc7f1).
Report is 1 commits behind head on main.

Files Patch % Lines
crates/katana/tasks/src/task.rs 63.88% 26 Missing ⚠️
crates/katana/tasks/src/manager.rs 97.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2318      +/-   ##
==========================================
+ Coverage   66.18%   67.31%   +1.13%     
==========================================
  Files         352      354       +2     
  Lines       46739    46433     -306     
==========================================
+ Hits        30933    31258     +325     
+ Misses      15806    15175     -631     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not for now, but wondering at which point this could be re-used by different part of the stack, and not only Katana. :)

As this concept of having critical tasks auto-cancelling others for a graceful shutdown can actually be handy for long running processes like Torii or even Sozo could leverage that using multitasking for speeding up the migration process.

@kariy
Copy link
Member Author

kariy commented Aug 20, 2024

Maybe not for now, but wondering at which point this could be re-used by different part of the stack, and not only Katana. :)

As this concept of having critical tasks auto-cancelling others for a graceful shutdown can actually be handy for long running processes like Torii or even Sozo could leverage that using multitasking for speeding up the migration process.

yeah agree i think this component is generic enough to be used outside the scope of katana. would be useful for something like torii that has to manage a bunch of async tasks at the same time. we should definitely use this (or a variation of it) in torii.

this PR doesn't include integration into katana itself but i think implementing the task manger now would be helpful just to move katana bit by bit around the task management abstraction. if i am to integrate it now there would only be 1 task (ie node service) running inside the manager. something that i need to do is to separate the node service task into smaller individual tasks ie block production and messaging. later on the servers stuff should ideally be spawned in the manager as well.

@kariy kariy merged commit bb7d7df into main Aug 20, 2024
15 checks passed
@kariy kariy deleted the katana/task-manager branch August 20, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants