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.

Sign up for GitHub

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

Jump to bottom

feat: detect failures before transmitting request for side-effect heuristic #71

Merged
merged 1 commit into from
Nov 26, 2024
Merged

feat: detect failures before transmitting request for side-effect heuristic #71

merged 1 commit into from
Nov 26, 2024

Conversation

sgbalogh
Copy link
Member

@sgbalogh sgbalogh commented Nov 20, 2024 •
edited
Loading

  • frame_monitor module for detecting if a failure (tonic::Status) occurred before the request body ever emitted a frame
  • RetryableRequest impl for unary AppendServiceRequest
    • retry-ability for append session is a bit more involved, as it will involve caching inflight records -- working on that next..
  • modify should_retry to consider signal from the frame monitor service

@sgbalogh sgbalogh requested a review from a team as a code owner November 20, 2024 05:11
shikhar
shikhar reviewed Nov 20, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 20, 2024
View reviewed changes
src/service/stream.rs Outdated Show resolved Hide resolved
@sgbalogh sgbalogh force-pushed the transparent-retries branch 2 times, most recently from 9c89f7a to dec4ec9 Compare November 21, 2024 21:21
@sgbalogh sgbalogh closed this Nov 21, 2024
@sgbalogh sgbalogh reopened this Nov 21, 2024
@sgbalogh sgbalogh marked this pull request as draft November 21, 2024 21:23
@sgbalogh sgbalogh force-pushed the transparent-retries branch from dec4ec9 to ad22143 Compare November 23, 2024 19:14
sgbalogh
sgbalogh commented Nov 23, 2024
View reviewed changes
src/append_session.rs Outdated Show resolved Hide resolved
sgbalogh
sgbalogh commented Nov 23, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
@sgbalogh sgbalogh force-pushed the transparent-retries branch from ccc3835 to 9647bd6 Compare November 24, 2024 19:09
@sgbalogh sgbalogh marked this pull request as ready for review November 24, 2024 19:15
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
sgbalogh
sgbalogh commented Nov 25, 2024
View reviewed changes
src/append_session.rs Outdated Show resolved Hide resolved
sgbalogh
sgbalogh commented Nov 25, 2024
View reviewed changes
src/append_session.rs Outdated Show resolved Hide resolved
sgbalogh
sgbalogh commented Nov 25, 2024
View reviewed changes
src/append_session.rs Outdated Show resolved Hide resolved
@sgbalogh sgbalogh force-pushed the transparent-retries branch from 62b37ad to 7169c95 Compare November 25, 2024 19:28
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
@sgbalogh sgbalogh force-pushed the transparent-retries branch from 233a309 to c3c6526 Compare November 25, 2024 22:36
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
src/append_session.rs Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 25, 2024
View reviewed changes
src/client.rs Show resolved Hide resolved
shikhar
shikhar reviewed Nov 26, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 26, 2024
View reviewed changes
src/lib.rs Outdated Show resolved Hide resolved
shikhar
shikhar reviewed Nov 26, 2024
View reviewed changes
src/client.rs Outdated Show resolved Hide resolved
shikhar
shikhar approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@shikhar shikhar left a comment

Choose a reason for hiding this comment

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

Good stuff!

@sgbalogh sgbalogh force-pushed the transparent-retries branch from 091fc3f to 595b1d4 Compare November 26, 2024 03:17
@sgbalogh
Copy link
Member Author

Resolves: #42

@sgbalogh sgbalogh merged commit d697575 into main Nov 26, 2024
3 checks passed
@sgbalogh sgbalogh deleted the transparent-retries branch November 26, 2024 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vrongmeal vrongmeal vrongmeal left review comments

@shikhar shikhar shikhar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sgbalogh @shikhar @vrongmeal