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

Modify FunctionGetOutputs proto for client retries #2577

Closed
wants to merge 1 commit into from

Conversation

rculbertson
Copy link
Contributor

@rculbertson rculbertson commented Nov 26, 2024

~This is based on #2403. That should me reviewed and merged first. ~
Update: That PR has been merged, and this one has been rebased on it.

This modifies the request and response of FunctionGetOutputs so the client can retry lost inputs. This applies to sync inputs only.

The request has a new field expected_input_ids, so the client can optionally pass the server a list of input ids that it expects the server to be processing.

The server will check if it has a record of all these IDs. If not, it will immediately return, populating the response's new lost_input_ids field with the IDs that missing. The client can the resend those inputs.

Merging the proto changes first so we can implement and test this in the server.

Part of SVC-171


Check these boxes or delete any item (or this section) if not relevant for this PR.

  • Client+Server: this change is compatible with old servers
  • Client forward compatibility: this change ensures client can accept data intended for later versions of itself

Note on protobuf: protobuf message changes in one place may have impact to
multiple entities (client, server, worker, database). See points above.


Changelog

@rculbertson rculbertson requested a review from gongy November 26, 2024 18:42
Copy link
Contributor

@gongy gongy left a comment

Choose a reason for hiding this comment

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

I think we may need entry_ids here.

@rculbertson rculbertson force-pushed the ryan/remote-lost-inputs-proto-changes branch from 11c7806 to 99809fa Compare December 2, 2024 16:07
@rculbertson
Copy link
Contributor Author

Closing in favor of #2600

@rculbertson rculbertson closed this Dec 3, 2024
@rculbertson rculbertson deleted the ryan/remote-lost-inputs-proto-changes branch December 3, 2024 02:42
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