github-actions[bot] commented on code in PR #66194:
URL: https://github.com/apache/doris/pull/66194#discussion_r3668753511
##########
.github/workflows/comment-to-trigger-teamcity.yml:
##########
@@ -29,8 +29,54 @@ permissions:
jobs:
check-comment-if-need-to-trigger-teamcity:
- # This job only runs for pull request comments, and comment body contains
'run'
- if: ${{ github.event.issue.pull_request &&
(contains(github.event.comment.body, 'run') ||
contains(github.event.comment.body, 'skip buildall') ||
contains(github.event.comment.body, 'skip check_coverage')) }}
+ # Keep this server-side filter aligned with the supported commands parsed
below.
+ # A generic "run" match also catches performance reports such as "Total
hot run time".
+ if: >-
+ ${{
+ github.event.issue.pull_request &&
+ (
+ contains(github.event.comment.body, 'run buildall') ||
+ contains(github.event.comment.body, 'run compile') ||
+ contains(github.event.comment.body, 'run beut') ||
+ contains(github.event.comment.body, 'run feut') ||
+ contains(github.event.comment.body, 'run cloudut') ||
+ contains(github.event.comment.body, 'run p0') ||
+ contains(github.event.comment.body, 'run p1') ||
+ contains(github.event.comment.body, 'run external') ||
+ contains(github.event.comment.body, 'run cloud_p0') ||
+ contains(github.event.comment.body, 'run cloud_p1') ||
+ contains(github.event.comment.body, 'run vault_p0') ||
+ contains(github.event.comment.body, 'run nonConcurrent') ||
+ contains(github.event.comment.body, 'run check_coverage') ||
+ contains(github.event.comment.body, 'run performance') ||
+ contains(github.event.comment.body, 'skip buildall') ||
+ contains(github.event.comment.body, 'skip check_coverage')
+ )
+ }}
+
+ # Serialize duplicate commands for one PR. The next run then sees the build
+ # queued by the previous run and can safely skip an identical PR revision.
Review Comment:
[P1] Coordinate `buildall` through the complete fan-out
The helper and GitHub lock cover only Compile even though this workflow
skips the downstream steps because Compile triggers them. This fails in both
directions: for PR 66196, Compile 1008441 finished while same-revision P0
1008463, External 1008464, Cloud P0 1008465, and NonConcurrent 1008467 were
still running, so another `buildall` starts a new root; while Compile is still
active, a direct `run p0` can queue P0 before Compile later enqueues the same
P0 entirely inside TeamCity, bypassing this helper and every GitHub group.
Please enforce exact-request coordination at the TeamCity fan-out boundary, or
route both direct and fan-out requests through one atomic coordinator for the
full descendant lifetime.
##########
regression-test/pipeline/common/teamcity-utils.sh:
##########
@@ -159,6 +159,64 @@ get_queue_build_of_pr() {
}
# get_queue_build_of_pr "$1" "$2"
+get_active_builds_of_revision() {
+ # Return active build IDs for the same PR, pipeline, and revision.
+ # Return 0 when a duplicate exists, 1 when none exists, and 2 when lookup
fails.
+ local PULL_REQUEST_NUM="${PULL_REQUEST_NUM:-$1}"
+ local COMMENT_TRIGGER_TYPE="${COMMENT_TRIGGER_TYPE:-$2}"
+ local COMMIT_ID_FROM_TRIGGER="${COMMIT_ID_FROM_TRIGGER:-$3}"
+ if [[ -z "${PULL_REQUEST_NUM}" ||
+ -z "${COMMENT_TRIGGER_TYPE}" ||
+ -z "${COMMIT_ID_FROM_TRIGGER}" ]]; then
+ echo "Usage: get_active_builds_of_revision PULL_REQUEST_NUM
COMMENT_TRIGGER_TYPE COMMIT_ID_FROM_TRIGGER" >&2
+ return 2
+ fi
+
+ local PIPELINE="${comment_to_pipeline[${COMMENT_TRIGGER_TYPE}]}"
+ local teamcity_rest_url="http://43.132.222.7:8111/app/rest";
+ local queue_response
+ local running_response
+ if ! queue_response=$(
+ curl -sSf -X GET \
+ -u OneMoreChance:OneMoreChance \
+ -H "Accept: application/json" \
+
"${teamcity_rest_url}/buildQueue?locator=buildType:(id:${PIPELINE})&fields=build(id,branchName,revisions(revision(version)))"
Review Comment:
[P2] Follow TeamCity pagination when looking for duplicates
TeamCity collection responses may be partial and advertise the next page
through `nextHref`, but this projection does not request that field and the
function performs only one request. Since the queue locator can filter by build
type but not by PR branch/revision, a busy pipeline can place the exact
duplicate on a later page; this returns “none”, the single-page cancel path
misses it too, and line 378 queues another copy. Please follow `nextHref` (for
queued and running collections) until a match is found or the collection is
exhausted, with a multi-page fixture.
##########
regression-test/pipeline/common/teamcity-utils.sh:
##########
@@ -159,6 +159,64 @@ get_queue_build_of_pr() {
}
# get_queue_build_of_pr "$1" "$2"
+get_active_builds_of_revision() {
+ # Return active build IDs for the same PR, pipeline, and revision.
+ # Return 0 when a duplicate exists, 1 when none exists, and 2 when lookup
fails.
+ local PULL_REQUEST_NUM="${PULL_REQUEST_NUM:-$1}"
+ local COMMENT_TRIGGER_TYPE="${COMMENT_TRIGGER_TYPE:-$2}"
+ local COMMIT_ID_FROM_TRIGGER="${COMMIT_ID_FROM_TRIGGER:-$3}"
+ if [[ -z "${PULL_REQUEST_NUM}" ||
+ -z "${COMMENT_TRIGGER_TYPE}" ||
+ -z "${COMMIT_ID_FROM_TRIGGER}" ]]; then
+ echo "Usage: get_active_builds_of_revision PULL_REQUEST_NUM
COMMENT_TRIGGER_TYPE COMMIT_ID_FROM_TRIGGER" >&2
+ return 2
+ fi
+
+ local PIPELINE="${comment_to_pipeline[${COMMENT_TRIGGER_TYPE}]}"
+ local teamcity_rest_url="http://43.132.222.7:8111/app/rest";
+ local queue_response
+ local running_response
+ if ! queue_response=$(
+ curl -sSf -X GET \
+ -u OneMoreChance:OneMoreChance \
+ -H "Accept: application/json" \
+
"${teamcity_rest_url}/buildQueue?locator=buildType:(id:${PIPELINE})&fields=build(id,branchName,revisions(revision(version)))"
+ ); then
+ echo "WARNING: failed to get queued builds for duplicate check" >&2
+ return 2
+ fi
+ if ! running_response=$(
+ curl -sSf -X GET \
+ -u OneMoreChance:OneMoreChance \
+ -H "Accept: application/json" \
+
"${teamcity_rest_url}/builds?locator=buildType:(id:${PIPELINE}),branch:(name:pull/${PULL_REQUEST_NUM}),running:true&fields=build(id,branchName,revisions(revision(version)))"
+ ); then
+ echo "WARNING: failed to get running builds for duplicate check" >&2
+ return 2
+ fi
+
+ local build_ids
+ if ! build_ids=$(
+ printf '%s\n%s\n' "${queue_response}" "${running_response}" |
+ jq -s -r \
+ --arg branch "pull/${PULL_REQUEST_NUM}" \
+ --arg revision "${COMMIT_ID_FROM_TRIGGER}" \
+ '.[] | .build[]? |
Review Comment:
[P1] Include repeat count in the duplicate identity
Repeat count changes the request: the parser preserves it and
`trigger_build` sends it as `env.repeat_times_from_trigger`, which the
regression pipelines use for their iteration count and timeout. This selector
compares only PR branch and revision, so while `run cloud_p0` is active, a
subsequent `run cloud_p0 3` on the same head is reported as a duplicate and the
requested three passes are never scheduled. Please normalize the empty count to
1 and compare the active build's repeat property as part of duplicate
equivalence, while keeping all requests for one PR/pipeline in the same
serialization domain.
##########
.github/workflows/comment-to-trigger-teamcity.yml:
##########
@@ -29,8 +29,54 @@ permissions:
jobs:
check-comment-if-need-to-trigger-teamcity:
- # This job only runs for pull request comments, and comment body contains
'run'
- if: ${{ github.event.issue.pull_request &&
(contains(github.event.comment.body, 'run') ||
contains(github.event.comment.body, 'skip buildall') ||
contains(github.event.comment.body, 'skip check_coverage')) }}
+ # Keep this server-side filter aligned with the supported commands parsed
below.
+ # A generic "run" match also catches performance reports such as "Total
hot run time".
+ if: >-
+ ${{
+ github.event.issue.pull_request &&
+ (
+ contains(github.event.comment.body, 'run buildall') ||
Review Comment:
[P1] Use one canonical command grammar before entering concurrency
This admission test is not equivalent to the parser below: it examines the
raw body with case-insensitive `contains`, while lines 91/179 normalize
whitespace and then shell-match case-sensitively. Thus `run p0` (or a line
break between the words) previously normalizes to valid `run p0` but is now
skipped; conversely `RUN P0` is admitted and assigned the `p0` group, then the
parser no-ops. Because the default concurrency queue keeps only one pending
job, that no-op can replace a legitimate pending `p0` request. Please make
admission, grouping, and dispatch use the same canonical command and preserve
distinct pending requests.
##########
regression-test/pipeline/common/teamcity-utils.sh:
##########
@@ -300,6 +358,21 @@ trigger_or_skip_build() {
fi
if [[ "${FILE_CHANGED:-"true"}" == "true" ]]; then
+ local duplicate_build_ids
+ local duplicate_lookup_status=0
+ duplicate_build_ids=$(
+ get_active_builds_of_revision \
+ "${PULL_REQUEST_NUM}" \
+ "${COMMENT_TRIGGER_TYPE}" \
+ "${COMMIT_ID_FROM_TRIGGER}"
+ ) || duplicate_lookup_status=$?
+ if [[ ${duplicate_lookup_status} -eq 0 ]]; then
+ echo "INFO: active build(s) ${duplicate_build_ids//$'\n'/,}
already exist for PR ${PULL_REQUEST_NUM}, pipeline ${COMMENT_TRIGGER_TYPE},
revision ${COMMIT_ID_FROM_TRIGGER}; skip duplicate trigger"
+ return 0
Review Comment:
[P1] Still clean up stale revisions before returning
`get_active_builds_of_revision` reports only IDs matching the requested
revision, so this return says nothing about other active builds for the same
PR/pipeline. If a current-revision build is queued while an older revision is
still running or queued (for example after an external trigger or a
failed/asynchronous cancellation), this branch bypasses both cleanup calls
below and leaves the stale build alive. That contradicts the stated
preservation of cancel-old-revision behavior. Please classify the full active
set, cancel every non-current build, and skip only the new trigger when a
current build remains.
##########
.github/workflows/comment-to-trigger-teamcity.yml:
##########
@@ -29,8 +29,54 @@ permissions:
jobs:
check-comment-if-need-to-trigger-teamcity:
- # This job only runs for pull request comments, and comment body contains
'run'
- if: ${{ github.event.issue.pull_request &&
(contains(github.event.comment.body, 'run') ||
contains(github.event.comment.body, 'skip buildall') ||
contains(github.event.comment.body, 'skip check_coverage')) }}
+ # Keep this server-side filter aligned with the supported commands parsed
below.
+ # A generic "run" match also catches performance reports such as "Total
hot run time".
+ if: >-
+ ${{
+ github.event.issue.pull_request &&
+ (
+ contains(github.event.comment.body, 'run buildall') ||
+ contains(github.event.comment.body, 'run compile') ||
+ contains(github.event.comment.body, 'run beut') ||
+ contains(github.event.comment.body, 'run feut') ||
+ contains(github.event.comment.body, 'run cloudut') ||
+ contains(github.event.comment.body, 'run p0') ||
+ contains(github.event.comment.body, 'run p1') ||
+ contains(github.event.comment.body, 'run external') ||
+ contains(github.event.comment.body, 'run cloud_p0') ||
+ contains(github.event.comment.body, 'run cloud_p1') ||
+ contains(github.event.comment.body, 'run vault_p0') ||
+ contains(github.event.comment.body, 'run nonConcurrent') ||
+ contains(github.event.comment.body, 'run check_coverage') ||
+ contains(github.event.comment.body, 'run performance') ||
+ contains(github.event.comment.body, 'skip buildall') ||
+ contains(github.event.comment.body, 'skip check_coverage')
+ )
+ }}
+
+ # Serialize duplicate commands for one PR. The next run then sees the build
+ # queued by the previous run and can safely skip an identical PR revision.
+ concurrency:
+ group: >-
+ comment-to-trigger-teamcity-${{ github.event.issue.number }}-${{
+ contains(github.event.comment.body, 'run buildall') && 'buildall' ||
Review Comment:
[P1] Serialize mutations of the same concrete pipeline
This key does not cover every run that executes the same check-then-trigger
operation. `run buildall` is placed in the `buildall` group but later calls the
helper for FEUT, BEUT, CloudUT, Compile, and Performance, while each direct
command uses a different group. Also, `run feut run beut` is grouped as `beut`
by this fixed precedence but the parser executes the first textual command,
`feut`. Two such jobs can both observe no active FEUT build and then both
enqueue it. Please lock by the concrete PR/pipeline mutation (or use a
PR-wide/TeamCity-side atomic lock) and derive the key from the same canonical
command used for dispatch.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
