Skip to content

Add support for retrying internal failures#30

Closed
rculbertson wants to merge 3 commits intonathan/inputplanefrom
ryan/system-retries
Closed

Add support for retrying internal failures#30
rculbertson wants to merge 3 commits intonathan/inputplanefrom
ryan/system-retries

Conversation

@rculbertson
Copy link
Contributor

@rculbertson rculbertson commented Jun 11, 2025

This is based on #15.

Note: Please review typescript only! Everything described below hasn't been implemented in go yet. If the typescript looks good, I'll do the same in go.

This builds on the above PR in the following ways:

  • Retries up to 8 times on INTERNAL_FAILURE which can happen if inputs are lost for some reason.
  • Adds InputStrategy interface, which provides a uniform API for sending inputs to both the input plane and control plane. The interface is implemented by two classes: InputPlaneStrategy and ControlPlaneStrategy. The client will decide which to use based on the function configuration - specifically if the function definition has an input plane region specified.

@rculbertson rculbertson requested a review from ekzhang June 11, 2025 00:45
@ekzhang ekzhang changed the base branch from main to nathan/inputplane June 11, 2025 17:18
@ekzhang
Copy link
Contributor

ekzhang commented Jun 11, 2025

This seems like a pretty substantial first change — maybe we could write down what the plan is for input strategy and retries tracking long-term?

@rculbertson
Copy link
Contributor Author

@ekzhang Yes, sorry for the large PR :(. If it makes things easier, we could start by reviewing #15 - that uses the simpler FunctionOutputPoller abstraction, which is replaced by InputStrategy in this PR. Happy to do whichever is easier.

I added this comment to the InputStrategy interface explaining that it can be deleted once we have moved all traffic over to the input plane.

Co-authored-by: Eric Zhang <ekzhang1@gmail.com>
@rculbertson
Copy link
Contributor Author

Chatted with @ekzhang about this PR. To make it easier to review, I will open a new PR adding just the Strategy abstraction, implemented for just the control plane. It will not introduce any new functionality (i.e. no input plane support). We can then add input plane support in subsequent PRs.

@rculbertson
Copy link
Contributor Author

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