Skip to content

ddl, ingest: improve test coverage for checkpoint validation#65421

Merged
ti-chi-bot[bot] merged 6 commits into
pingcap:masterfrom
wjhuang2016:add-index-test-coverage
Jan 13, 2026
Merged

ddl, ingest: improve test coverage for checkpoint validation#65421
ti-chi-bot[bot] merged 6 commits into
pingcap:masterfrom
wjhuang2016:add-index-test-coverage

Conversation

@wjhuang2016
Copy link
Copy Markdown
Member

@wjhuang2016 wjhuang2016 commented Jan 5, 2026

What problem does this PR solve?

Issue Number: close #65420

Problem Summary:
Improve add index ingest test coverage for checkpoint validation and empty partitions, and fix realtikvtest reliability by using real backend failpoints, enforcing concurrent ingest overlap, and canceling after ingest starts.

What changed and how does it work?

  • Add mock-backend tests for instance addr validation, checkpoint physical_id validation via reorg_meta JSON, and empty-partition iteration via afterUpdatePartitionReorgInfo
  • Refine tests to prevent global state leaks and improve determinism: use testfailpoint.EnableCall for auto-disable, restore global flags/config, and tighten cancel assertions
  • Fix realtikvtest concurrency/cancel coverage using beforeBackendIngest barrier and atomic jobID, and parse physical_id as int64 for empty-partition checks

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Run: go test -v -run 'TestCheckpointInstanceAddrValidation|TestCheckpointPhysicalIDValidation|TestAddIndexWithEmptyPartitions' --tags=intest ./pkg/ddl/ingest
Run: go test -v -run 'TestIngestConcurrentJobCleanupRace|TestIngestCancelCleanupOrder' --tags=intest ./tests/realtikvtest/addindextest3/...

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 5, 2026
@wjhuang2016 wjhuang2016 force-pushed the add-index-test-coverage branch from 6476c6d to b6717f7 Compare January 5, 2026 14:46
Add test cases for the following previously uncovered scenarios:

1. TestIngestOwnerTransferEmptyPartition (pingcap#44265): Tests owner transfer
   with empty partitions ensures checkpoint contains partition ID.

2. TestIngestPartitionCheckpointRecovery (pingcap#43997/pingcap#44024): Tests that
   checkpoint correctly saves partition info for recovery.

3. TestIngestConcurrentJobCleanupRace (pingcap#44137/pingcap#44140): Tests parallel
   add index jobs don't cause panic from cleanup race.

4. TestIngestCancelCleanupOrder (pingcap#43323/pingcap#43326): Tests cancel during
   execution doesn't cause nil pointer panic.

5. TestIngestGCSafepointBlocking (pingcap#40074/pingcap#40081): Tests add index
   uses correct TS and blocks GC safepoint advancement.

6. TestCheckpointInstanceAddrValidation (pingcap#43983/pingcap#43957): Tests
   checkpoint instance address validation works correctly.

7. TestCheckpointPhysicalIDValidation: Tests checkpoint physical
   table ID validation during recovery.
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.7759%. Comparing base (6b3f20a) to head (ffaa17f).
⚠️ Report is 44 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #65421        +/-   ##
================================================
+ Coverage   78.4730%   78.7759%   +0.3028%     
================================================
  Files          1938       1917        -21     
  Lines        534229     543576      +9347     
================================================
+ Hits         419226     428207      +8981     
- Misses       113483     113794       +311     
- Partials       1520       1575        +55     
Flag Coverage Δ
integration 47.4775% <ø> (-0.6946%) ⬇️
unit 76.8225% <ø> (+0.3390%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 57.5552% <ø> (+0.9360%) ⬆️
parser ∅ <ø> (∅)
br 59.9285% <ø> (-4.9601%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add tests to verify checkpoint validation in add index ingest mode:

- TestCheckpointInstanceAddrValidation: Verify instance address uses
  unique identifier (AdvertiseAddress + TempDir) instead of just
  host:port, covering pingcap#43983 and pingcap#43957

- TestCheckpointPhysicalIDValidation: Verify checkpoint physical_id
  is always a valid partition ID by parsing from reorg_meta JSON

- TestAddIndexWithEmptyPartitions: Verify all partitions (including
  empty ones) are correctly iterated during add index, using
  afterUpdatePartitionReorgInfo failpoint to capture each partition
  switch, covering pingcap#44265

Also clean up redundant/slow tests in realtikvtest:
- Remove TestIngestOwnerTransferEmptyPartition (51s, too slow)
- Remove TestIngestPartitionCheckpointRecovery (redundant)
- Simplify TestIngestCancelCleanupOrder

Issue Number: close pingcap#65420
@wjhuang2016 wjhuang2016 force-pushed the add-index-test-coverage branch from a4bc6b2 to fea2c99 Compare January 5, 2026 16:35
@ti-chi-bot ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 6, 2026
Comment thread tests/realtikvtest/addindextest3/ingest_test.go Outdated
@hawkingrei
Copy link
Copy Markdown
Member

/retest

@YangKeao
Copy link
Copy Markdown
Member

/retest

4 similar comments
@YangKeao
Copy link
Copy Markdown
Member

/retest

@YangKeao
Copy link
Copy Markdown
Member

/retest

@YangKeao
Copy link
Copy Markdown
Member

/retest

@YangKeao
Copy link
Copy Markdown
Member

/retest

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jan 13, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, joechenrh

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 lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 13, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jan 13, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-01-06 07:34:48.231167815 +0000 UTC m=+687644.049476247: ☑️ agreed by D3Hunter.
  • 2026-01-13 07:56:56.844666157 +0000 UTC m=+344260.906531065: ☑️ agreed by joechenrh.

@YangKeao
Copy link
Copy Markdown
Member

/retest

2 similar comments
@YangKeao
Copy link
Copy Markdown
Member

/retest

@YangKeao
Copy link
Copy Markdown
Member

/retest

@ti-chi-bot ti-chi-bot Bot merged commit de74b88 into pingcap:master Jan 13, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ddl, ingest: improve test coverage for checkpoint validation

5 participants