fix: Fixed memoization is unchecked after mutex synchronization. Fixes #11219 #11456

shmruin · 2023-07-26T18:35:36Z

Motivation

As it is described in issue #11219 , currently when using Mutex synchronization with Memoization cache, it seems working not correctly.
When running like the example in verification, two jobs related to same mutex and memozation works like this.

job-1 runs first (acquire the key)
when job-1 runs end, job-2 runs after that (no matter what memoization is)

This means mutex synchronization is OK but memoization cache doesn't have an effect.
However, the correct way I think how it works to be is,

job-1 runs first, and write the memoization cache.
when job-1 runs end, job-2 trys to run (acquire the key), but check the memoization.
as memoization cache already be written, job-2 just succeed and completed immediately.

I think in this way, mutex will be more usable with memoization.

Modifications

I just change the order of check & update/initialize logic in executeTemplate in operator.go
Currently it works like below.

Check if node is NIL and using memoization, and if so, initialize the nodes with memoization features on them.
Check if synchronization is added, if so, mark lock/unlock to the node. which is related to synchronization managing.

This is why memoization cache never set to 'NO' to all nodes when mutex is applied.
Two (or can be more) nodes sharing same mutex key will pass the memoization check stage, which is of course not hit, and waiting for key acquired at the next synchronization checking stage.
When acquiring or waiting for the key, the are already initialized with memoization as not hit.

I twisted this behaviour to check synchronization first and then memoization. When synchronization is applied, and when it acquires the key, it goes down to the memoization check logic if the template own it.

Verification

I added a test in operator_concurrency_test.go or you can check it with the case below.
Mutex synchronization should work and memoization of job-1 is NO and job-2 is YES, which immediately make job-2 as succeed.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: example-steps-simple
spec:
  entrypoint: main

  templates:
    - name: main
      steps:
        - - name: job-1
            template: sleep
            arguments:
              parameters:
                - name: sleep_duration
                  value: 30
          - name: job-2
            template: sleep
            arguments:
              parameters:
                - name: sleep_duration
                  value: 15

    - name: sleep
      synchronization:
        mutex:
          name: mutex-example-steps-simple
      inputs:
        parameters:
          - name: sleep_duration
      script:
        image: alpine:latest
        command: [/bin/sh]
        source: |
          echo "Sleeping for {{ inputs.parameters.sleep_duration }}"
          sleep {{ inputs.parameters.sleep_duration }}
      memoize:
        key: "memo-key-1"
        cache:
          configMap:
            name: cache-example-steps-simple

juliev0 · 2023-08-01T20:25:56Z