You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "ym0506 (via GitHub)" <gi...@apache.org> on 2026/03/05 05:50:22 UTC
[PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
ym0506 opened a new pull request, #38352:
URL: https://github.com/apache/shardingsphere/pull/38352
### What this PR changes
- Explicitly sets `ref: ${{ github.ref }}` for all `actions/checkout` steps in `.github/workflows/e2e-sql.yml`.
### Why
Issue #38298 reports intermittent checkout failures in E2E-SQL:
- `fatal: remote error: upload-pack: not our ref <sha>`
A representative failure run is:
- https://github.com/apache/shardingsphere/actions/runs/22547669595
By checking out `github.ref` explicitly, PR-triggered jobs consistently target the PR merge ref (`refs/pull/<number>/merge`) and keep merged-with-base validation semantics.
### Scope
- Only workflow checkout configuration in E2E-SQL is changed.
- No test logic or matrix generation logic is changed.
Fixes #38298
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "ym0506 (via GitHub)" <gi...@apache.org>.
ym0506 commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4010034035
> > Thanks. I updated the checkout ref selection as suggested.
> > Current behavior in all 5 `actions/checkout` steps is now: `ref: ${{ github.event_name == 'pull_request' && github.ref || github.sha }}`
> > So:
> >
> > * `pull_request` keeps using the PR merge ref
> > * non-PR runs stay SHA-pinned and preserve the previous deterministic checkout behavior
> >
> > The patch is pushed in commit `2df1dcf9`.
>
> Hi @ym0506 Thanks for the contribution! However, I’ve analyzed the root cause and believe this specific change might not fully resolve the `not our ref` error. Here is a breakdown:
>
> ### 1. Analysis
> * **`github.ref`**: In a `pull_request` event, this defaults to `refs/pull/XXX/merge`. This is a "synthetic" commit created by GitHub to simulate the merge result.
>
> ### 2. Why the error persists
> The error `fatal: remote error: upload-pack: not our ref` occurs because these synthetic merge commits are volatile. If a Matrix job takes time to initialize or if the PR is updated/synchronized during the run, GitHub may garbage-collect the old merge ref, causing late-starting jobs to fail when they try to fetch it. Since `actions/checkout` already uses `github.ref` by default, explicitly declaring it doesn't prevent the ref from disappearing.
>
> ### Summary
> To ensure stability across Matrix jobs, we should consider pinning the checkout to the actual head of the PR branch instead of the unstable merge ref: `ref: ${{ github.event.pull_request.head.sha || github.sha }}`
>
> This ensures that all jobs in the matrix point to a persistent commit that won't be deleted during the workflow execution.
>
> But this is not preferred solution.
>
> Could you help to investigate `Solution 2. Take source code snapshot and reuse it in matrix jobs (preferred) ` in [#38298 (comment)](https://github.com/apache/shardingsphere/issues/38298#issuecomment-3983951285) ?
Thanks for the clarification. That makes sense.
I agree that the current change only addresses the determinism concern for non-PR events, and does not fundamentally eliminate the volatility of `refs/pull/<number>/merge` for matrix jobs.
I also agree that switching to `github.event.pull_request.head.sha` would likely improve stability, but it is probably not the preferred final direction because it weakens merged-with-base validation.
I’ll look into Solution 2 next:
- create a source snapshot in `prepare-e2e-artifacts`
- reuse that snapshot in the E2E-SQL matrix jobs instead of running `actions/checkout` again in each job
The current workflow already reuses Maven and Docker artifacts, so I’ll check whether a source snapshot can be integrated cleanly into the same flow. If it looks feasible, I’ll update this PR in that direction. If not, I’ll summarize the constraints and tradeoffs here before proposing a different approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "ym0506 (via GitHub)" <gi...@apache.org>.
ym0506 commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4010082173
I investigated Solution 2 and updated this PR in that direction.
What changed:
- `prepare-e2e-artifacts` still performs the single checkout, but now also creates a source snapshot with `git archive HEAD` and uploads it as an artifact
- `e2e-sql-smoke`, `detect-remaining-matrix`, and `e2e-sql-stage2` no longer run `actions/checkout`
- those downstream jobs now download and restore the same source snapshot before using the local workflow resources and running tests
- the earlier explicit `ref` overrides in `detect-and-generate-matrix` / `prepare-e2e-artifacts` were removed, so those steps are back to the default checkout behavior
The intent is to avoid repeated late fetches of the PR synthetic merge ref in downstream jobs, while keeping the workflow scope limited to E2E-SQL.
The new patch is pushed in commit `767f28bf`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "ym0506 (via GitHub)" <gi...@apache.org>.
ym0506 commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4011386704
Thanks, that makes sense. I updated the source snapshot creation to exclude `docs/` from the archive.
Current step:
```yaml
git archive --format=tar.gz --output=/tmp/e2e-sql-source-snapshot.tar.gz HEAD -- . ':(exclude)docs'
```
I also checked locally that the downstream-required files are still present after restore, including:
- `mvnw`
- `.github/workflows/resources/actions/setup-build-environment/action.yml`
- `test/e2e/sql/pom.xml`
And the local archive size dropped significantly in my check:
- full archive: about `109 MB`
- archive without `docs/`: about `5.1 MB`
The follow-up patch is pushed in commit `24a65e6c`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "ym0506 (via GitHub)" <gi...@apache.org>.
ym0506 commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4008895601
Thanks. I updated the checkout ref selection as suggested.
Current behavior in all 5 `actions/checkout` steps is now:
`ref: ${{ github.event_name == 'pull_request' && github.ref || github.sha }}`
So:
- `pull_request` keeps using the PR merge ref
- non-PR runs stay SHA-pinned and preserve the previous deterministic checkout behavior
The patch is pushed in commit `2df1dcf9`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4010684372
> I investigated Solution 2 and updated this PR in that direction.
>
> What changed:
>
> * `prepare-e2e-artifacts` still performs the single checkout, but now also creates a source snapshot with `git archive HEAD` and uploads it as an artifact
> * `e2e-sql-smoke`, `detect-remaining-matrix`, and `e2e-sql-stage2` no longer run `actions/checkout`
> * those downstream jobs now download and restore the same source snapshot before using the local workflow resources and running tests
> * the earlier explicit `ref` overrides in `detect-and-generate-matrix` / `prepare-e2e-artifacts` were removed, so those steps are back to the default checkout behavior
>
> The intent is to avoid repeated late fetches of the PR synthetic merge ref in downstream jobs, while keeping the workflow scope limited to E2E-SQL.
>
> The new patch is pushed in commit `767f28bf`.
LGTM
If `e2e-sql-source-snapshot` artifact size could be smaller, then it could be better.
I found the uploaded `e2e-sql-source-snapshot.tar.gz.zip` cost `109 MB`. And I downloaded it to local, `docs/` cost `121M` and it's not necessary for `mvn install` in e2e-sql, so could we exclude `docs/` on `git archive`? To save storage, upload/download artifact time, restore source snapshot time.
```
% du -h -d 1 e2e-sql-source-snapshot
8.7M e2e-sql-source-snapshot/database
7.2M e2e-sql-source-snapshot/infra
4.7M e2e-sql-source-snapshot/proxy
30M e2e-sql-source-snapshot/test
2.4M e2e-sql-source-snapshot/mode
48K e2e-sql-source-snapshot/jdbc-dialect
7.7M e2e-sql-source-snapshot/features
464K e2e-sql-source-snapshot/distribution
1.1M e2e-sql-source-snapshot/agent
121M e2e-sql-source-snapshot/docs
7.8M e2e-sql-source-snapshot/parser
1.1M e2e-sql-source-snapshot/jdbc
76K e2e-sql-source-snapshot/.codex
8.0K e2e-sql-source-snapshot/.mvn
472K e2e-sql-source-snapshot/examples
180K e2e-sql-source-snapshot/.github
8.9M e2e-sql-source-snapshot/kernel
20K e2e-sql-source-snapshot/.idea
124K e2e-sql-source-snapshot/src
202M e2e-sql-source-snapshot
```
How do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "sandynz (via GitHub)" <gi...@apache.org>.
sandynz commented on PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352#issuecomment-4009161543
> Thanks. I updated the checkout ref selection as suggested.
>
> Current behavior in all 5 `actions/checkout` steps is now: `ref: ${{ github.event_name == 'pull_request' && github.ref || github.sha }}`
>
> So:
>
> * `pull_request` keeps using the PR merge ref
> * non-PR runs stay SHA-pinned and preserve the previous deterministic checkout behavior
>
> The patch is pushed in commit `2df1dcf9`.
Hi @ym0506 Thanks for the contribution! However, I’ve analyzed the root cause and believe this specific change might not fully resolve the `not our ref` error. Here is a breakdown:
### 1. Analysis
* **`github.ref`**: In a `pull_request` event, this defaults to `refs/pull/XXX/merge`. This is a "synthetic" commit created by GitHub to simulate the merge result.
### 2. Why the error persists
The error `fatal: remote error: upload-pack: not our ref` occurs because these synthetic merge commits are volatile. If a Matrix job takes time to initialize or if the PR is updated/synchronized during the run, GitHub may garbage-collect the old merge ref, causing late-starting jobs to fail when they try to fetch it. Since `actions/checkout` already uses `github.ref` by default, explicitly declaring it doesn't prevent the ref from disappearing.
### Summary
To ensure stability across Matrix jobs, we should consider pinning the checkout to the actual head of the PR branch instead of the unstable merge ref:
`ref: ${{ github.event.pull_request.head.sha || github.sha }}`
This ensures that all jobs in the matrix point to a persistent commit that won't be deleted during the workflow execution.
But this is not preferred solution.
Could you help to investigate `Solution 2. Take source code snapshot and reuse it in matrix jobs (preferred) ` in https://github.com/apache/shardingsphere/issues/38298#issuecomment-3983951285 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix intermittent checkout ref failures in E2E-SQL workflow [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #38352:
URL: https://github.com/apache/shardingsphere/pull/38352
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org