*: don't handle live updates of column size #58596

winoros · 2024-12-28T16:23:12Z

What problem does this PR solve?

Issue Number: close #58595

Problem Summary:

What changed and how does it work?

Just as the issue said. We dropped the maintenance of the column size updates during various DML statements.

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

codecov · 2024-12-28T16:40:47Z

Codecov Report

Attention: Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.4767%. Comparing base (be6396c) to head (ad182af).
Report is 78 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58596        +/-   ##
================================================
+ Coverage   73.5326%   74.4767%   +0.9440%     
================================================
  Files          1680       1711        +31     
  Lines        464728     481843     +17115     
================================================
+ Hits         341727     358861     +17134     
+ Misses       102158     101381       -777     
- Partials      20843      21601       +758

Flag	Coverage Δ
integration	`46.3054% <44.4444%> (?)`
unit	`73.0329% <96.2963%> (+0.7591%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`45.7300% <ø> (-0.0385%)`	⬇️

ti-chi-bot · 2024-12-30T02:13:24Z

[LGTM Timeline notifier]

Timeline:

2024-12-29 01:13:24.15158243 +0000 UTC m=+226539.507586969: ☑️ agreed by lance6716.
2024-12-30 02:13:22.48680886 +0000 UTC m=+316537.842813398: ☑️ agreed by qw4990.

Rustin170506

/hold

Thanks!

Rustin170506 · 2024-12-30T03:56:49Z

pkg/lightning/backend/kv/context.go

-func (ctx *litTableMutateContext) UpdatePhysicalTableDelta(
-	physicalTableID int64, _ int64,
-	_ int64, cols variable.DeltaCols,
+func (*litTableMutateContext) UpdatePhysicalTableDelta(


Can we even delete this function? I believe we can delete the interface as well.

The interface MutateContext will be used in the normal write path. We cannot delete the MutateContext interface.
We can only let the litTableMutateContext not impl the interface StatisticsSupport and return a nil for its GetStatisticsSupport and add an intest.Assert(false) inside it.

pkg/table/table.go

time-and-fate · 2024-12-30T14:43:14Z

I think the changes in the latest commit are incorrect.

This reverts commit 3ca5e1c.

This reverts commit be227be.

winoros · 2024-12-30T18:01:28Z

some tests like TestReplaceConflictOneUniqueKey failed if we cleaned the litTableMutateContext.
Reverted them.

Rustin170506

Thanks!

Rustin170506 · 2024-12-31T09:56:38Z

pkg/lightning/backend/kv/context_test.go

-		require.Equal(t, map[int64]int64{1: 2, 3: 9, 4: 3}, tblCtx.GetColumnSize(123))
-		// test gets a non-existed table
-		require.Empty(t, tblCtx.GetColumnSize(456))
+		stats.UpdatePhysicalTableDelta(123, 5, 2)


Perhaps we can delete them at all.

ti-chi-bot · 2025-01-05T09:55:33Z

@winoros: 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-unit-test-ddlv1	`b63ca1d`	link	true	`/test pull-unit-test-ddlv1`

Full PR test history. Your PR dashboard.

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.

winoros · 2025-01-06T14:26:55Z

/unhold
/retest

yudongusa

@winoros @songrijie We need to make this change clearly stated in the document as well as communication with users.

ti-chi-bot · 2025-01-06T18:56:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, lance6716, qw4990, Rustin170506, yudongusa

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~~ [Rustin170506,cfzjywxk,lance6716,qw4990]
~~pkg/disttask/OWNERS~~ [lance6716]
~~pkg/executor/importer/OWNERS~~ [lance6716]
~~pkg/lightning/OWNERS~~ [lance6716]
~~pkg/planner/OWNERS~~ [Rustin170506,qw4990]
~~pkg/sessionctx/variable/OWNERS~~ [yudongusa]
~~pkg/statistics/OWNERS~~ [Rustin170506,qw4990]
~~pkg/table/OWNERS~~ [cfzjywxk]

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

…long-vector * commit '510d0037b18f258f505abc6cf13a8128563e9359': *: upgrade pd client to make sure tso client initiate successfully (#58752) ttl, test: scale TTL workers during the fault tests (#58750) planner: improve warning messages for unsupported HASH_JOIN hints (#58646) planner: prealloc the slices in the SplitCorColAccessCondFromFilters (#58785) ddl: supports non-unique global index (#58678) util/stmtsummary: add the network traffic related fields (#58101) var: enable `pd_enable_follower_handle_region` as default (#58385) statistics: refactor stats meta handling to use DeltaUpdate for multi-table support (#58657) parser: move 'model' to 'ast' pkg (#58704) statistics: add recover to protect background task (#58739) disttask: cancel subtask context if scheduled away (#58615) *: don't handle live updates of column size (#58596) *: fix a bug for default_authentication_plugin (2) (#58723) dupdetect: gRPC cancel should trigger retry (#58542) *: fix a bug for default_authentication_plugin (#57391) distsql: Fix backoff execution info inaccurate issue (#58707)

ti-chi-bot · 2025-01-13T09:13:37Z

In response to a cherrypick label: new pull request created to branch release-8.5: #58890.

close pingcap#58595

*: don't handle live updates of column size

b3a376f

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. sig/planner SIG: Planner labels Dec 28, 2024

lance6716 approved these changes Dec 29, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 29, 2024

fix static check and unit test

0edef39

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 29, 2024

fix tests

111a58d

winoros force-pushed the remove-cols-live-updates branch from 297fb21 to 111a58d Compare December 29, 2024 20:24

qw4990 approved these changes Dec 30, 2024

View reviewed changes

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

Rustin170506 reviewed Dec 30, 2024

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 Dec 30, 2024

time-and-fate reviewed Dec 30, 2024

View reviewed changes

pkg/table/table.go Show resolved Hide resolved

winoros added 2 commits December 31, 2024 01:15

address comment

be227be

address comment 2

1805653

winoros force-pushed the remove-cols-live-updates branch from b63ca1d to 1805653 Compare December 30, 2024 17:36

winoros added 3 commits December 31, 2024 01:45

fix lint

3ca5e1c

Revert "fix lint"

6da5f94

This reverts commit 3ca5e1c.

Revert "address comment"

92f2870

This reverts commit be227be.

fix test

e2693d3

Rustin170506 approved these changes Dec 31, 2024

View reviewed changes

cfzjywxk approved these changes Jan 2, 2025

View reviewed changes

remove unused struct

ad182af

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 6, 2025

yudongusa approved these changes Jan 6, 2025

View reviewed changes

ti-chi-bot bot added the approved label Jan 6, 2025

ti-chi-bot bot merged commit bf939fa into pingcap:master Jan 6, 2025
25 checks passed

Rustin170506 added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Jan 13, 2025

ti-chi-bot mentioned this pull request Jan 13, 2025

*: don't handle live updates of column size (#58596) #58890

Open

13 tasks

Rustin170506 pushed a commit to Rustin170506/tidb that referenced this pull request Jan 16, 2025

*: don't handle live updates of column size (pingcap#58596)

72823b0

close pingcap#58595

Rustin170506 pushed a commit to Rustin170506/tidb that referenced this pull request Jan 16, 2025

*: don't handle live updates of column size (pingcap#58596)

4c429a4

close pingcap#58595

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: don't handle live updates of column size #58596

*: don't handle live updates of column size #58596

winoros commented Dec 28, 2024

codecov bot commented Dec 28, 2024 •

edited

Loading

ti-chi-bot bot commented Dec 30, 2024

Rustin170506 left a comment

Rustin170506 Dec 30, 2024 •

edited

Loading

winoros Dec 30, 2024

time-and-fate commented Dec 30, 2024

winoros commented Dec 30, 2024

Rustin170506 left a comment

Rustin170506 Dec 31, 2024

ti-chi-bot bot commented Jan 5, 2025 •

edited

Loading

winoros commented Jan 6, 2025

yudongusa left a comment

ti-chi-bot bot commented Jan 6, 2025

ti-chi-bot commented Jan 13, 2025

*: don't handle live updates of column size #58596

*: don't handle live updates of column size #58596

Conversation

winoros commented Dec 28, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

codecov bot commented Dec 28, 2024 • edited Loading

Codecov Report

ti-chi-bot bot commented Dec 30, 2024

[LGTM Timeline notifier]

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

winoros Dec 30, 2024

Choose a reason for hiding this comment

time-and-fate commented Dec 30, 2024

winoros commented Dec 30, 2024

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Dec 31, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 5, 2025 • edited Loading

winoros commented Jan 6, 2025

yudongusa left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 6, 2025

ti-chi-bot commented Jan 13, 2025

codecov bot commented Dec 28, 2024 •

edited

Loading

Rustin170506 Dec 30, 2024 •

edited

Loading

ti-chi-bot bot commented Jan 5, 2025 •

edited

Loading