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

[Merged by Bors] - feat: fail fast if socket is stale #3054

Closed
wants to merge 6 commits into from

Conversation

morenol
Copy link
Contributor

@morenol morenol commented Mar 6, 2023

Right now if socket is stale, we get this error after 300 seconds:

    Io {
        source: Custom {
            kind: TimedOut,
            error: "Timed out: 300 secs waiting for response. API_KEY=1003, CorrelationId=27",
        },
        msg: "",
    },

This fail fast is a socket stale is detected.

/// send and wait for reply serially
#[instrument(level = "trace", skip(self, request))]
pub async fn send_receive<R>(&self, request: R) -> Result<R::Response, SocketError>
where
R: Request + Send + Sync,
{
if self.is_stale() {
return Err(SocketError::SocketStale);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should handle this on Fluvio and FluvioAdmin and call reconnect if happens

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely

Copy link
Contributor

Choose a reason for hiding this comment

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

let's create issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in #3059

@morenol morenol marked this pull request as ready for review March 7, 2023 20:02
@morenol morenol requested review from sehz and galibey March 7, 2023 20:02
@morenol morenol changed the title wip: fail fast if socket is stale feat: fail fast if socket is stale Mar 7, 2023
@sehz sehz added SPU SPU related network labels Mar 7, 2023
@sehz sehz added this to the 0.10.6 milestone Mar 7, 2023
crates/fluvio-socket/Cargo.toml Outdated Show resolved Hide resolved
crates/fluvio-socket/src/versioned.rs Outdated Show resolved Hide resolved
/// send and wait for reply serially
#[instrument(level = "trace", skip(self, request))]
pub async fn send_receive<R>(&self, request: R) -> Result<R::Response, SocketError>
where
R: Request + Send + Sync,
{
if self.is_stale() {
return Err(SocketError::SocketStale);
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely

/// send and wait for reply serially
#[instrument(level = "trace", skip(self, request))]
pub async fn send_receive<R>(&self, request: R) -> Result<R::Response, SocketError>
where
R: Request + Send + Sync,
{
if self.is_stale() {
return Err(SocketError::SocketStale);
Copy link
Contributor

Choose a reason for hiding this comment

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

let's create issue

crates/fluvio-socket/src/versioned.rs Outdated Show resolved Hide resolved
crates/fluvio-socket/src/versioned.rs Outdated Show resolved Hide resolved
@morenol morenol requested a review from sehz March 7, 2023 20:48
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

LGTM

@morenol
Copy link
Contributor Author

morenol commented Mar 7, 2023

bors r+

bors bot pushed a commit that referenced this pull request Mar 7, 2023
Right now if socket is stale, we get this error after 300 seconds:

```
    Io {
        source: Custom {
            kind: TimedOut,
            error: "Timed out: 300 secs waiting for response. API_KEY=1003, CorrelationId=27",
        },
        msg: "",
    },
```

This fail fast is a socket stale is detected.
@bors
Copy link

bors bot commented Mar 7, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: fail fast if socket is stale [Merged by Bors] - feat: fail fast if socket is stale Mar 7, 2023
@bors bors bot closed this Mar 7, 2023
@morenol morenol deleted the lmm/timeout-stale branch March 7, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
network SPU SPU related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants