mesg-foundation / engine

Build apps or autonomous workflows with reusable, shareable integrations connecting any service, app, blockchain or decentralized network.
https://mesg.com/
Apache License 2.0
130 stars 13 forks source link

sync completion of executions #637

Closed ilgooz closed 5 years ago

ilgooz commented 5 years ago

Currently it is possible that same execution can be completed multiple times because this check at the beginning is not enough.

Multiple results submitted for the execution at the same time can pass that check when one of them cannot be saved to db before. We need a locking mechanism per execution basis.

antho1404 commented 5 years ago

I think this issue is related to this PR https://github.com/mesg-foundation/core/pull/596

ilgooz commented 5 years ago

This issue is not related with #596. I need to explain it in a different path.

    if execution.Status != InProgress {
        return StatusError{
            ExpectedStatus: InProgress,
            ActualStatus:   execution.Status,
        }
    }

We have the check above in the execution.Complete() func. Put a hypothetical beakpoint right after this if statement. And submit two different results at the same time from service clients. In this case they will both pass this if statement and we'll have a race condition here because only one of them should pass. We need a locking mechanism per execution id in the Go level or database level to avoid this.

Clients should not pass multiple results for the same execution but we should avoid it anyway.

NicolasMahe commented 5 years ago

Closing in favor of https://github.com/mesg-foundation/core/issues/671