ci: restrict concurrency

PATCH In review Josh Soref 4 participants 8 messages Jan 12, 2022 – Jun 23, 2022

lore

Share & tools

lore

Josh Soref

author

2022-01-12 06:50 UTC #0 lore raw

cover ci: restrict concurrency

dpdk is fairly expensive to build in GitHub.

It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.

Josh Soref (1):
ci: restrict concurrency

 .github/workflows/build.yml | 3 +++
 1 file changed, 3 insertions(+)

Signature · 1 line

2.32.0 (Apple Git-132)

Josh Soref

author

2022-01-12 06:50 UTC #1 lore raw

PATCH 1/1 ci: restrict concurrency

Signed-off-by Josh Soref <[email protected]>

---
 .github/workflows/build.yml | 3 +++
 1 file changed, 3 insertions(+)

.github/workflows/build.yml +3 -0

index 6cf997d6..a171d430 100644
@@ -12,6 +12,9 @@ defaults:
 jobs:
   build:
     name: ${{ join(matrix.config.*, '-') }}
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler }}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
+      cancel-in-progress: true
     runs-on: ${{ matrix.config.os }}
     env:
       AARCH64: ${{ matrix.config.cross == 'aarch64' }}

Signature · 1 line

2.32.0 (Apple Git-132)

Thomas Monjalon

2022-01-13 11:42 UTC #2 lore raw

Hi,

The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.

It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"

12/01/2022 07:50, Josh Soref:

.github/workflows/build.yml defaults: · line 17

Quoted 5 lines

Signed-off-by Josh Soref <[email protected]>

---

.github/workflows/build.yml +3 -0

+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler }}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
+      cancel-in-progress: true

The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?

Josh Soref

author

2022-01-13 12:41 UTC #3 lore raw

Quoted 18 lines · Thomas Monjalon

Hi,
The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.
It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"
12/01/2022 07:50, Josh Soref:
Review
Quoted 4 lines
Signed-off-by Josh Soref <[email protected]>
---
(quoted diff) +2 -0
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler

Quoted 7 lines

}}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
Quoted 1 line
+ cancel-in-progress: true
The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?

If it's on the same branch or if it's in the same pull request yes, otherwise, no.

Quoted 1 line

Thomas Monjalon

2022-02-02 14:17 UTC #4 lore raw

Aaron, David,
Please could you review this patch?
Thanks

13/01/2022 13:41, Josh Soref:

Quoted 30 lines · Thomas Monjalon

Quoted 19 lines
Hi,
The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.
It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"
12/01/2022 07:50, Josh Soref:
Review
Quoted 4 lines
Signed-off-by Josh Soref <[email protected]>
---
(quoted diff) +2 -0
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler
Quoted 7 lines
}}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
Quoted 1 line
+ cancel-in-progress: true
The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?
If it's on the same branch or if it's in the same pull request yes, otherwise, no.

Aaron Conole

2022-02-03 20:21 UTC #5 lore raw

Quoted 4 lines · Thomas Monjalon

Aaron, David,
Please could you review this patch?
Thanks

Quoted 31 lines

13/01/2022 13:41, Josh Soref:
Quoted 30 lines · Thomas Monjalon
Quoted 19 lines
Hi,
The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.
It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"
12/01/2022 07:50, Josh Soref:
Review
Quoted 4 lines
Signed-off-by Josh Soref <[email protected]>
---
(quoted diff) +2 -0
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler
Quoted 7 lines
}}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
Quoted 1 line
+ cancel-in-progress: true
The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?
If it's on the same branch or if it's in the same pull request yes, otherwise, no.

We currently have a report on every patch, which helps us when a patch series has a breaking failure in the middle and then fixes it in a later patch. With the mechanism you have here, we lose that ability - it is important to have, as a git bisect can be broken without this feature.

How much of a problem is this in practice? I want us to be good citizens, but also I don't want to lose the bisect-ability of the series.

Thomas Monjalon

2022-02-03 21:44 UTC #6 lore raw

03/02/2022 21:21, Aaron Conole:

Quoted 35 lines

Quoted 33 lines · Thomas Monjalon
Aaron, David,
Please could you review this patch?
Thanks
13/01/2022 13:41, Josh Soref:
Quoted 28 lines · Thomas Monjalon
Quoted 26 lines
Hi,
The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.
It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"
12/01/2022 07:50, Josh Soref:
Review
Quoted 4 lines
Signed-off-by Josh Soref <[email protected]>
---
(quoted diff) +2 -0
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler
}}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
Quoted 1 line
+ cancel-in-progress: true
The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
If it's on the same branch or if it's in the same pull request yes, otherwise, no.
We currently have a report on every patch, which helps us when a patch series has a breaking failure in the middle and then fixes it in a later patch. With the mechanism you have here, we lose that ability - it is important to have, as a git bisect can be broken without this feature.

Good point.

Quoted 3 lines

How much of a problem is this in practice? I want us to be good citizens, but also I don't want to lose the bisect-ability of the series.

Bisectability is important.

So we have to reject this patch, right? Or any other idea?

David Marchand

2022-06-23 08:00 UTC #7 lore raw

Quoted 39 lines · Thomas Monjalon

03/02/2022 21:21, Aaron Conole:
Quoted 37 lines
Quoted 35 lines · Thomas Monjalon
Aaron, David,
Please could you review this patch?
Thanks
13/01/2022 13:41, Josh Soref:
Quoted 30 lines · Thomas Monjalon
Quoted 19 lines
Hi,
The explanation should be in the patch, not the cover letter. Actually, you don't need a cover letter for a single patch. Copying it here:
"
dpdk is fairly expensive to build in GitHub.
It's helpful to abandon old builds as soon as there's a new build waiting instead of wasting resources on the previous round.
"
12/01/2022 07:50, Josh Soref:
Review
Quoted 4 lines
Signed-off-by Josh Soref <[email protected]>
---
(quoted diff) +2 -0
+    concurrency:
+      group: build-${{ matrix.config.os }}-${{ matrix.config.compiler
Quoted 7 lines
}}-${{ matrix.config.library }}-${{ matrix.config.cross }}-${{ matrix.config.mini }}-${{ github.event.pull_request.number || github.ref }}
Quoted 1 line
+ cancel-in-progress: true
The goal of the CI is to catch any issue in a submitted patch. Is your change cancelling a test of a patch when another one is submitted?
If it's on the same branch or if it's in the same pull request yes, otherwise, no.

You can manually (in the GHA webui) interrupt an older build if you pushed to a same branch.

Quoted 8 lines

Quoted 5 lines
We currently have a report on every patch, which helps us when a patch series has a breaking failure in the middle and then fixes it in a later patch. With the mechanism you have here, we lose that ability - it is important to have, as a git bisect can be broken without this feature.
Good point.

Quoted 7 lines

Quoted 3 lines
How much of a problem is this in practice? I want us to be good citizens, but also I don't want to lose the bisect-ability of the series.
Bisectability is important.
So we have to reject this patch, right? Or any other idea?

I prefer the current behavior too.
Marking patch as rejected.

Signature · 1 line

David Marchand