Skip to content

retries: Early commit can break transparent retries #10011

@ejona86

Description

@ejona86

There is the transparent retry condition:

if (rpcProgress == RpcProgress.MISCARRIED
|| (rpcProgress == RpcProgress.REFUSED
&& noMoreTransparentRetry.compareAndSet(false, true))) {

And then within the transparent retry handling, it surprisingly looks at the hedging/retry policy:

if (retryPolicy == null || retryPolicy.maxAttempts == 1) {

That's just... wrong. There should never have been be any early commit in the transparent retry case based on the retry policy. I could understand more if this was in the normal retry/hedging flows, but those don't have the early commit. And it is good they don't, as it would break transparent retries.

Seems we should just remove all the early commits.

It may be fair to have @temawi or @larry-safran work on this, as it is easy/surgical and @YifeiZhuang reviews. Y'all can discuss amongst yoruselves.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions