Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

[Conjure Java Runtime] Part 19: Retrying is Futile... Sometimes. #4370

Merged
merged 6 commits into from
Nov 1, 2019

Conversation

jeremyk-91
Copy link
Contributor

Goals (and why):

Implementation Description (bullets):

  • Remove FastFailoverProxy creation from the createProxy (no failover) codepath.
  • Rename semantics of configuration in TestProxyUtils to make it obvious that the default params are with retrying enabled.

Testing (What was existing testing like? What have you done to improve it?):

  • Existing tests should work, and should be faster.

Concerns (what feedback would you like?):

  • In production things shouldn't be affected, as the single node clients created e.g. timelock's PaxosLearner etc. aren't gated behind an AwaitingLeadershipProxy. Does this make sense / do we need to perform more drastic action?

Where should we start reviewing?: ConjureJavaRuntimeTargetFactory

Priority (whenever / two weeks / yesterday): yesterday 🔥Until this merges, all builds will probably be ten minutes slower than they should be.

@changelog-app
Copy link

changelog-app bot commented Oct 31, 2019

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

We no longer create a FastFailoverProxy when creating a single node proxy. Previously, if this was created when talking to an individual TimeLock node that was not the leader (or a service that would otherwise return 308s), we would spin for about 10 seconds before returning. Users of failover proxies to TimeLock (e.g. if using the timestamp or lock services from a TransactionManager) are unaffected.

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@felixdesouza felixdesouza left a comment

Choose a reason for hiding this comment

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

Interesting!

.remotingClientConfig(() -> REMOTING_CLIENT_CONFIG)
.shouldRetry(true)
.build();
.shouldLimitPayload(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

strange indent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Not sure what happened here

@jeremyk-91 jeremyk-91 merged commit 461d0e0 into develop Nov 1, 2019
@delete-merged-branch delete-merged-branch bot deleted the jkong/bad-retryproxy branch November 1, 2019 14:51
@svc-autorelease
Copy link
Collaborator

Released 0.171.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants