Deployment distribution is only retried twice

[deepthidevaki]

Describe the bug

When a push deployment response is not received with in a specified timeout, DeploymentDistributor is expected to retry sending the deployment. However it was observed that it retries once, but if this retry also did not receive a response it is not retried again. As a result, the partition that did not receive the deployment will never receive it (unless a new leader is elected).

To Reproduce

Observed in the flaky test #8445

Expected behavior
Distribution is retried until success.

Environment:

• Zeebe Version: 1.3.0

[Zelldon]

Hm it is somehow hard for me to believe this 🙈 I think I have seen multiple retries if a leader is not avail in benchmarks but also in chaos experiments.

If we take a look at the code we see that the request is send via a timeout https://github.com/camunda-cloud/zeebe/blob/develop/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L108

And we handle the ERROR via retry https://github.com/camunda-cloud/zeebe/blob/develop/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L118

https://github.com/camunda-cloud/zeebe/blob/06f2f683ddc27e181ecdf7a89ea4f64f5e76f0a5/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L193-L207

I'm wondering your described scenario could happen 🤔 would this mean the send is never timedout? Which would mean we have a bug in the transport.

[deepthidevaki]

The problematic code is this
https://github.com/camunda-cloud/zeebe/blob/d89a46ebadbdeb1b3f947fe8d955dbce84eca420/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L70-L75

Here it is waiting or the response from the StreamProcessor of other partition.

As far as I understood, the code that you linked is retrying when the sending itself failed (eg:- immediately rejected by the receiver because it is not the leader).

[romansmirnov]

To my understanding, @Zelldon refers to this code in the messaging layer/service:

https://github.com/camunda-cloud/zeebe/blob/06f2f683ddc27e181ecdf7a89ea4f64f5e76f0a5/atomix/cluster/src/main/java/io/atomix/cluster/messaging/impl/NettyMessagingService.java#L225-L233

Meaning, when a request times out then the future is completed exceptionally with a TimeoutException. This TimeoutException should be handled by the DeploymentDistributor which retires the push request respectively.

That's why I am wondering why this code actually is in place

https://github.com/camunda-cloud/zeebe/blob/06f2f683ddc27e181ecdf7a89ea4f64f5e76f0a5/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L70-L82

WDYT?

[deepthidevaki]

Ah.. understood what you are refering to.
But in this case the request did not reach transport layer because
https://github.com/camunda-cloud/zeebe/blob/d89a46ebadbdeb1b3f947fe8d955dbce84eca420/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L94-L96
There was no leader at that time. (In the reproducer code the only replica is shutdown).

[deepthidevaki]

That's why I am wondering why this code actually is in place

This is necessary because it is not actually using a request-response pattern.
When Deploymentdistribute request is sent, the receiver receives and write to the logstream and immediately send a response. This response can fail due to timeout in transport or the receiver rejecting the message because it is not the leader. A success response here does not mean that the command was processed by that partition. So another message is send by the StreamProcessor in the receiver after the command is processed. This can happen after a long delay due to various reasons such as leader changes. It can also happen that this is never processed because of a leader change and this record was never committed. This message is received by the subscription created before sending the request. If this message was not received with in the timeout, we should retry sending the message. This retrying is handled by https://github.com/camunda-cloud/zeebe/blob/06f2f683ddc27e181ecdf7a89ea4f64f5e76f0a5/broker/src/main/java/io/camunda/zeebe/broker/engine/impl/DeploymentDistributorImpl.java#L70-L82

[Zelldon]

Thanks @deepthidevaki makes sense I think I can now remember again. For the sake of completeness be aware that these things are also retried after replay https://github.com/camunda-cloud/zeebe/blob/d89a46ebadbdeb1b3f947fe8d955dbce84eca420/engine/src/main/java/io/camunda/zeebe/engine/processing/deployment/distribute/DeploymentRedistributor.java

[deepthidevaki]

Thanks @deepthidevaki makes sense I think I can now remember again. For the sake of completeness be aware that these things are also retried after replay https://github.com/camunda-cloud/zeebe/blob/d89a46ebadbdeb1b3f947fe8d955dbce84eca420/engine/src/main/java/io/camunda/zeebe/engine/processing/deployment/distribute/DeploymentRedistributor.java

This also goes through DeploymentDistributorImpl, right? So we don't handle this differently?

[Zelldon]

Yes this uses also this impl class

[romansmirnov]

@deepthidevaki, I got it now! Thanks for your clarification.