Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polling causes duplicate messages #5526

Open
MetRonnie opened this issue May 9, 2023 · 9 comments · May be fixed by #5562
Open

Polling causes duplicate messages #5526

MetRonnie opened this issue May 9, 2023 · 9 comments · May be fixed by #5562
Assignees
Labels
Milestone

Comments

@MetRonnie
Copy link
Member

Description

Polling a task that uses cylc message will result in those messages coming through again even if they have already been sent.

This is most obviously seen in the UI tree view.

Expected Behaviour

(Need to define properly)

@MetRonnie MetRonnie added the bug label May 9, 2023
@MetRonnie MetRonnie added this to the cylc-8.1.x milestone May 9, 2023
@wxtim wxtim added BLOCKED question Flag this as a question for the next Cylc project meeting. and removed BLOCKED labels May 12, 2023
@wxtim
Copy link
Member

wxtim commented May 12, 2023

I presume the the expected behaviour is not to get the message again - but is that correct @hjoliver , @oliver-sanders - I can imagine scenarios where 2 copies of a message is more useful than 0 ?

@wxtim
Copy link
Member

wxtim commented May 12, 2023

Very simply demonstrated with

[scheduling]
    initial cycle point = 1919
    [[graph]]
        R1 = foo & bar

[runtime]
    [[foo]]
        script = """
            for i in {1..5}; do
                echo $i
                cylc message -- "CRITICAL: [${i}] Hello World"
                sleep 5
            done
        """
    [[bar]]
        script = """
            for i in {1..5}; do
                echo $i
                cylc poll "${CYLC_WORKFLOW_ID}//${CYLC_TASK_CYCLE_POINT}/foo"
                sleep 5
            done
        """
cylc run --no-detach

Not completely sure it's a bug - what if we are using the message to trigger outputs and we are polling because something appears to have gone wrong?

Additionaly, in the scheduler log the message is clearly marked as having been got by polling.

Perhaps this should be a UI feature request to collapse multiple instances of the same message?

@oliver-sanders
Copy link
Member

oliver-sanders commented May 15, 2023

I can't think of a use case for logging the same message twice.

Where comms method = poll, messages could get logged a large number of times.

@wxtim
Copy link
Member

wxtim commented May 15, 2023

What happens if Cylc message has failed on the remote and we are using poll as a backup? We might then get no messages - or could we put a switch in for the poll-as-backup scenario?

@oliver-sanders
Copy link
Member

oliver-sanders commented May 15, 2023

What happens if Cylc message has failed on the remote and we are using poll as a backup?

All messages should get logged once, whether received via SSH/TCP or polling.

This issue covers the problem where Cylc doesn't check whether a message has already been logged before logging it again.

@hjoliver
Copy link
Member

Yes, once per message regardless of comms method is the desired behavior.

Note, on current master:

  • duplicate polled output messages do not re-trigger the downstream task (which wouldn't work anyway because of the "already spawned in this flow" check)
  • but duplicate polled messages do re-run associated event handlers, which is not desirable

@MetRonnie MetRonnie removed the question Flag this as a question for the next Cylc project meeting. label May 17, 2023
@wxtim wxtim self-assigned this May 24, 2023
@wxtim
Copy link
Member

wxtim commented May 24, 2023

Workflow so far...

[scheduling]
    initial cycle point = 1919
    [[graph]]
        R1 = foo & bar

[runtime]
    [[foo]]
        script = """
            cylc message -- "CRITICAL: Hello World"
            sleep 500   # Keep the job live so that I can play with the workflow.
        """
    [[bar]]
        script = """
            for i in {1..5}; do
                echo $i
                cylc poll "${CYLC_WORKFLOW_ID}//${CYLC_TASK_CYCLE_POINT}/foo"
                sleep 5
            done
        """

Every time I poll I get all previous messages - polling should check for these.

@wxtim
Copy link
Member

wxtim commented May 25, 2023

So I can work out how to avoid logging duplicate polling messages (Check the new polled message against the activity log). But it's less clear at the moment how to stop the first polled message -

If the polled line from job.status indicates CYLC_MESSAGE we know that the message has been sent. Can we assume that it has been received?

@wxtim wxtim linked a pull request Jun 1, 2023 that will close this issue
8 tasks
@MetRonnie MetRonnie linked a pull request Jun 6, 2023 that will close this issue
8 tasks
@hjoliver hjoliver modified the milestones: cylc-8.1.x, cylc-8.2.0 Jun 13, 2023
@MetRonnie MetRonnie modified the milestones: cylc-8.2.0, cylc-8.2.1 Jul 18, 2023
@wxtim
Copy link
Member

wxtim commented Jul 19, 2023

Additional questions from a conversation with @dpmatthews :

Why does the job activity log appear to show two separate log events for each poll?

It doesn't - it shows Number of messages +1.

For each line of the output from the poll it looks for [TASK JOB MESSAGE]. If it finds that it re-processes the message in that line as if it were a separate subprocess output, including logging the message separately to the overall message of the poll.

Can you get the original email to send you multiple task event handler emails? If not why?

Yes, you can. But unless you have task events set up in the global.cylc, it's relatively unlikely that users will hit on the bug in this form, which is why we've not seen it in the wild.

Even if a user does create this scenario, email batching is likely to hide the full badness of this bug.

@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.1, cylc-8.2.2 Aug 14, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.2, cylc-8.2.3 Oct 4, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.3, cylc-8.2.4 Nov 1, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.4, cylc-8.x Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants