planner: introduce external storage for plan replayer #65477

zeminzhou · 2026-01-08T06:03:57Z

What problem does this PR solve?

Issue Number: close #65478

Problem Summary:

What changed and how does it work?

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.

Side effects

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

Documentation

Release note

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

None

tiprow · 2026-01-08T06:04:14Z

Hi @zeminzhou. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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.

D3Hunter

/hold

need more discussion about the new config, and how it can be used

pkg/config/config.go

D3Hunter · 2026-01-08T06:27:27Z

pkg/external/external_storage.go

+
+var (
+	globalMu              sync.RWMutex
+	globalExternalStorage *externalStorage


this var can create when it's needed, don't use global var

There are many places where storage is used, but creating it frequently is too cumbersome.

no need to be global, you can still create a local var that's reused

codecov · 2026-01-08T06:30:29Z

Codecov Report

❌ Patch coverage is 76.36364% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.6529%. Comparing base (5da5f2a) to head (a37ba43).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #65477        +/-   ##
================================================
- Coverage   77.7956%   77.6529%   -0.1428%     
================================================
  Files          2000       1923        -77     
  Lines        545038     533274     -11764     
================================================
- Hits         424016     414103      -9913     
+ Misses       119360     119158       -202     
+ Partials       1662         13      -1649

Flag	Coverage Δ
integration	`41.5024% <ø> (-6.6682%)`	⬇️
unit	`76.7343% <76.3636%> (+0.3056%)`	⬆️

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

Components	Coverage Δ
dumpling	`56.7974% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.8675% <ø> (-12.1040%)`	⬇️

🚀 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.

hawkingrei · 2026-01-13T08:17:01Z

/ok-to-test

zeminzhou · 2026-01-21T09:44:12Z

/test pull-integration-ddl-test

tiprow · 2026-01-21T09:44:37Z

@zeminzhou: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow

/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test pull-integration-ddl-test

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.

zeminzhou · 2026-01-21T09:57:35Z

/test pull-unit-test-next-gen

tiprow · 2026-01-21T09:58:00Z

@zeminzhou: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow

/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test pull-unit-test-next-gen

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.

D3Hunter · 2026-01-23T10:58:39Z

pkg/planner/extstore/extstore.go

what's the point to wrap on top of object storage again, why not use it directly?

Two reasons:

This storage is used in many places, and creating it every time is too cumbersome.
We need to confirm the path at the beginning to prevent the path from changing during the process, causing inconsistencies between where files are written and where they are read.

if the URI is changed, the file store location should follow the change. keep using the old uri is not an expected behavior

and the uri will not be changed on the cloud, this can be controlled by us

This storage is used in many places, and creating it every time is too cumbersome.

you can write a method like this to encapsulate all the detail of storage creation

tidb/pkg/dxf/framework/handle/handle.go

Lines 392 to 402 in 28cce6a

func newObjStore(ctx context.Context, uri string, opts *storeapi.Options) (storeapi.Storage, error) {

storeBackend, err := objstore.ParseBackend(uri, nil)

if err != nil {

return nil, err

}

store, err := objstore.New(ctx, storeBackend, opts)

if err != nil {

return nil, err

}

return store, nil

}

D3Hunter · 2026-01-23T10:59:32Z

cmd/tidb-server/main.go

+	}
+	logutil.BgLogger().Info("initialize external storage", zap.String("path", path), zap.String("namespace", namespace))
+
+	err := external.CreateExternalStorage(path, namespace, nil)


you can create it on demand, no need to setup at tidb startup

zeminzhou · 2026-01-26T05:52:44Z

/test all

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

hawkingrei · 2026-01-27T08:48:02Z

pkg/external/external_storage.go

+func init() {
+	if tempDir := config.GetGlobalConfig().TempDir; tempDir != "" {
+		_ = CreateExternalStorage(tempDir, "", nil)
+	}
+}


This already has a default value at startup; the init function seems problematic. Value initialization should be placed in a unified location.

good catch! thanks

hawkingrei · 2026-01-27T08:49:23Z

/lgtm cancel

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

D3Hunter · 2026-01-28T03:26:47Z

pkg/planner/extstore/BUILD.bazel

if you insist to use this wrapper, please move it to where it's used, AKA, planner

and external is not a good term to describe what it does, maybe extstore, and then the files under it can also be renamed

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

cmd/tidb-server/main.go

ti-chi-bot · 2026-01-28T04:41:13Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~pkg/planner/OWNERS~~ [hawkingrei]

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 · 2026-01-28T04:41:17Z

[LGTM Timeline notifier]

Timeline:

2026-01-23 09:15:54.725314002 +0000 UTC m=+744582.339270858: ☑️ agreed by hawkingrei.
2026-01-27 08:49:33.203881768 +0000 UTC m=+1088600.817838614: ✖️🔁 reset by hawkingrei.
2026-01-28 04:35:01.49955795 +0000 UTC m=+1159729.113514816: ☑️ agreed by D3Hunter.
2026-01-28 04:41:16.204010638 +0000 UTC m=+1160103.817967493: ☑️ agreed by hawkingrei.

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

hawkingrei · 2026-01-28T05:34:09Z

/unhold

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked 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 8, 2026

ti-chi-bot bot removed do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Jan 8, 2026

D3Hunter reviewed Jan 8, 2026

View reviewed changes

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2026

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 13, 2026

zeminzhou force-pushed the new-external-storage branch 2 times, most recently from 88e480b to 92f4d33 Compare January 13, 2026 09:15

hawkingrei approved these changes Jan 23, 2026

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jan 23, 2026

D3Hunter reviewed Jan 23, 2026

View reviewed changes

zeminzhou added 7 commits January 27, 2026 10:07

introduce external storage

ede2f7f

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

add fileWriter

9d65010

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

make check

882da64

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

make check

8312e28

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

remove external_storage_path config

ac308d0

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

make bazel_prepare

7c2e3e9

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

fix ut

ea2ac6f

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

revert unnecessary changes

7018858

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

zeminzhou force-pushed the new-external-storage branch from 9f2cd2d to 7018858 Compare January 27, 2026 02:08

hawkingrei reviewed Jan 27, 2026

View reviewed changes

ti-chi-bot bot removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jan 27, 2026

hawkingrei self-requested a review January 27, 2026 08:50

fix comment

f962117

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

D3Hunter reviewed Jan 28, 2026

View reviewed changes

zeminzhou added 3 commits January 28, 2026 12:13

remove global storage

161b2fd

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

rename

ebec17f

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

fix build & move extstore to planner

8421684

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

ti-chi-bot bot added the sig/planner SIG: Planner label Jan 28, 2026

D3Hunter reviewed Jan 28, 2026

View reviewed changes

cmd/tidb-server/main.go Outdated Show resolved Hide resolved

Update cmd/tidb-server/main.go

bbfb86f

D3Hunter approved these changes Jan 28, 2026

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 28, 2026

D3Hunter changed the title *: introduce external storage planner: introduce external storage for plan replayer Jan 28, 2026

hawkingrei approved these changes Jan 28, 2026

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 28, 2026

zeminzhou added 2 commits January 28, 2026 12:51

fix ci

4e78eb6

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

Error -> Warn

a37ba43

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2026

ti-chi-bot bot merged commit a919687 into pingcap:master Jan 28, 2026
31 checks passed

	func newObjStore(ctx context.Context, uri string, opts *storeapi.Options) (storeapi.Storage, error) {
	storeBackend, err := objstore.ParseBackend(uri, nil)
	if err != nil {
	return nil, err
	}
	store, err := objstore.New(ctx, storeBackend, opts)
	if err != nil {
	return nil, err
	}
	return store, nil
	}

planner: introduce external storage for plan replayer #65477

planner: introduce external storage for plan replayer #65477

Uh oh!

Conversation

zeminzhou commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Jan 8, 2026

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hawkingrei commented Jan 13, 2026

Uh oh!

zeminzhou commented Jan 21, 2026

Uh oh!

tiprow bot commented Jan 21, 2026

Uh oh!

zeminzhou commented Jan 21, 2026

Uh oh!

tiprow bot commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeminzhou commented Jan 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hawkingrei commented Jan 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ti-chi-bot bot commented Jan 28, 2026

Uh oh!

ti-chi-bot bot commented Jan 28, 2026

[LGTM Timeline notifier]

Uh oh!

hawkingrei commented Jan 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zeminzhou commented Jan 8, 2026 •

edited

Loading

codecov bot commented Jan 8, 2026 •

edited

Loading