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

Increase katana RPC request timeout from 2 -> 20 secs #1455

Merged

Conversation

kariy
Copy link
Member

@kariy kariy commented Jan 18, 2024

Refer to #1456 for full context.

The new timeout value has been chosen arbitrarily, I just thought it seems reasonable enough.

@kariy
Copy link
Member Author

kariy commented Jan 18, 2024

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (de5bbe8) 68.51% compared to head (c1de18e) 68.51%.

Additional details and impacted files
@@             Coverage Diff             @@
##           dev/katana    #1455   +/-   ##
===========================================
  Coverage       68.51%   68.51%           
===========================================
  Files             220      220           
  Lines           21690    21690           
===========================================
  Hits            14861    14861           
  Misses           6829     6829           

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

@kariy kariy requested a review from tarrencev January 18, 2024 09:38
@kariy
Copy link
Member Author

kariy commented Jan 18, 2024

Merge activity

  • Jan 18, 10:24 AM: @kariy started a stack merge that includes this pull request via Graphite.
  • Jan 18, 10:25 AM: @kariy merged this pull request with Graphite.

@kariy kariy merged commit cb58498 into dev/katana Jan 18, 2024
10 checks passed
@kariy kariy deleted the 01-18-Increase_katana_RPC_request_timeout_from_2_-_20_secs branch January 18, 2024 15:25
kariy added a commit that referenced this pull request Jan 18, 2024
Resolves #1448

Creates new crate under `katana`, `katana-tasks`, for managing spawning blocking tasks. RPC calls that mostly perform blocking tasks are now sent to their designated threadpools and won't block the async threads.

- `TokioTaskSpawner`: mainly for spawning blocking IO-bound tasks (ie reading from storage)
- `BlockingThreadPool`: mainly for spawning expensive CPU-bound tasks

Depends on #1455 because now the RPC requests (that used to block the thread before) have to wait for the blocking tasks to finish and thus may be idling for more than 2 seconds which will result in a connection timeout.

Doing `sozo migrate` on 2s timeout will failed when calling `/esimateFee` for estimating the World contract declare tx with error `connection closed before message completed`.

Raw error message from `sozo` against the new changes:
```console
Caused by:
    Failed to deploy world: Failed to migrate world: Migrator(Provider(Other(TransportError(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(5050), path: "/", query: None, fragment: None }, source: hyper::Error(IncompleteMessage) })))))
```
kariy added a commit that referenced this pull request Jan 18, 2024
Refer to #1456 for full context.

The new timeout value has been chosen arbitrarily, I just thought it seems reasonable enough.
kariy added a commit that referenced this pull request Jan 18, 2024
Resolves #1448

Creates new crate under `katana`, `katana-tasks`, for managing spawning blocking tasks. RPC calls that mostly perform blocking tasks are now sent to their designated threadpools and won't block the async threads.

- `TokioTaskSpawner`: mainly for spawning blocking IO-bound tasks (ie reading from storage)
- `BlockingThreadPool`: mainly for spawning expensive CPU-bound tasks

Depends on #1455 because now the RPC requests (that used to block the thread before) have to wait for the blocking tasks to finish and thus may be idling for more than 2 seconds which will result in a connection timeout.

Doing `sozo migrate` on 2s timeout will failed when calling `/esimateFee` for estimating the World contract declare tx with error `connection closed before message completed`.

Raw error message from `sozo` against the new changes:
```console
Caused by:
    Failed to deploy world: Failed to migrate world: Migrator(Provider(Other(TransportError(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(5050), path: "/", query: None, fragment: None }, source: hyper::Error(IncompleteMessage) })))))
```
kariy added a commit that referenced this pull request Jan 19, 2024
Refer to #1456 for full context.

The new timeout value has been chosen arbitrarily, I just thought it seems reasonable enough.
kariy added a commit that referenced this pull request Jan 19, 2024
Resolves #1448

Creates new crate under `katana`, `katana-tasks`, for managing spawning blocking tasks. RPC calls that mostly perform blocking tasks are now sent to their designated threadpools and won't block the async threads.

- `TokioTaskSpawner`: mainly for spawning blocking IO-bound tasks (ie reading from storage)
- `BlockingThreadPool`: mainly for spawning expensive CPU-bound tasks

Depends on #1455 because now the RPC requests (that used to block the thread before) have to wait for the blocking tasks to finish and thus may be idling for more than 2 seconds which will result in a connection timeout.

Doing `sozo migrate` on 2s timeout will failed when calling `/esimateFee` for estimating the World contract declare tx with error `connection closed before message completed`.

Raw error message from `sozo` against the new changes:
```console
Caused by:
    Failed to deploy world: Failed to migrate world: Migrator(Provider(Other(TransportError(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(5050), path: "/", query: None, fragment: None }, source: hyper::Error(IncompleteMessage) })))))
```
@kariy kariy mentioned this pull request Jan 22, 2024
kariy added a commit that referenced this pull request Jan 22, 2024
Refer to #1456 for full context.

The new timeout value has been chosen arbitrarily, I just thought it seems reasonable enough.
kariy added a commit that referenced this pull request Jan 22, 2024
Resolves #1448

Creates new crate under `katana`, `katana-tasks`, for managing spawning blocking tasks. RPC calls that mostly perform blocking tasks are now sent to their designated threadpools and won't block the async threads.

- `TokioTaskSpawner`: mainly for spawning blocking IO-bound tasks (ie reading from storage)
- `BlockingThreadPool`: mainly for spawning expensive CPU-bound tasks

Depends on #1455 because now the RPC requests (that used to block the thread before) have to wait for the blocking tasks to finish and thus may be idling for more than 2 seconds which will result in a connection timeout.

Doing `sozo migrate` on 2s timeout will failed when calling `/esimateFee` for estimating the World contract declare tx with error `connection closed before message completed`.

Raw error message from `sozo` against the new changes:
```console
Caused by:
    Failed to deploy world: Failed to migrate world: Migrator(Provider(Other(TransportError(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(5050), path: "/", query: None, fragment: None }, source: hyper::Error(IncompleteMessage) })))))
```
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.

3 participants