Skip to content

Conversation

@zymap
Copy link
Member

@zymap zymap commented Jan 28, 2021


Motivation

After introducing the new cancel CI workflow, it takes
a lot of job place which cause we have more and more CI
in the queue. I saw how the apache/airflow uses, the
apache/airflow run it as a step, so let's remove it into
the separate workflows. It will reduce our CI job usage.
Also, I test it in my own repo, it looks like working well.
https://github.com/zymap/pulsar/actions?query=branch%3Atest-duplicate-ci'

---

**Motivation**

After introducing the new cancel CI workflow, it takes
a lot of job place which cause we have more and more CI
in the queue. I saw how the apache/airflow uses, the
apache/airflow run it as a step, so let's remove it into
the separate workflows. It will reduce our CI job usage.
Also, I test it in my own repo, it looks like working well.
https://github.com/zymap/pulsar/actions?query=branch%3Atest-duplicate-ci'
@zymap zymap self-assigned this Jan 28, 2021
@codelipenghui codelipenghui added this to the 2.8.0 milestone Jan 28, 2021
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great !

I left one comment PTAL

- name: cancle previours runs
uses: apache/airflow-cancel-workflow-runs@953e057dc81d3458935a18d1184c386b0f6b5738
with:
cancelMode: allDuplicates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the documentation of apache/airflow-cancel-workflow-runs, I'd assume that this should be duplicates instead of allDuplicates.
Since duplicates is the default value, the cancelMode parameter can be omitted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some clarification: allDuplicates means "Cancels duplicate runs from all running workflows.". I don't think this makes sense for per-workflow cancellation. the default cancelMode duplicates means "Cancels duplicate runs from the same repo/branch as current run."

with:
cancelMode: allDuplicates
token: ${{ secrets.GITHUB_TOKEN }}
sourceRunId: ${{ github.event.workflow_run.id }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sourceRunId should be omitted when used in the same workflow which should be cancelled. Documentation says about sourceRunId: "Useful only in workflow_run triggered events."

Comment on lines 43 to 45
cancelMode: allDuplicates
token: ${{ secrets.GITHUB_TOKEN }}
sourceRunId: ${{ github.event.workflow_run.id }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous comments apply to all cancelMode and sourceRunId parameters

@lhotari
Copy link
Member

lhotari commented Jan 28, 2021

Adding some description about the background:
The "Cancelling duplicates" workflow jobs are piling up in the Actions runner queue as can be seen in https://github.com/apache/pulsar/actions?query=workflow%3A%22Cancelling+Duplicates%22 . Each PR update will cause 26 of these "Cancelling duplicates" jobs being added to the Actions runner queue.

The workflow that creates these jobs was introduced by PR #9159 .
The reason why it was needed was due to the previous cancellation solution introduced in #8393 getting possibly unintentionally removed by #9069. Since the solution was removed it possibly lead to the issue #9154 .

This PR will bring back a solution which is similar to #8393, however using a more advanced Github Action implementation.

@zymap
Copy link
Member Author

zymap commented Jan 28, 2021

@lhotari Thank you for the more information about this!

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zymap
Copy link
Member Author

zymap commented Jan 28, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 29, 2021

/pulsarbot run-failure-checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants