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

Use juliet's default timeout and timeout bubbling to make missed ACKs fatal. #4354

Conversation

marc-casperlabs
Copy link
Collaborator

Addition to #4269.

  • Makes the timeout configurable (from hardcoded 30 seconds).
  • Every request has its timeout set through juliet means (default timeout), instead of requiring special node logic.
  • In the same vein, timeouts bubble up to be a server error as well as a per-request one if configured to do so.

Copy link
Collaborator

@Fraser999 Fraser999 left a comment

Choose a reason for hiding this comment

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

Only one non-blocking nitpick. Otherwise LGTM.

let is_expired = |t: &Reverse<(Instant, IoId)>| t.0 .0 <= now;

// Track the number of actual timeouts hit.
let mut timed_out = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why bother counting them if any non-zero value triggers the error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are logged. In general, the info is free, so I thought it best to not just discard it in the library.

@marc-casperlabs marc-casperlabs merged commit ccf53fd into casper-network:feat-1.6 Oct 20, 2023
2 checks passed
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.

None yet

2 participants