ddl: use static contexts in `NewReorgCopContext` #55823

lcwangchao · 2024-09-03T09:40:44Z

What problem does this PR solve?

Issue Number: ref #53388

What changed and how does it work?

This PR:

Introduces new functions newReorgExprCtxWithReorgMeta and newReorgDistSQLCtxWithReorgMeta to build ExprContext and DistSQLContext for DDL reorg.
Use above methods to construct static contexts in NewReorgCopContext

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 · 2024-09-03T09:40:59Z

Hi @lcwangchao. 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.

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.

lcwangchao · 2024-09-03T10:00:36Z

pkg/util/mock/context.go

@@ -272,6 +272,7 @@ func (c *Context) GetDistSQLCtx() *distsqlctx.DistSQLContext {
 		TiFlashMaxBytesBeforeExternalSort:    vars.TiFlashMaxBytesBeforeExternalSort,
 		TiFlashMaxQueryMemoryPerNode:         vars.TiFlashMaxQueryMemoryPerNode,
 		TiFlashQuerySpillRatio:               vars.TiFlashQuerySpillRatio,
+		ResourceGroupName:                    sc.ResourceGroupName,


It seems ResourceGroupName is lost in the previous implementation. PTAL @YangKeao

lcwangchao · 2024-09-03T10:01:27Z

pkg/util/mock/context.go

@@ -641,8 +642,8 @@ func NewContext() *Context {
 	vars.MemTracker.SetBytesLimit(-1)
 	vars.DiskTracker.SetBytesLimit(-1)
 	vars.StmtCtx.MemTracker, vars.StmtCtx.DiskTracker = memory.NewTracker(-1, -1), disk.NewTracker(-1, -1)
-	vars.MemTracker.AttachTo(vars.MemTracker)
-	vars.DiskTracker.AttachTo(vars.DiskTracker)
+	vars.StmtCtx.MemTracker.AttachTo(vars.MemTracker)


After discussing with @XuHuaiyu , we should use vars.StmtCtx.MemTracker here

codecov · 2024-09-03T10:18:56Z

Codecov Report

Attention: Patch coverage is 85.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 57.5838%. Comparing base (eb19c10) to head (a400bfb).
Report is 19 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #55823         +/-   ##
=================================================
- Coverage   72.8613%   57.5838%   -15.2776%     
=================================================
  Files          1595       1753        +158     
  Lines        449065     647102     +198037     
=================================================
+ Hits         327195     372626      +45431     
- Misses       101574     248858     +147284     
- Partials      20296      25618       +5322

Flag	Coverage Δ
integration	`40.6081% <77.2727%> (?)`
unit	`72.5753% <85.3333%> (+0.6376%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.9567% <ø> (+0.0266%)`	⬆️
parser	`∅ <ø> (∅)`
br	`64.0922% <ø> (+18.7166%)`	⬆️

lcwangchao · 2024-09-04T03:21:42Z

/retest

tiprow · 2024-09-04T03:22:04Z

@lcwangchao: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lcwangchao · 2024-09-04T03:57:22Z

/retest

tiprow · 2024-09-04T03:57:43Z

@lcwangchao: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lcwangchao · 2024-09-04T08:11:44Z

/retest

tiprow · 2024-09-04T08:12:07Z

@lcwangchao: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

YangKeao

LGTM

ti-chi-bot · 2024-09-05T01:59:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tangenta, YangKeao

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [YangKeao,tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]

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 · 2024-09-05T01:59:05Z

[LGTM Timeline notifier]

Timeline:

2024-09-04 09:51:39.201997945 +0000 UTC m=+438023.720050867: ☑️ agreed by tangenta.
2024-09-05 01:59:04.287132378 +0000 UTC m=+496068.805185303: ☑️ agreed by YangKeao.

lcwangchao · 2024-09-05T02:21:41Z

/retest

tiprow · 2024-09-05T02:22:03Z

@lcwangchao: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

* origin/master: (33 commits) build(deps): bump github.com/prometheus/common from 0.55.0 to 0.57.0 (pingcap#55762) build(deps): bump golang.org/x/sys from 0.24.0 to 0.25.0 (pingcap#55894) planner: fix incorrect estRows with global index and json column (pingcap#55842) ddl, stat: switch to new struct for add/truncate/drop partition (pingcap#55893) planner: hide instance plan cache eviction log if no plan is evicted (pingcap#55918) expression: support tidb encode key function (pingcap#51678) planner: fix incorrect maintenance of `handleColHelper` for recursive CTE (pingcap#55732) executor: some code refine of hash join v2 (pingcap#55887) infoschema, meta: fix wrong auto id after `rename table` (pingcap#55847) ddl/ingest: set `minCommitTS` when detect remote duplicate keys (pingcap#55588) planner: move index advisor into the kernel (pingcap#55874) ddl, stats: switch to new struct for create/truncate table (pingcap#55811) executor: avoid new small objects in probe stage of hash join v2 (pingcap#55855) *: Add tidbCPU/tikvCPU into system tables (pingcap#55455) ddl: use static contexts in `NewReorgCopContext` (pingcap#55823) executor: fix index out of range bug in hash join v2 (pingcap#55864) executor: record index usage for the clustered primary keys (pingcap#55602) domain: load all non-public tables into infoschema (pingcap#55142) test: fix unstable TestShowViewPriv (pingcap#55868) ttl: add `varbinary` case for `TestSplitTTLScanRangesWithBytes` (pingcap#55863) ...

ti-chi-bot bot added 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 Sep 3, 2024

lcwangchao mentioned this pull request Sep 3, 2024

remove mock.Context usage in production code #53388

Closed

lcwangchao force-pushed the copctxstatic branch 2 times, most recently from 5ab267f to ba40b05 Compare September 3, 2024 09:58

lcwangchao commented Sep 3, 2024

View reviewed changes

lcwangchao force-pushed the copctxstatic branch 2 times, most recently from 441d41f to b0a35ab Compare September 3, 2024 10:05

lcwangchao requested review from tangenta and YangKeao September 3, 2024 10:40

ddl: use static contexts in NewReorgCopContext

205df25

lcwangchao force-pushed the copctxstatic branch from b0a35ab to 205df25 Compare September 3, 2024 13:37

lcwangchao added 2 commits September 4, 2024 11:58

Merge branch 'master' into copctxstatic

69ec9aa

Merge branch 'master' into copctxstatic

a400bfb

tangenta approved these changes Sep 4, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 4, 2024

YangKeao approved these changes Sep 5, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 5, 2024

ti-chi-bot bot merged commit edbec22 into pingcap:master Sep 5, 2024
25 checks passed

lcwangchao deleted the copctxstatic branch September 5, 2024 06:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: use static contexts in `NewReorgCopContext` #55823

ddl: use static contexts in `NewReorgCopContext` #55823

lcwangchao commented Sep 3, 2024 •

edited

Loading

tiprow bot commented Sep 3, 2024

lcwangchao Sep 3, 2024

lcwangchao Sep 3, 2024

codecov bot commented Sep 3, 2024 •

edited

Loading

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

YangKeao left a comment

ti-chi-bot bot commented Sep 5, 2024

ti-chi-bot bot commented Sep 5, 2024

lcwangchao commented Sep 5, 2024

tiprow bot commented Sep 5, 2024

ddl: use static contexts in NewReorgCopContext #55823

ddl: use static contexts in NewReorgCopContext #55823

Conversation

lcwangchao commented Sep 3, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Sep 3, 2024

lcwangchao Sep 3, 2024

Choose a reason for hiding this comment

lcwangchao Sep 3, 2024

Choose a reason for hiding this comment

codecov bot commented Sep 3, 2024 • edited Loading

Codecov Report

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

lcwangchao commented Sep 4, 2024

tiprow bot commented Sep 4, 2024

YangKeao left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 5, 2024

ti-chi-bot bot commented Sep 5, 2024

[LGTM Timeline notifier]

lcwangchao commented Sep 5, 2024

tiprow bot commented Sep 5, 2024

ddl: use static contexts in `NewReorgCopContext` #55823

ddl: use static contexts in `NewReorgCopContext` #55823

lcwangchao commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 3, 2024 •

edited

Loading