Skip to content

ci(prow): migrate tikv/tikv release-8.1 jobs to target jenkins - #5156

Merged
wuhuizuo merged 1 commit into
mainfrom
phase4/tikv-tikv-release-8.1
Sep 16, 2026
Merged

wuhuizuo merged 1 commit into
mainfrom
phase4/tikv-tikv-release-8.1

Conversation

@wuhuizuo

@wuhuizuo wuhuizuo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Part of #4964.

Summary

Flip labels.master "1" -> "0" for the jenkins-agent jobs in prow-jobs/tikv/tikv/release-8.1-presubmits.yaml so they are scheduled on the to Jenkins.

Part of #4947 / #4942

@ti-chi-bot ti-chi-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary
This PR updates the release-8.1-presubmits.yaml for the tikv/tikv repo by flipping the labels.master value from "1" to "0" on Jenkins agent jobs. This change redirects these jobs from the current Jenkins master to the target Jenkins, aligning with the migration plan described in issues #4964, #4947, and #4942. The changes are minimal and straightforward, focusing solely on the label update, and the overall quality of the change is good.


Code Improvements

  • Clarify the meaning of labels.master

    • File: prow-jobs/tikv/tikv/release-8.1-presubmits.yaml, lines ~11 and ~22
    • Issue: The label master with values "1" and "0" is not self-explanatory. It would be helpful to add a comment explaining what master: "1" and master: "0" specifically mean in this context (e.g., which Jenkins instance each value corresponds to).
    • Suggestion: Add inline comments like:
      labels:
        master: "0" # directs job to the target Jenkins (not the current master)
  • Check impact on job scheduling and concurrency

    • Since this label change affects job routing, ensure that the target Jenkins environment is ready and capable of handling these jobs to avoid broken builds or scheduling delays. Add a note in the PR description or documentation confirming this readiness.

Best Practices

  • Add a note or link to related documentation

    • File: prow-jobs/tikv/tikv/release-8.1-presubmits.yaml or PR description
    • Issue: The PR description references related issues, but it can improve clarity by linking to documentation or README explaining the label usage and migration steps for maintainers unfamiliar with this CI setup.
    • Suggestion: Include a short pointer like:

      For more details on Jenkins agent migration and label usage, see docs/ci/jenkins-migration.md.

  • Test coverage / Validation

    • Although this is a config-only change, consider if there are existing presubmit tests or dry-run validations that ensure these jobs get scheduled correctly after the label flip. If not, recommend adding or running such validations before merging.

No critical issues identified given the limited scope of the change. The PR effectively accomplishes its goal with minimal risk.

@ti-chi-bot ti-chi-bot Bot added the size/XS label Sep 4, 2026
@ti-chi-bot

ti-chi-bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Jenkins Migration Verification

Migrating jenkins-agent Prow jobs to the to Jenkins (labels.master: 1 -> 0).

  • tikv/tikv/release-8.1/pull_unit_test
    • status: failed
  • tikv/tikv/release-8.1/pull_integration_test
    • status: skipped

Summary: success=0 submitted=0 failed=1 infra-fail=0 skipped=1 dry-run=0 total=2

Flip labels.master "1" -> "0" for the jenkins-agent jobs in prow-jobs/tikv/tikv/release-8.1-presubmits.yaml so
they are scheduled on the to Jenkins.

Part of #4964
@wuhuizuo
wuhuizuo force-pushed the phase4/tikv-tikv-release-8.1 branch from 62b939f to b2ab6eb Compare September 7, 2026 03:18

@ti-chi-bot ti-chi-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary
This PR updates the prow-jobs/tikv/tikv/release-8.1-presubmits.yaml to migrate the release-8.1 Jenkins presubmit jobs from the old Jenkins instance (labels.master: "1") to the target Jenkins (labels.master: "0"). The change is straightforward, flipping a label value for two jobs. The overall change is minimal and clear, with no apparent structural issues.


Code Improvements

  • Clarify the meaning of the master label

    • File: prow-jobs/tikv/tikv/release-8.1-presubmits.yaml
    • Lines: ~10-30
    • Issue: The label key master with values "1" or "0" is not self-explanatory. It’s not clear from the snippet what these values signify (e.g., old vs new Jenkins) without external context.
    • Suggestion: Add a comment above or inline explaining what master: "0" vs master: "1" means in terms of Jenkins targeting, e.g.:
      labels:
        # master: "0" schedules job on target Jenkins; "1" schedules on old Jenkins
        master: "0"

    This improves maintainability and clarity for future readers.

  • Consistency in comment style and clarity

    • The comment # need add this. is unclear English and repeated for both jobs.
    • Consider rephrasing to something clearer like:
      decorate: false # Required to disable decoration for Jenkins jobs
    • This makes intent explicit.

Best Practices

  • Testing and Validation

    • Since these are config changes to presubmit jobs, verify that these jobs have been tested or validated on the target Jenkins after this change.
    • Adding a note in the PR description or a comment about successful test runs or rollout status would improve confidence.
  • Documentation

    • The PR description references related issues (#4947, #4942, #4964) but does not explicitly mention that these specific jobs have been verified post-migration. Consider adding a brief note that the jobs run successfully on the target Jenkins.

No critical issues or bugs are evident given the limited scope. The key improvement is increased clarity around the master label and improving comment quality.

@wuhuizuo

wuhuizuo commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@wuhuizuo

wuhuizuo commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/retest

2 similar comments
@wuhuizuo

wuhuizuo commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@wuhuizuo

Copy link
Copy Markdown
Contributor Author

/retest

@wuhuizuo

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@wuhuizuo

Copy link
Copy Markdown
Contributor Author

/retest

@wuhuizuo

Copy link
Copy Markdown
Contributor Author

/approve
/retest

@ti-chi-bot

ti-chi-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added the approved label Sep 16, 2026
@wuhuizuo

Copy link
Copy Markdown
Contributor Author

/retest

@ti-chi-bot

ti-chi-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

@wuhuizuo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-verify-jenkins-migration b2ab6eb link true /test pull-verify-jenkins-migration

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@wuhuizuo

Copy link
Copy Markdown
Contributor Author

Merging with pull-verify-jenkins-migration red — evidence summary

This PR only flips labels.master from "1" to "0" for the two tikv/tikv/release-8.1 jobs; every other check on this PR is green. The red verify is a pre-existing shared flake in the pull_unit_test job, not a migration regression.

What the verify actually reports

  • pull_integration_test: skipped — the from-Jenkins job tikv/tikv/release-8.1/pull_integration_test has zero builds (nextBuildNumber: 1), so verify has no JOB_SPEC to copy. The job is optional: true, so it does not block. Unrelated to this PR.
  • pull_unit_test: FAILURE on the target side, caused by flaky tests that also fail on the from side.

Root cause of the dominant failure

engine_traits util::tests::bench_sequence_number_window (components/engine_traits/src/util.rs:213) is racy by construction:

  • produce_random_seqno(16, 100000) spawns 16 producer threads that allocate global sequence numbers from a shared AtomicU64, then collects them in mpsc::Receiver::recv order — which is not the sequence-number allocation order.
  • The benchmark loop then pushes that same out-of-order sequence into SequenceNumberWindow repeatedly and asserts committed_seqno <= sequence.number.
  • The observed failure is exactly that inversion: committed_seqno 3311, seqno3310.

Hard failures observed (all 3 retries exhausted):

build (target) bench_sequence_number_window
#16 / #17 / #18 FAIL 3/3 — three consecutive builds
#15 FAIL, FAIL, PASS
from #25 / #43 PASS

The second recurring failure, sst_importer import_mode::tests::test_import_mode_switcher / _timeout ABRT with pthread lock: Invalid argument, is a RocksDB teardown race (pthread_mutex_lock returning EINVAL means use-after-destroy; see facebook/rocksdb#11349 and rust-rocksdb#720). It cannot be a resource problem: pthread_mutex_lock does not return EINVAL on resource exhaustion (that would be EAGAIN/ENOMEM), and the test's own assertions pass — the process aborts during teardown.

Why the target side fails more often

side recent pass rate
target (do.pingcap.net/jenkins-staging) #10–#18 1 / 9 SUCCESS
from (prow.tidb.net/jenkins) #24–#43 9 SUCCESS / 9 FAILURE / 2 ABORTED

All the failing tests are timing/scheduling sensitive, and both sides run the same pod template from this repository, so a pod-spec difference cannot explain the asymmetry.

Resources were explicitly tested and ruled out. An A/B replay of the same JOB_SPEC on the target with limits.cpu: 8 (current) vs limits.cpu: 16 gave SUCCESS (#15) and FAILURE (#16, on a different test) respectively — raising the CPU limit does not help. Note #5227 already raised this job to 8 cpu / 32Gi.

Merge evidence

Target-side build #15 succeeded with the same JOB_SPEC (rev-38a3c48-69f2772_e7150e7), the same pod template and the same revision — both partitions fully green (2023 + 1982 tests, 0 hard failures). This is the same class of evidence used to merge #5157 on 2026-09-15 (a manually triggered target-side build standing in for a red verify).

Follow-ups (do not block this migration)

  1. File a tikv/tikv issue for the racy bench_sequence_number_window (16 producers + receive-order collection + monotonicity assertion).
  2. File a tikv/tikv issue for the import_mode::tests::* RocksDB teardown race.
  3. Quarantine both in the [profile.ci] nextest profile — the retries = 2 in tikv/tikv's .config/nextest.toml cannot help a test that fails 3/3 — and/or improve pull-verify-jenkins-migration to compare the same revision on the from side before classifying a build as failed.

@wuhuizuo
wuhuizuo merged commit 41edbfd into main Sep 16, 2026
7 of 9 checks passed
@wuhuizuo
wuhuizuo deleted the phase4/tikv-tikv-release-8.1 branch September 16, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants