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

fix: Make Java gRPC client use timeouts as expected #4237

Conversation

acevedosharp
Copy link
Contributor

@acevedosharp acevedosharp commented May 29, 2024

What this PR does / why we need it:

In my previous PR I added deadlines to the gRPC stub used in Java FeastClient, however deadlines in gRPC don't behave like timeouts and in that PR are only set once when the client is constructed, causing requests after construction to fail with deadline exceeded even though the time they took was was within the deadline the used thought they had set up; here's an example of the issue:

// Create the client, deadline starts counting from the moment client is instantiated
FeastClient client = new FeastClient(channel, Optional.empty(), Optional.of(Deadline.after(200, TimeUnit.MILLISECONDS)));
Thread.sleep(200);
client.getOnlineFeatures(...); // throws a StatusRuntimeException: DEADLINE_EXCEEDED

The getOnlineFeatures(...) call would exceed the deadline even if it took 1ms.

The official gRPC examples show the correct usage for adding timeouts is to create a copy stub with a new deadline setup for every new request as shown here, which is what is included in this PR.

Which issue(s) this PR fixes:

N/A

Fixes

N/A

Signed-off-by: Jose Acevedo <sharp.acevedo@gmail.com>
@acevedosharp acevedosharp force-pushed the fix-java-grpc-client-deadline branch from 256351d to 40d8b24 Compare May 29, 2024 21:35
Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

lgtm

@franciscojavierarceo franciscojavierarceo enabled auto-merge (squash) May 29, 2024 23:43
@franciscojavierarceo franciscojavierarceo merged commit f5a37c1 into feast-dev:master May 30, 2024
25 of 26 checks passed
franciscojavierarceo pushed a commit that referenced this pull request Jun 18, 2024
# [0.39.0](v0.38.0...v0.39.0) (2024-06-18)

### Bug Fixes

* Feast UI importlib change ([#4248](#4248)) ([5d486b8](5d486b8))
* Feature server no_feature_log argument error ([#4255](#4255)) ([15524ce](15524ce))
* Feature UI Server image won't start in an OpenShift cluster ([#4250](#4250)) ([4891f76](4891f76))
* Handles null values in data during GO Feature retrieval ([#4274](#4274)) ([c491e57](c491e57))
* Make Java gRPC client use timeouts as expected ([#4237](#4237)) ([f5a37c1](f5a37c1))
* Remove self assignment code line. ([#4238](#4238)) ([e514f66](e514f66))
* Set default values for feature_store.serve() function ([#4225](#4225)) ([fa74438](fa74438))

### Features

* Add online_read_async for dynamodb ([#4244](#4244)) ([b5ef384](b5ef384))
* Add the ability to list objects by `tags` ([#4246](#4246)) ([fbf92da](fbf92da))
* Added deadline to gRPC Java client ([#4217](#4217)) ([ff429c9](ff429c9))
* Adding vector search for sqlite ([#4176](#4176)) ([2478831](2478831))
* Change get_online_features signature, move online retrieval functions to utils ([#4278](#4278)) ([7287662](7287662))
* Feature/adding remote online store ([#4226](#4226)) ([9454d7c](9454d7c))
* List all feature views ([#4256](#4256)) ([36a574d](36a574d))
* Make RegistryServer writable ([#4231](#4231)) ([79e1143](79e1143))
* Remote offline Store  ([#4262](#4262)) ([28a3d24](28a3d24))
* Set optional full-scan for deletion ([#4189](#4189)) ([b9cadd5](b9cadd5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants