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

Implement retry and timeout policy for gRPC client. #889

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

artursouza
Copy link
Member

@artursouza artursouza commented Jul 13, 2023

Description

This is intended as a reference implementation for the proposal to support resiliency in SDKs.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: dapr/dapr#6609

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@artursouza artursouza requested review from a team as code owners July 13, 2023 23:59
@artursouza artursouza marked this pull request as draft July 14, 2023 00:00
@artursouza artursouza force-pushed the retry_policy branch 12 times, most recently from dc5ec90 to 0488e9f Compare July 29, 2023 06:44
@artursouza artursouza force-pushed the retry_policy branch 3 times, most recently from 7cf8b41 to 37f00d5 Compare August 6, 2023 16:25
@artursouza artursouza marked this pull request as ready for review August 6, 2023 18:19
value: testCollection
scopes:
- grpcstateclientit
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to make it easier to run individual ITs without requiring MongoDB

}

@Test
@Ignore("Flaky when running on GitHub actions")
Copy link
Member Author

Choose a reason for hiding this comment

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

I spent significant amount of time to remove flakiness. I did fix an issue in AbstractActor because of this test and removed flakiness running locally - not sure why it still flaky in GH Actions.

Comment on lines -14 to -23
package io.dapr.utils;

class Retry {

private static final long RETRY_WAIT_MILLISECONDS = 1000;

private Retry() {
}

static void callWithRetry(Runnable function, long retryTimeoutMilliseconds) throws InterruptedException {
Copy link
Contributor

Choose a reason for hiding this comment

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

this was used in tests and not being used anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was only used in the NetworkUtils. So, I moved it there. I want to avoid ambiguity into people thinking this is the RetryPolicy.

Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this a breaking change?
Though I believe that the users have to use the same package name, to access the method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. Package here is used for internal scope.

@mukundansundar
Copy link
Contributor

LGTM just few questions.

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #889 (919419b) into master (6d65991) will decrease coverage by 0.46%.
The diff coverage is 65.32%.

@@             Coverage Diff              @@
##             master     #889      +/-   ##
============================================
- Coverage     77.49%   77.04%   -0.46%     
- Complexity     1304     1327      +23     
============================================
  Files           123      126       +3     
  Lines          4013     4091      +78     
  Branches        472      484      +12     
============================================
+ Hits           3110     3152      +42     
- Misses          662      693      +31     
- Partials        241      246       +5     
Files Changed Coverage Δ
...a/io/dapr/client/resiliency/ResiliencyOptions.java 0.00% <0.00%> (ø)
...rc/main/java/io/dapr/client/DaprClientBuilder.java 67.56% <33.33%> (-3.87%) ⬇️
...ava/io/dapr/internal/resiliency/TimeoutPolicy.java 37.50% <37.50%> (ø)
...a/io/dapr/config/MillisecondsDurationProperty.java 50.00% <50.00%> (ø)
...ain/java/io/dapr/actors/runtime/AbstractActor.java 83.78% <58.33%> (-1.55%) ⬇️
...main/java/io/dapr/actors/runtime/ActorManager.java 80.00% <60.00%> (-1.82%) ⬇️
.../java/io/dapr/internal/resiliency/RetryPolicy.java 68.42% <68.42%> (ø)
...ain/java/io/dapr/actors/client/DaprGrpcClient.java 88.88% <75.00%> (-4.66%) ⬇️
sdk/src/main/java/io/dapr/utils/NetworkUtils.java 83.33% <76.47%> (-5.13%) ⬇️
...k/src/main/java/io/dapr/client/DaprClientGrpc.java 89.13% <81.81%> (-0.24%) ⬇️
... and 2 more

... and 1 file with indirect coverage changes

Copy link
Contributor

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

lgtm

@artursouza artursouza merged commit cf8040d into dapr:master Aug 16, 2023
macromania pushed a commit to ISE-Neutrino/java-sdk that referenced this pull request Aug 21, 2023
* Implement retry and timeout policy for gRPC client.

Signed-off-by: Artur Souza <[email protected]>

* Fix invoke actor after aborted flow.

Signed-off-by: Artur Souza <[email protected]>

---------

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
mukundansundar added a commit that referenced this pull request Sep 13, 2023
* Add ElementType.Type to ActorType (#812)

Signed-off-by: LionTao <[email protected]>

Signed-off-by: LionTao <[email protected]>
Co-authored-by: Mukundan Sundararajan <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Bump codecov/codecov-action from 3.1.0 to 3.1.1 (#788)

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.0 to 3.1.1.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md)
- [Commits](codecov/codecov-action@v3.1.0...v3.1.1)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Update springboot to latest minor.patch version. (#826)

Signed-off-by: Mahmut Canga <[email protected]>

* Use runtime 1.10.0-rc.X and CLI 1.10.0-rc.X (#827)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Upgrade the version to 1.9.0-SNAPSHOT (#829)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Generate updated javadocs for 1.8.0 (#836)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Update Dapr runtime and CLI to 1.10. (#837)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Inject autoconfiguration in the Spring Boot 3 style (#831)

* Bump from spring boot 2.3.5.RELEASE to 2.7.8

Signed-off-by: Sergio <[email protected]>
(cherry picked from commit 9152c91)

* Ensure old versions of spring boot are still compatible

Signed-off-by: Sergio <[email protected]>

---------

Signed-off-by: champel <[email protected]>
Signed-off-by: Sergio <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Bump from reactor 2.3.5.RELEASE to 2.7.8 (#830)

* Bump from reactor 2.3.5.RELEASE to 2.7.8

Signed-off-by: Sergio <[email protected]>

* Simplification

Signed-off-by: Sergio <[email protected]>

---------

Signed-off-by: Sergio <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Test multiple reminder state types + improve timer tests. (#855)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Convert Config API to Stable endpoints. (#846)

Signed-off-by: Mahmut Canga <[email protected]>

* Add PubSub subscriber examples over gPRC (#833)

* add grpc subscriber

Signed-off-by: MregXN <[email protected]>

* modify README.md

Signed-off-by: MregXN <[email protected]>

* modify README.md in examples

Signed-off-by: MregXN <[email protected]>

* Modify DaprApplication to support examples where protocol is not specified.

Signed-off-by: MregXN <[email protected]>

* modify formatter to pass checkstyle

Signed-off-by: MregXN <[email protected]>

* Update springboot to latest minor.patch version. (#826)

Signed-off-by: MregXN <[email protected]>

* Use runtime 1.10.0-rc.X and CLI 1.10.0-rc.X (#827)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: MregXN <[email protected]>

* Upgrade the version to 1.9.0-SNAPSHOT (#829)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: MregXN <[email protected]>

* Generate updated javadocs for 1.8.0 (#836)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: MregXN <[email protected]>

* Update Dapr runtime and CLI to 1.10. (#837)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: MregXN <[email protected]>

* Inject autoconfiguration in the Spring Boot 3 style (#831)

* Bump from spring boot 2.3.5.RELEASE to 2.7.8

Signed-off-by: Sergio <[email protected]>
(cherry picked from commit 9152c91)

* Ensure old versions of spring boot are still compatible

Signed-off-by: Sergio <[email protected]>

---------

Signed-off-by: champel <[email protected]>
Signed-off-by: Sergio <[email protected]>
Signed-off-by: MregXN <[email protected]>

* Bump from reactor 2.3.5.RELEASE to 2.7.8 (#830)

* Bump from reactor 2.3.5.RELEASE to 2.7.8

Signed-off-by: Sergio <[email protected]>

* Simplification

Signed-off-by: Sergio <[email protected]>

---------

Signed-off-by: Sergio <[email protected]>
Signed-off-by: MregXN <[email protected]>

* rerun checks

Signed-off-by: MregXN <[email protected]>

* modify the way of grpc server starts

Signed-off-by: MregXN <[email protected]>

* modify README

Signed-off-by: MregXN <[email protected]>

* Update pom.xml

Signed-off-by: MregXN <[email protected]>

---------

Signed-off-by: MregXN <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: champel <[email protected]>
Signed-off-by: Sergio <[email protected]>
Signed-off-by: MregXN <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
Co-authored-by: champel <[email protected]>
Co-authored-by: Mukundan Sundararajan <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* auto validate actors (#863)

Signed-off-by: Mukundan Sundararajan <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Bump codecov/codecov-action from 3.1.1 to 3.1.4 (#862)

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.1 to 3.1.4.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v3.1.1...v3.1.4)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mahmut Canga <[email protected]>

* Fix 787 (#832)

* prepare before testing

* Update tests

* fix checkstyle

---------

Co-authored-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Upgrade to 1.11 RCs. (#867)

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Init for workflows

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Updating some javadocs and Years.

Signed-off-by: Hannah Kennedy <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Add missing Header

Signed-off-by: Hannah Kennedy <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* respond to PR feedback

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Update workflow example README

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Address PR feedback

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* fixup deprecated pom.xml variable

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Updates based on PR feedback

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Update pom files per feedback

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* GetInstanceState implementation (#1)

* addiny getInstanceMetadata, waitForInstanceStart and waitForInstanceCompletion implementation
---------

Co-authored-by: aymanmahmoud_microsoft <[email protected]>
Signed-off-by: Aymand Mahmoud <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Management API

Signed-off-by: Mahmut Canga <[email protected]>

* remove try/catch

Signed-off-by: Mahmut Canga <[email protected]>

* implementing getIsReplaying() method for Authoring API (#7)

Co-authored-by: Julio Rezende <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Signed-off-by: Julio Rezende <[email protected]>

* Implementing getCurrentInstant() authoring method (#5)

Co-authored-by: Julio Rezende <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Signed-off-by: Julio Rezende <[email protected]>

* Activity Implementation (#3)

Signed-off-by: Mahmut Canga <[email protected]>

* fixing issue with getIsReplaying() call (#8)

Co-authored-by: Julio Rezende <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Signed-off-by: Julio Rezende <[email protected]>

* Generate updated javadocs for 1.9.0 (#878)

* Generate updated javadocs for 1.9.0

Signed-off-by: Artur Souza <[email protected]>

* Update _index.md

Signed-off-by: Artur Souza <[email protected]>

---------

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Add .sdkmanrc config file and JDK installation instructions (#873)

* Add .sdkmanrc file with installation instructions

Signed-off-by: Emanuel Alves <[email protected]>

* Update README.md

Signed-off-by: Artur Souza <[email protected]>

---------

Signed-off-by: Emanuel Alves <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
Co-authored-by: Mukundan Sundararajan <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Add unit testing example

Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* implementing getIsReplaying() method for Authoring API (#7)

Co-authored-by: Julio Rezende <[email protected]>

Signed-off-by: Julio Rezende <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* fix parent pom

Signed-off-by: Mahmut Canga <[email protected]>

* Send Event Implementation (#10)

Signed-off-by: Mahmut Canga <[email protected]>

* Implementing allOf, anyOf, createTimer methods (#11)

Co-authored-by: Julio Rezende <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Co-authored-by: Julio Rezende <[email protected]>

* Support remote endpoint. (#877)

* Support remote endpoint.

Signed-off-by: Artur Souza <[email protected]>

* Use GRPC_ENDPOINT and HTTP_ENDPOINT in integration tests.

Signed-off-by: Artur Souza <[email protected]>

* Fix happy path for waiting for sidecar test.

Signed-off-by: Artur Souza <[email protected]>

---------

Signed-off-by: Artur Souza <[email protected]>
Co-authored-by: Mukundan Sundararajan <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* Add callSubWorkflow Implementation

Co-authored-by: Aymand Mahmoud <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Signed-off-by: Aymand Mahmoud <[email protected]>

* rename DemoSubWorkflow

Co-authored-by: Aymand Mahmoud <[email protected]>
Signed-off-by: Aymand Mahmoud <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* continueAsNew Implementation (#13)

Signed-off-by: Mahmut Canga <[email protected]>

* remove duplicate class

Signed-off-by: Mahmut Canga <[email protected]>

* add missing mockito test dependency

Signed-off-by: Mahmut Canga <[email protected]>

* use new workflow client implementation

Signed-off-by: Mahmut Canga <[email protected]>

* moved implementations to new workflow and context

Signed-off-by: Mahmut Canga <[email protected]>

* relocate duplicate implemantation

Signed-off-by: Mahmut Canga <[email protected]>

* remove duplicate test and increase test coverage

Signed-off-by: Mahmut Canga <[email protected]>

* Implement retry and timeout policy for gRPC client. (#889)

* Implement retry and timeout policy for gRPC client.

Signed-off-by: Artur Souza <[email protected]>

* Fix invoke actor after aborted flow.

Signed-off-by: Artur Souza <[email protected]>

---------

Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>

* renamed getIsReplaying

Signed-off-by: Mahmut Canga <[email protected]>

* rollback changes on client

Signed-off-by: Mahmut Canga <[email protected]>

* move workflow runtime state package

Signed-off-by: Mahmut Canga <[email protected]>

* rename workflow instance state to status

Signed-off-by: Mahmut Canga <[email protected]>

* remove unnecessary else

Signed-off-by: Mahmut Canga <[email protected]>

* removed unknown state

Signed-off-by: Mahmut Canga <[email protected]>

* updated comment

Signed-off-by: Mahmut Canga <[email protected]>

* updated workflow failure details

Signed-off-by: Mahmut Canga <[email protected]>

* fix style issues

Signed-off-by: Mahmut Canga <[email protected]>

* rollback merge change

Signed-off-by: Mahmut Canga <[email protected]>

* fixed pom files

Signed-off-by: Mahmut Canga <[email protected]>

* rollback actors pom changes on autoformat

Signed-off-by: Mahmut Canga <[email protected]>

* fixe actors pom

Signed-off-by: Mahmut Canga <[email protected]>

* fix styling on actors pom

Signed-off-by: Mahmut Canga <[email protected]>

* fix pom spacing

Signed-off-by: Mahmut Canga <[email protected]>

* move test to match the package

Signed-off-by: Mahmut Canga <[email protected]>

* add missing dependencies

Signed-off-by: Mahmut Canga <[email protected]>

* increased test coverage

Signed-off-by: Mahmut Canga <[email protected]>

* moved workflow runtime package

Signed-off-by: Mahmut Canga <[email protected]>

* add exception for missing case

Signed-off-by: Mahmut Canga <[email protected]>

* add null check for metadata

Signed-off-by: Mahmut Canga <[email protected]>

* add runtime exception error messages

Signed-off-by: Mahmut Canga <[email protected]>

* update try catch scope

Signed-off-by: Mahmut Canga <[email protected]>

* update activity definition to an interface

Signed-off-by: Mahmut Canga <[email protected]>

* update comments

Signed-off-by: Mahmut Canga <[email protected]>

* removed redundant method

Signed-off-by: Mahmut Canga <[email protected]>

* PR updates

Signed-off-by: Mahmut Canga <[email protected]>

---------

Signed-off-by: LionTao <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: champel <[email protected]>
Signed-off-by: Sergio <[email protected]>
Signed-off-by: MregXN <[email protected]>
Signed-off-by: MregXN <[email protected]>
Signed-off-by: Mukundan Sundararajan <[email protected]>
Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Hannah Kennedy <[email protected]>
Signed-off-by: Bill DeRusha <[email protected]>
Signed-off-by: Aymand Mahmoud <[email protected]>
Signed-off-by: Julio Rezende <[email protected]>
Signed-off-by: Emanuel Alves <[email protected]>
Signed-off-by: Mahmut Canga <[email protected]>
Co-authored-by: LionTao <[email protected]>
Co-authored-by: Mukundan Sundararajan <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: champel <[email protected]>
Co-authored-by: MregXN <[email protected]>
Co-authored-by: MatejNedic <[email protected]>
Co-authored-by: Bill DeRusha <[email protected]>
Co-authored-by: Hannah Kennedy <[email protected]>
Co-authored-by: Bill DeRusha <[email protected]>
Co-authored-by: Aymalla <[email protected]>
Co-authored-by: aymanmahmoud_microsoft <[email protected]>
Co-authored-by: swetakumari <[email protected]>
Co-authored-by: julio <[email protected]>
Co-authored-by: Julio Rezende <[email protected]>
Co-authored-by: Emanuel Alves <[email protected]>
Co-authored-by: Artur Souza <[email protected]>
@mukundansundar mukundansundar added this to the v1.10 milestone Sep 20, 2023
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.

SDKs to support retry and reconnection to sidecar
2 participants