Transient database errors with offload enabled cause workflow to fail

[markterm]

Summary

If there is a transient database connection error with offload enabled, then active workflows are marked as failed. I would at least expect some kind of retry.

Diagnostics

What Kubernetes provider are you using?
GKE

What version of Argo Workflows are you running?
2.11.0-rc1

Paste the logs from the workflow controller:
kubectl logs -n argo $(kubectl get pods -l app=workflow-controller -n argo -o name) | grep ${workflow}

 time="2020-11-02T13:32:42Z" level=error msg="hydration failed: dial tcp 10.X.X.X:5432: connect: connection refused" namespace=default workflow=XXXX
 time="2020-11-02T13:32:42Z" level=error msg="Failed to archive workflow" err="dial tcp 10.X.X.X:5432: connect: connection refused" namespace=default workflow=XXXX

Seems to be caused by controller.go:517 which marks any workflows as 'error' if hydration fails.

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

[alexec]

This should be fixed. I think we should create a RetryingOffloadNodeStatusRepo that delegates to another OffloadNodeStatusRepo - but adds retry using wait.ExponentialBackoff. Would you be interested in submitting a PR?

[markterm]

Thanks - I’m afraid I don’t have capacity to work on this in the immediate future.

…

On Thu, 5 Nov 2020 at 22:47, Alex Collins ***@***.***> wrote: This should be fixed. I think we should create a RetryingOffloadNodeStatusRepo that delegates to another OffloadNodeStatusRepo - but adds retry using wait.ExponentialBackoff. Would you be interested in submitting a PR? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4464 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABO7LQMZGTQPAZVYYKLT3TSOMTRXANCNFSM4TLJAWZQ> .

[alexec]

ee84892

[alexec]

Can you try with v2.11.7?

[markterm]

I tried turning off incoming connections for the DB for a few seconds while a workflow was running, and got this, which looks like a hard fail on dehydration:

workflow-controller-59d5969b9d-9j568 workflow-controller time="2020-11-09T16:37:01Z" level=warning msg="Failed to dehydrate: pq: database \"staging_codec\" is not currently accepting connections" namespace=staging workflow= XXXX-7ldnh
workflow-controller-59d5969b9d-9j568 workflow-controller time="2020-11-09T16:37:01Z" level=info msg="Updated phase Running -> Error" namespace=staging workflow= XXXX-7ldnh
workflow-controller-59d5969b9d-9j568 workflow-controller time="2020-11-09T16:37:01Z" level=info msg="Updated message  -> pq: database \"staging_codec\" is not currently accepting connections" namespace=staging workflow=XXXX-7ldnh

[alexec]

How long was your database offline for?

[markterm]

Ah - about a minute and I can see that the maximum retry is under a second. How about having more retries for DB connection issues? eg single-AZ database deployments with Amazon RDS can have outages of several minutes during maintenance.

…

On Mon, 9 Nov 2020 at 19:34, Alex Collins ***@***.***> wrote: How long was your database offline for? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4464 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABO7LSI2BXTVUU2WJSLWELSPA747ANCNFSM4TLJAWZQ> .

[alexec]

Maybe a transient error should not fail the workflow, instead, it should leave it running?

[markterm]

Yes - I think if there's an error accessing the database on hydration then we should leave it running and try again later. However if the error is on dehydration then we could end up with a set of workflow updates that we can't persist, I don't see a good way out of that so failing the workflow seems reasonable.

[alexec]

@jessesuen interesting point - SQL database could be offline for minutes.

• For hydration, was can just re-queue the workflow and try again.
• For dehydration data might be lost.

What should we do - keep retrying for minutes?

[alexec]

@markterm I've created an engineering build for you to test:

docker pull argoproj/workflow-controller:nr

[markterm]

It looked like this worked :)

[markterm]

Just checking if this is going to be merged?

[alexec]

I've just re-implemented this is a simpler version. It's targetted for v2.12 which is rc-3 today.

[markterm]

great - thanks!

…

On Mon, 23 Nov 2020 at 18:21, Alex Collins ***@***.***> wrote: I've just re-implemented this is a simpler version. It's targetted for v2.12 which is rc-3 today. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4464 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABO7LXJU7ZBIYJ5S6Z2D2TSRKRZZANCNFSM4TLJAWZQ> .

[alexec]

Merged into https://github.com/argoproj/argo/releases/tag/v2.12.0-rc4