Skip to content

[fix](build) Run Performance only on explicit comment requests - #70486

Merged
hello-stephen merged 1 commit into
apache:masterfrom
hello-stephen:codex/teamcity-performance-explicit-20261112
Nov 12, 2026
Merged

[fix](build) Run Performance only on explicit comment requests#70486
hello-stephen merged 1 commit into
apache:masterfrom
hello-stephen:codex/teamcity-performance-explicit-20261112

Conversation

@hello-stephen

@hello-stephen hello-stephen commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Background

The comment workflow routes run buildall to the applicable TeamCity pipelines. Performance is optional and is also available through the explicit run performance command on master.

What problem does this PR solve?

Issue Number: None
Related PR: #70459 (introduced the current comment routing)
PR Type: Bugfix

Problem Summary:

run buildall currently includes Performance. Its temporary validation configuration rejects general PR submissions, causing the comment workflow to fail after the other pipelines have already been queued. Make Performance explicit-only so aggregate runs do not attempt to submit it.

Details

  • Frequency: Master run buildall requests with Performance-related file changes.
  • Root cause: The Performance step condition includes both performance and buildall.
  • Why existing tests missed it: Existing tests cover parsing and per-file selection, but do not execute the GitHub Actions step condition for aggregate requests.
  • Fix: Change the single step condition to require COMMENT_TRIGGER_TYPE == 'performance'.
  • Other paths and branches: Retain the positive master-only condition, file selection, repeat count, authentication and skip buildall. Other pipeline steps are unchanged.
  • Behavior change: run buildall no longer triggers Performance. run performance remains its explicit command, subject to the existing branch and file checks.

Testing

  • actionlint .github/workflows/comment-to-trigger-teamcity.yml passed.
  • All 26 existing shell/workflow tests passed.
  • git diff --check passed.
  • Confirmed the diff changes only the Performance step condition. Live comment behavior can be verified after merge into the default branch.

Release note

None.

Check List (For Author)

  • Behavior changed: Yes (see Details)
  • Does this need documentation: No
  • Pre-submission status: Partially verified: local checks passed; post-merge comment execution remains pending.

AI Disclosure

  • Tool: Codex
  • Generated: One workflow condition and this description.
  • Human review: Pending.

@hello-stephen
[fix](build) Run Performance only on explicit comment requests
3b34140 
### Background

The comment workflow routes `run buildall` to the applicable TeamCity pipelines. Performance is optional and is also available through the explicit `run performance` command on master.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70459 (introduced the current comment routing)
PR Type: Bugfix

Problem Summary:

`run buildall` currently includes Performance. Its temporary validation configuration rejects general PR submissions, causing the comment workflow to fail after the other pipelines have already been queued. Make Performance explicit-only so aggregate runs do not attempt to submit it.

### Details

- **Frequency:** Master `run buildall` requests with Performance-related file changes.
- **Root cause:** The Performance step condition includes both `performance` and `buildall`.
- **Why existing tests missed it:** Existing tests cover parsing and per-file selection, but do not execute the GitHub Actions step condition for aggregate requests.
- **Fix:** Change the single step condition to require `COMMENT_TRIGGER_TYPE == 'performance'`.
- **Other paths and branches:** Retain the positive master-only condition, file selection, repeat count, authentication and `skip buildall`. Other pipeline steps are unchanged.
- **Behavior change:** `run buildall` no longer triggers Performance. `run performance` remains its explicit command, subject to the existing branch and file checks.

### Testing

- `actionlint .github/workflows/comment-to-trigger-teamcity.yml` passed.
- All 26 existing shell/workflow tests passed.
- `git diff --check` passed.
- Confirmed the diff changes only the Performance step condition. Live comment behavior can be verified after merge into the default branch.

### Release note

None.

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Partially verified: local checks passed; post-merge comment execution remains pending.

### AI Disclosure

- Tool: Codex
- Generated: One workflow condition and this description.
- Human review: Pending.
@hello-stephen
hello-stephen merged commit e125ea8 into apache:master Nov 12, 2026
32 of 33 checks passed
@hello-stephen
hello-stephen deleted the codex/teamcity-performance-explicit-20261112 branch November 12, 2026 05:30
@hello-stephen hello-stephen mentioned this pull request Nov 12, 2026
Merged
hello-stephen added a commit to hello-stephen/doris that referenced this pull request Nov 12, 2026
@hello-stephen
[fix](build) Resolve regression artifacts in TeamCity preflight
ac8b105 
### Background

PR comments select TeamCity jobs. Compile publishes a binary/case package pair and automatically dispatches P0, CloudP0, VaultP0, External and NonConcurrent with an exact artifact binding.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70486 (superseded Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Direct regression requests currently fail artifact resolution against the default `UNBOUND` build number before their first runner can execute. Keep the comment entry lightweight: submit only PR number, requested commit and target branch. Move Compile lookup, download and verification into the first TeamCity step; missing or invalid packages fail that build without starting another Compile. `run buildall` selects FEUT, BEUT, CloudUT, Compile and master-only Performance, leaving regression fanout to Compile.

### Details

- **Change map:** The comment helper changes by only a few lines. A shared TeamCity-side preflight handles producer selection, the existing 43-parameter consumer contract and verified downloads; its tests exercise failure and publication boundaries. Workflow conditions and a short installation guide complete the integration.
- **Frequency:** Direct comments for any of the five regression consumers with no artifact producer supplied.
- **Root cause:** Static artifact dependencies resolve before build steps, while direct requests do not have a Compile build number. Looking up artifacts in a later runner cannot repair that startup failure.
- **Why existing tests missed it:** They tested comment enqueue payloads without exercising TeamCity artifact resolution or runner-side preparation.
- **Fix:** Remove the static Compile artifact dependency and run the trusted preflight at the start of each consumer's first shell step. Use TeamCity's temporary build credentials and actual consumer revision to select an existing finished successful Compile. Validate PR/head/target, producer, source integrity, expiry and package presence; download binary, cases and tools and verify byte counts/SHA256. Export bindings to the current shell and subsequent steps only after all downloads verify. No new personal token or Compile trigger is introduced.
- **Other paths and branches:** All five regression consumers use the same preflight. Automatic fanout retains and validates its exact producer and pins instead of selecting a newer Compile. Performance remains independent and master-only. Existing source-head guards, rootless cluster execution, case selection and cleanup continue after preflight.
- **Behavior change:** Explicit regression comments always reach the pipeline, even if file filters would otherwise skip them. Missing packages now produce a failed TeamCity preflight step. Independent job/Compile file filters and `skip buildall` remain intact.

### Testing

- `python3 -m unittest discover -s regression-test/pipeline/common/tests -p 'test_teamcity*.py' -q`: 39 tests passed locally. Coverage includes command/branch selection, lightweight enqueue payloads, all five artifact consumers, missing/wrong/expired/incomplete packages, exact automatic-fanout pins, pagination, actual/requested revision mismatches, download hashes, failed-download cleanup and delayed parameter publication.
- `actionlint .github/workflows/comment-to-trigger-teamcity.yml`, `shellcheck -S warning regression-test/pipeline/common/teamcity-utils.sh`, Python syntax compilation and `git diff --check` passed.
- On an actual ordinary-user Agent, native build credentials successfully read Compile metadata and artifacts; the new resolver retained the current build's exact producer and verified all 43 bindings without downloading or disturbing its workload.
- The first-step integration and removal of the obsolete static artifact dependency were applied and read back on all five TeamCity configurations. Other steps and configuration fields were preserved; existing builds were not canceled or restarted.
- One External validation was queued with only PR, commit and target branch, and no Compile/artifact dependency. Its runtime preflight result is pending; full regression success is not claimed. The GitHub comment workflow itself takes effect after merge into the trusted default branch.

### Notes for Reviewers

`teamcity_artifacts.py` is trusted TeamCity runner code, not code executed from the PR checkout. See `regression-test/pipeline/common/teamcity-artifacts.md` for installation and the required dependency change. The helper uses read-only REST requests, bounded metadata responses, streaming downloads and no redirects carrying credentials.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: Yes, the integration guide is included.
- Pre-submission status: Partially verified: local tests, native-auth resolver validation and configuration readback passed; runtime canary and post-merge comment validation remain.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests, integration guide and this description.
- Human review: Pending maintainer review.
hello-stephen added a commit to hello-stephen/doris that referenced this pull request Nov 12, 2026
@hello-stephen
[fix](build) Use native TeamCity artifact dependencies
f9c50f2 
### Background

PR comments submit TeamCity jobs through a shared shell helper. Compile publishes the binary/case packages and dispatches the five regression consumers.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70486 (updates the Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Direct regression comments left the artifact dependency pointing at the default `UNBOUND` build number. Keep comments simple: pass PR, commit and target branch, and let TeamCity select/download the artifacts. A first pipeline step checks the downloaded package identity before the existing preparation runs. Missing packages or a different commit fail the requested regression build without starting Compile.

### Details

- **Change map:** About 160 lines of pipeline binding code, small workflow/helper changes, about 140 test lines, and a short TeamCity integration guide. Most binding code maps the aliases already required by the five existing runners.
- **Frequency:** Direct regression requests with an unbound artifact dependency fail before running any steps.
- **Root cause:** Native artifact resolution happens before a runner can fill the producer build number.
- **Why existing tests missed it:** Trigger tests covered enqueueing but did not check native artifact selection and the consumer's package identity gate together.
- **Fix:** Use a native Compile artifact dependency filtered to the requested PR. The trusted first step checks the actual revision, manifest identity, producer success and existing automatic-fanout pins, then fills existing runner parameters. There is no custom artifact search, pagination or downloader. Existing package/checksum validation remains in the preparation steps. The server integration is documented in `teamcity-artifacts.md`.
- **Other paths and branches:** `run buildall` selects FEUT, BEUT, CloudUT, Compile and master-only Performance. P0, CloudP0, VaultP0, External and NonConcurrent run directly only for their explicit commands; Compile retains automatic fanout. Independent jobs retain their existing file filters.
- **Behavior change:** Direct regression commands reach TeamCity even if their file filter says unchanged. TeamCity fails when its selected Compile package is unavailable, unsuccessful or for another commit; it does not rebuild Compile or search older builds.

### Testing

- 34 local unit tests passed, covering trigger selection, existing enqueue checks, package PR/commit/producer identity, failed producers and immutable automatic-fanout bindings.
- `actionlint`, `shellcheck -S warning`, Python compilation and `git diff --check` passed.
- All five TeamCity configurations use the native dependency and plaintext binding step. No static Compile snapshot dependency was added.
- A direct External validation build was submitted with only PR/head/target parameters; live artifact-stage verification is in progress. Earlier custom-download prototypes failed on endpoint/authentication constraints and were removed.
- End-to-end execution through the comment workflow still requires merging this PR.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No user documentation; pipeline integration is included here.
- Pre-submission status: Partially verified: local checks passed; live artifact-stage validation and post-merge comment execution remain.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests, integration notes and this description.
- Human review: Pending maintainer review.
hello-stephen added a commit to hello-stephen/doris that referenced this pull request Nov 12, 2026
@hello-stephen
[fix](build) Read the resolved dependency from the current build
d6f93ea 
### Background

PR comments submit TeamCity jobs through a shared shell helper. Compile publishes the binary/case packages and dispatches the five regression consumers.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70486 (updates the Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Direct regression comments left the artifact dependency pointing at the default `UNBOUND` build number. Keep comments simple: pass PR, commit and target branch, and let TeamCity select/download the artifacts. A first pipeline step checks the downloaded package identity before the existing preparation runs. Missing packages or a different commit fail the requested regression build without starting Compile.

### Details

- **Change map:** About 160 lines of pipeline binding code, small workflow/helper changes, about 140 test lines, and a short TeamCity integration guide. Most binding code maps the aliases already required by the five existing runners.
- **Frequency:** Direct regression requests with an unbound artifact dependency fail before running any steps.
- **Root cause:** Native artifact resolution happens before a runner can fill the producer build number.
- **Why existing tests missed it:** Trigger tests covered enqueueing but did not check native artifact selection and the consumer's package identity gate together.
- **Fix:** Use a native Compile artifact dependency filtered to the requested PR. The trusted first step checks the actual revision, manifest identity, producer success and existing automatic-fanout pins, then fills existing runner parameters. There is no custom artifact search, pagination or downloader. Existing package/checksum validation remains in the preparation steps. The server integration is documented in `teamcity-artifacts.md`.
- **Other paths and branches:** `run buildall` selects FEUT, BEUT, CloudUT, Compile and master-only Performance. P0, CloudP0, VaultP0, External and NonConcurrent run directly only for their explicit commands; Compile retains automatic fanout. Independent jobs retain their existing file filters.
- **Behavior change:** Direct regression commands reach TeamCity even if their file filter says unchanged. TeamCity fails when its selected Compile package is unavailable, unsuccessful or for another commit; it does not rebuild Compile or search older builds.

### Testing

- 35 local unit tests passed, covering trigger selection, existing enqueue checks, package PR/commit/producer identity, failed producers and immutable automatic-fanout bindings.
- `actionlint`, `shellcheck -S warning`, Python compilation and `git diff --check` passed.
- All five TeamCity configurations use the native dependency and plaintext binding step. No static Compile snapshot dependency was added.
- A direct External validation build was submitted with only PR/head/target parameters; live artifact-stage verification is in progress. Earlier custom-download prototypes failed on endpoint/authentication constraints and were removed.
- End-to-end execution through the comment workflow still requires merging this PR.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No user documentation; pipeline integration is included here.
- Pre-submission status: Partially verified: local checks passed; live artifact-stage validation and post-merge comment execution remain.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests, integration notes and this description.
- Human review: Pending maintainer review.
hello-stephen added a commit to hello-stephen/doris that referenced this pull request Nov 12, 2026
@hello-stephen
[refactor](build) Remove unused repository artifact helper
20869c6 
### Background

PR comments select TeamCity jobs. Compile publishes the binary/case packages and automatically dispatches P0, CloudP0, VaultP0, External and NonConcurrent. Artifact selection and package validation belong to those TeamCity pipelines.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70486 (updates the Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Make `run buildall` select only FEUT, BEUT, CloudUT, Compile and master-only Performance. Explicit regression commands submit the requested lane with PR, commit and target branch, even when its file filter says unchanged, so TeamCity can check the corresponding Compile package and fail if it is unavailable. This PR contains only comment dispatch changes and their tests.

### Details

- **Frequency:** A direct regression command can be skipped whenever its file filter reports no relevant changes.
- **Root cause:** The comment helper used the same file-filter skip path for explicit regression reruns and independent jobs, bypassing the pipeline's package checks.
- **Why existing tests missed it:** Tests did not require explicit regression commands to enqueue when the file filter was false, or verify the full command/branch selection matrix.
- **Fix:** Give each regression workflow step an explicit-command condition, keep artifact handling in TeamCity, and pass the requested head alongside PR/target information. Preserve existing filters for independent jobs and Compile.
- **Other paths and branches:** Apply the same direct-command behavior to all five regression consumers. Performance participates in `buildall` only on master. Compile retains automatic regression fanout.
- **Behavior change:** Direct regression requests reach TeamCity instead of reporting an unrelated-file skip. Missing or mismatched packages fail there without triggering another Compile.

### Testing

- 28 local TeamCity helper tests passed, including command/branch selection and explicit requests with unchanged-file filters.
- `actionlint`, `shellcheck -S warning`, `bash -n` and `git diff --check` passed.
- Read-only inspection confirmed that all five TeamCity consumers execute inline package-validation code and do not reference a repository `teamcity_artifacts.py` file. The unused helper, its standalone tests and its installation notes have been removed from this PR.
- End-to-end execution through the updated comment workflow requires merging this PR.

### Notes for Reviewers

Native artifact dependencies and inline package validation are maintained in TeamCity. Their server configuration is separate from this PR; removing the unused repository files does not remove those checks.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Partially verified: local checks passed; post-merge comment workflow execution remains.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests and this description.
- Human review: Pending maintainer review.
hello-stephen added a commit to hello-stephen/doris that referenced this pull request Nov 12, 2026
@hello-stephen
[fix](build) Preserve file filters for explicit regression requests
94bdc53 
### Background

PR comments select TeamCity jobs through a shared shell helper and existing file filters. Compile publishes packages and automatically dispatches P0, CloudP0, VaultP0, External and NonConcurrent. Those pipelines handle artifact selection and validation.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#70486 (updates the Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Make `run buildall` select FEUT, BEUT, CloudUT, Compile and master-only Performance. Select regression jobs only for their explicit commands, leaving automatic regression fanout to Compile. Preserve the existing file filters for every command and pass the requested commit alongside PR and target-branch information.

### Details

- **Frequency:** The command-selection rules apply to every comment-triggered run.
- **Root cause:** Selection was split between workflow conditions and shell early exits: regression steps accepted `buildall` but immediately exited, while Performance was excluded from `buildall` entirely.
- **Why existing tests missed it:** Tests did not verify the complete command/target-branch selection matrix.
- **Fix:** Express job selection directly in workflow conditions and add the requested head to the existing trigger parameters. Keep artifact handling in TeamCity.
- **Other paths and branches:** All five regression consumers remain available through their explicit commands, subject to the original file filters. Performance runs only for master PRs. Compile retains automatic regression fanout.
- **Behavior change:** Master `run buildall` includes Performance under its existing file filter. Regression workflow steps are not selected by `buildall`. File-filter skip behavior is unchanged, including for direct regression commands.

### Testing

- 27 local TeamCity helper tests passed, covering command/branch selection, existing file-filter skips and submission parameters.
- `actionlint`, `shellcheck -S warning`, `bash -n` and `git diff --check` passed.
- End-to-end execution through the updated comment workflow requires merging this PR.

### Notes for Reviewers

Artifact dependencies and package validation are maintained separately in TeamCity. This PR changes only comment dispatch and its tests; it does not add a repository artifact helper or bypass file filters.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Partially verified: local checks passed; post-merge comment workflow execution remains.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests and this description.
- Human review: Pending maintainer review.
morningman pushed a commit that referenced this pull request Nov 12, 2026
@hello-stephen
[fix](build) Simplify TeamCity comment dispatch (#70515)
172120c 
### Background

PR comments select TeamCity jobs through a shared shell helper and
existing file filters. Compile publishes packages and automatically
dispatches P0, CloudP0, VaultP0, External and NonConcurrent. Those
pipelines handle artifact selection and validation.

### What problem does this PR solve?

Issue Number: None
Related PR: #70486 (updates the Performance trigger policy)
PR Type: Bugfix

Problem Summary:

Make `run buildall` select FEUT, BEUT, CloudUT, Compile and master-only
Performance. Select regression jobs only for their explicit commands,
leaving automatic regression fanout to Compile. Preserve the existing
file filters for every command and pass the requested commit alongside
PR and target-branch information.

### Details

- **Frequency:** The command-selection rules apply to every
comment-triggered run.
- **Root cause:** Selection was split between workflow conditions and
shell early exits: regression steps accepted `buildall` but immediately
exited, while Performance was excluded from `buildall` entirely.
- **Why existing tests missed it:** Tests did not verify the complete
command/target-branch selection matrix.
- **Fix:** Express job selection directly in workflow conditions and add
the requested head to the existing trigger parameters. Keep artifact
handling in TeamCity.
- **Other paths and branches:** All five regression consumers remain
available through their explicit commands, subject to the original file
filters. Performance runs only for master PRs. Compile retains automatic
regression fanout.
- **Behavior change:** Master `run buildall` includes Performance under
its existing file filter. Regression workflow steps are not selected by
`buildall`. File-filter skip behavior is unchanged, including for direct
regression commands.

### Testing

- 27 local TeamCity helper tests passed, covering command/branch
selection, existing file-filter skips and submission parameters.
- `actionlint`, `shellcheck -S warning`, `bash -n` and `git diff
--check` passed.
- End-to-end execution through the updated comment workflow requires
merging this PR.

### Notes for Reviewers

Artifact dependencies and package validation are maintained separately
in TeamCity. This PR changes only comment dispatch and its tests; it
does not add a repository artifact helper or bypass file filters.

### Release note

None

### Check List (For Author)

- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Partially verified: local checks passed;
post-merge comment workflow execution remains.

### AI Disclosure

- Tool: OpenAI Codex
- Generated: Code, tests and this description.
- Human review: Pending maintainer review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiguolei yiguolei yiguolei approved these changes
@morningman morningman Awaiting requested review from morningman morningman is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hello-stephen @yiguolei