[MDBF-1014] GitHub refs/pull/#ID#/merge is not up to date - Jira

Details

Type: Bug
Status: Verified (View Workflow)
Priority: Major
Resolution: Unresolved
Affects Version/s: BB V1.04
Fix Version/s: BB V1.05
Component/s: Buildbot, Github
Labels:
None

Epic Link:
Maintenance and support
Sprint:
Sprint 6 (24.03.2025), Sprint 7 (07.04.2025)

Description

Problem: BuildBot not testing the right revision for Pull Requests
Affected build (one of many) https://buildbot.mariadb.org/#/builders/1/builds/54818
where:

got_revision (what is actually in the tarball) is: 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33

and revision (what metadata buildbot received via webhook) is f041b02ec1e28ca0f743abf3bf07e2349b2fab1e

How to reproduce

$ git clone --no-checkout --depth 1 https://github.com/MariaDB/server .

### Try with pull/#ID#/merge - WHAT BUILDBOT DOES

$ git fetch https://github.com/MariaDB/server refs/pull/3826/merge --depth 1

...

...

From https://github.com/MariaDB/server

 * branch              refs/pull/3826/merge -> FETCH_HEAD

$ git checkout FETCH_HEAD

...

...

HEAD is now at 0202b071 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1

### ONE DAY OLD COMMIT

git log

commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 (grafted, HEAD)

Author: Marko Mäkelä <marko.makela@mariadb.com>

Date:   Mon Mar 17 08:52:31 2025 +0100

    Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1

### THIS IS NOT OK, LIMIT 45000 shouldn't be present as per PR.

$ cat mysql-test/suite/innodb/r/lock_memory_debug.result | grep "t1 g"

INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g LIMIT 45000;

### Now let's try with pull/#ID#/head

$ git fetch https://github.com/MariaDB/server refs/pull/3826/head --depth 1

...

...

From https://github.com/MariaDB/server

 * branch              refs/pull/3826/head -> FETCH_HEAD

$ git checkout FETCH_HEAD

...

...

HEAD is now at 1f278e70 squash! a878a8ffde539c9c8c1693290d58d1f2d9ba70a8

### THIS IS OK

$ cat mysql-test/suite/innodb/r/lock_memory_debug.result | grep "t1 g;"

INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g;

As seen from the printscreen below 1f278e70 is the right commit to test.
BuildBot will use the /merge branch and its HEAD commit is what is packaged on ci.mariadb.org and tested by all builders.

Attachments

- Sort By Name
- Sort By Date
- Ascending
- Descending
- Thumbnails
- List
- Download All

image-2025-03-18-18-12-16-638.png
146 kB
2025-03-18 16:12

Activity

Ascending order - Click to sort in descending order

Varzaru Razvan-Liviu created issue - 2025-03-18 16:34

Varzaru Razvan-Liviu made changes - 2025-03-18 16:39

Field	Original Value	New Value
Description	[Zulip report\|https://mariadb.zulipchat.com/#narrow/channel/236699-Buildbot/topic/corrupted.20file.20system.20on.20x86/near/506490342] Problem: BuildBot not testing the right revision for Pull Requests Affected build (one of many) https://buildbot.mariadb.org/#/builders/1/builds/54818 where: - got_revision (what is actually in the tarball) is: 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 - and revision (what metadata buildbot received via webhook) is f041b02ec1e28ca0f743abf3bf07e2349b2fab1e How to reproduce {code:bash} $ git clone --no-checkout --depth 1 https://github.com/MariaDB/server . ### Try with pull/#ID#/merge - WHAT BUILDBOT DOES $ git fetch https://github.com/MariaDB/server refs/pull/3826/merge --depth 1 ... ... From https://github.com/MariaDB/server * branch refs/pull/3826/merge -> FETCH_HEAD $ git checkout FETCH_HEAD ... ... HEAD is now at 0202b071 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1 ### ONE DAY OLD COMMIT git log commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 (grafted, HEAD) Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Mon Mar 17 08:52:31 2025 +0100 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1 ### THIS IS NOT OK, LIMIT 45000 shouldn't be present as per PR. $ cat mysql-test/suite/innodb/r/lock_memory_debug.result \| grep "t1 g" INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g LIMIT 45000; ### Now let's try with pull/#ID#/head $ git fetch https://github.com/MariaDB/server refs/pull/3826/head --depth 1 ... ... From https://github.com/MariaDB/server * branch refs/pull/3826/head -> FETCH_HEAD $ git checkout FETCH_HEAD ... ... HEAD is now at 1f278e70 squash! a878a8ffde539c9c8c1693290d58d1f2d9ba70a8 ### THIS IS OK $ cat mysql-test/suite/innodb/r/lock_memory_debug.result \| grep "t1 g;" INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g; {code} As seen from the printscreen below 1f278e70 is the right commit to test. BuildBot will use the /merge branch and the HEAD of it is what is packaged and tested by all builders. !image-2025-03-18-18-12-16-638.png\|thumbnail!	[Zulip report\|https://mariadb.zulipchat.com/#narrow/channel/236699-Buildbot/topic/corrupted.20file.20system.20on.20x86/near/506490342] Problem: BuildBot not testing the right revision for Pull Requests Affected build (one of many) https://buildbot.mariadb.org/#/builders/1/builds/54818 where: - got_revision (what is actually in the tarball) is: 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 - and revision (what metadata buildbot received via webhook) is f041b02ec1e28ca0f743abf3bf07e2349b2fab1e How to reproduce {code:bash} $ git clone --no-checkout --depth 1 https://github.com/MariaDB/server . ### Try with pull/#ID#/merge - WHAT BUILDBOT DOES $ git fetch https://github.com/MariaDB/server refs/pull/3826/merge --depth 1 ... ... From https://github.com/MariaDB/server * branch refs/pull/3826/merge -> FETCH_HEAD $ git checkout FETCH_HEAD ... ... HEAD is now at 0202b071 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1 ### ONE DAY OLD COMMIT git log commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 (grafted, HEAD) Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Mon Mar 17 08:52:31 2025 +0100 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1 ### THIS IS NOT OK, LIMIT 45000 shouldn't be present as per PR. $ cat mysql-test/suite/innodb/r/lock_memory_debug.result \| grep "t1 g" INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g LIMIT 45000; ### Now let's try with pull/#ID#/head $ git fetch https://github.com/MariaDB/server refs/pull/3826/head --depth 1 ... ... From https://github.com/MariaDB/server * branch refs/pull/3826/head -> FETCH_HEAD $ git checkout FETCH_HEAD ... ... HEAD is now at 1f278e70 squash! a878a8ffde539c9c8c1693290d58d1f2d9ba70a8 ### THIS IS OK $ cat mysql-test/suite/innodb/r/lock_memory_debug.result \| grep "t1 g;" INSERT INTO t1 SELECT a.* FROM t1 a, t1 b, t1 c, t1 d, t1 e, t1 f, t1 g; {code} As seen from the printscreen below 1f278e70 is the right commit to test. BuildBot will use the /merge branch and its HEAD commit is what is packaged on ci.mariadb.org and tested by all builders. !image-2025-03-18-18-12-16-638.png\|thumbnail!

Varzaru Razvan-Liviu added a comment - 2025-03-18 16:59 - edited

cvicentiu / danblack

I think we should consider telling buildbot to track refs/pull/#ID#/head. What do you think?
https://docs.buildbot.net/latest/manual/configuration/wwwhooks.html
https://docs.buildbot.net/latest/manual/configuration/steps/source_github.html

pullrequest_ref (default merge)

Remote ref to test if a pull request is sent to the endpoint. See the GitHub developer manual for possible values for pull requests. (e.g. head)

Something like:

c["www"]["change_hook_dialects"] = {

    "github": {

        "secret": config["private"]["gh_secret"],

        "strict": True,

        "pullrequest_ref": "head"

One of the best explanations I've read so far that motivates me to switch to refs/pull/head
https://github.com/readthedocs/readthedocs.org/issues/6958#issuecomment-655203986

And we already have enforced that the developer must rebase before merging, at this point pull/head == pull/merge
The Require branches to be up to date before merging box is checked on our branches.

Varzaru Razvan-Liviu added a comment - 2025-03-18 16:59 - edited cvicentiu / danblack I think we should consider telling buildbot to track refs/pull/#ID#/head. What do you think? https://docs.buildbot.net/latest/manual/configuration/wwwhooks.html https://docs.buildbot.net/latest/manual/configuration/steps/source_github.html pullrequest_ref ( default merge) Remote ref to test if a pull request is sent to the endpoint. See the GitHub developer manual for possible values for pull requests. (e.g. head) Something like: c[ "www" ][ "change_hook_dialects" ] = { "github" : { "secret" : config[ "private" ][ "gh_secret" ], "strict" : True, "pullrequest_ref" : "head" } } One of the best explanations I've read so far that motivates me to switch to refs/pull/head https://github.com/readthedocs/readthedocs.org/issues/6958#issuecomment-655203986 And we already have enforced that the developer must rebase before merging, at this point pull/head == pull/merge The Require branches to be up to date before merging box is checked on our branches.

Permalink

Daniel Black logged work - 2025-03-19 02:31

Time Spent:

1.5h

<No comment>

Daniel Black added a comment - 2025-03-19 02:31

manual test
$ git fetch https://github.com/MariaDB/server refs/pull/3826/merge
remote: Enumerating objects: 56, done.
remote: Counting objects: 100% (37/37), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 56 (delta 25), reused 23 (delta 23), pack-reused 19 (from 2)
Unpacking objects: 100% (56/56), 463.68 KiB \| 827.00 KiB/s, done.
From https://github.com/MariaDB/server
* branch refs/pull/3826/merge -> FETCH_HEAD
(base)

$ git show FETCH_HEAD
commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33
Merge: 839828e57fd ca561443a28
Author: Marko Mäkelä <marko.makela@mariadb.com>
Date: Mon Mar 17 08:52:31 2025 +0100

Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1

$ git fetch https://github.com/MariaDB/server refs/pull/3826/head
remote: Enumerating objects: 31, done.
remote: Counting objects: 100% (31/31), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 31 (delta 16), reused 31 (delta 16), pack-reused 0 (from 0)
Unpacking objects: 100% (31/31), 266.66 KiB \| 3.33 MiB/s, done.
From https://github.com/MariaDB/server
* branch refs/pull/3826/head -> FETCH_HEAD
(base)

$ git show FETCH_HEAD
commit 183498270e06bdbd37169c20f37bc184088daf69
Author: Marko Mäkelä <marko.makela@mariadb.com>
Date: Tue Mar 18 18:39:39 2025 +0200

So it looks like the merge reference is a stale cache - as it has been for a long time it seems https://github.com/orgs/community/discussions/21553

Looks fetching head is going to be needed, however our BB step might need to attempt the merge itself, as its this impact of the merge that we want protected branches to test.

Daniel Black added a comment - 2025-03-19 02:31 manual test $ git fetch https://github.com/MariaDB/server refs/pull/3826/merge remote: Enumerating objects: 56, done. remote: Counting objects: 100% (37/37), done. remote: Compressing objects: 100% (14/14), done. remote: Total 56 (delta 25), reused 23 (delta 23), pack-reused 19 (from 2) Unpacking objects: 100% (56/56), 463.68 KiB | 827.00 KiB/s, done. From https://github.com/MariaDB/server * branch refs/pull/3826/merge -> FETCH_HEAD (base) $ git show FETCH_HEAD commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 Merge: 839828e57fd ca561443a28 Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Mon Mar 17 08:52:31 2025 +0100 Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1 $ git fetch https://github.com/MariaDB/server refs/pull/3826/head remote: Enumerating objects: 31, done. remote: Counting objects: 100% (31/31), done. remote: Compressing objects: 100% (14/14), done. remote: Total 31 (delta 16), reused 31 (delta 16), pack-reused 0 (from 0) Unpacking objects: 100% (31/31), 266.66 KiB | 3.33 MiB/s, done. From https://github.com/MariaDB/server * branch refs/pull/3826/head -> FETCH_HEAD (base) $ git show FETCH_HEAD commit 183498270e06bdbd37169c20f37bc184088daf69 Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Tue Mar 18 18:39:39 2025 +0200 So it looks like the merge reference is a stale cache - as it has been for a long time it seems https://github.com/orgs/community/discussions/21553 Looks fetching head is going to be needed, however our BB step might need to attempt the merge itself, as its this impact of the merge that we want protected branches to test.

Daniel Black made changes - 2025-03-19 02:31

Worklog Id	134781 [ 134781 ]
Remaining Estimate		0d [ 0 ]
Time Spent		1.5h [ 5400 ]

Varzaru Razvan-Liviu added a comment - 2025-03-19 07:31 - edited

danblack
I do not entirely agree. I believe it is important for the developer's code version to be tested so that during development, the result of their work is not influenced by a pseudo-merge that could introduce bugs from integrating with what has been contributed to upstream in the meantime. Not testing this pseudo-merge helps the developer ensure that bugs encountered in CI are easily reproducible in their local environment.

By having the 'Require branches to be up to date before merging' option checked, integration testing will still take place within the pull request, as the developer will be required to bring in the latest changes from upstream to their branch, at which point refs/pull/head will be equal to refs/pull/merge.

The life cycle of a pull request in the case of refs/head would be:

The developer opens a pull request with their contribution.
The CI tests their code.
The code goes through iterations, during which the developer may or may not synchronize the branch with updates from upstream while fixing bugs.
When the code is ready, free of bugs, the developer is required by policy to bring in the changes from upstream in order to merge.
After the previous action, integration testing takes place, where if bugs appear, the code is further refined.

Varzaru Razvan-Liviu added a comment - 2025-03-19 07:31 - edited danblack I do not entirely agree. I believe it is important for the developer's code version to be tested so that during development, the result of their work is not influenced by a pseudo-merge that could introduce bugs from integrating with what has been contributed to upstream in the meantime. Not testing this pseudo-merge helps the developer ensure that bugs encountered in CI are easily reproducible in their local environment. By having the 'Require branches to be up to date before merging' option checked, integration testing will still take place within the pull request, as the developer will be required to bring in the latest changes from upstream to their branch, at which point refs/pull/head will be equal to refs/pull/merge. The life cycle of a pull request in the case of refs/head would be: The developer opens a pull request with their contribution. The CI tests their code. The code goes through iterations, during which the developer may or may not synchronize the branch with updates from upstream while fixing bugs. When the code is ready, free of bugs, the developer is required by policy to bring in the changes from upstream in order to merge. After the previous action, integration testing takes place, where if bugs appear, the code is further refined.

Daniel Black added a comment - 2025-03-19 21:44

Yep, I'm ok with that.

Daniel Black added a comment - 2025-03-19 21:44 Yep, I'm ok with that.

Vlad Radu made changes - 2025-03-24 09:17

Sprint

Sprint 6 (24.03.2025) [ 791 ]

Vlad Radu made changes - 2025-03-24 09:17

Rank

Ranked higher

Vlad Radu made changes - 2025-03-24 09:17

Assignee

Varzaru Razvan-Liviu [ JIRAUSER55297 ]

Vlad Radu made changes - 2025-03-24 09:18

Original Estimate

0.5d [ 14400 ]

Vlad Radu made changes - 2025-03-24 09:19

Remaining Estimate

0d [ 0 ]

0.5d [ 14400 ]

Varzaru Razvan-Liviu made changes - 2025-04-02 14:27

Affects Version/s

BB V1.04 [ 30120 ]

Varzaru Razvan-Liviu made changes - 2025-04-02 14:27

Fix Version/s

BB V1.05 [ 30132 ]

Varzaru Razvan-Liviu made changes - 2025-04-02 14:27

Status

Open [ 1 ]

In Progress [ 3 ]

Varzaru Razvan-Liviu added a comment - 2025-04-02 14:40

cvicentiu or danblack , review please?
https://github.com/MariaDB/buildbot/pull/734

Varzaru Razvan-Liviu added a comment - 2025-04-02 14:40 cvicentiu or danblack , review please? https://github.com/MariaDB/buildbot/pull/734

Varzaru Razvan-Liviu made changes - 2025-04-02 14:40

Status

In Progress [ 3 ]

In Review [ 10002 ]

Varzaru Razvan-Liviu made changes - 1 week ago

Status

In Review [ 10002 ]

In Testing [ 10301 ]

Vlad Radu made changes - 1 week ago

Sprint

Sprint 6 (24.03.2025) [ 791 ]

Sprint 6 (24.03.2025), Sprint 7 (07.04.2025) [ 791, 796 ]

Permalink

Varzaru Razvan-Liviu logged work - 2025-04-07 10:46

Time Spent:

0.25d

<No comment>

Varzaru Razvan-Liviu made changes - 6 days ago

Status

In Testing [ 10301 ]

Verified [ 10102 ]

Varzaru Razvan-Liviu made changes - 6 days ago

Worklog Id	135539 [ 135539 ]
Remaining Estimate	0.5d [ 14400 ]	0.25d [ 7200 ]
Time Spent	1.5h [ 5400 ]	3.5h [ 12600 ]

People

Assignee:: Varzaru Razvan-Liviu

Reporter:: Varzaru Razvan-Liviu

Votes:: 0 Vote for this issue

Watchers:: 2 Start watching this issue

Dates

Created:: 2025-03-18 16:34

Updated:: 1 week ago 10:46

Time Tracking

Estimated:

0.5d

Remaining:

0.25d

Logged:

3.5h

MariaDB Foundation Development

Details

Description

Attachments

Attachments

Activity

People

Dates

Time Tracking