Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

*: Reorg part update table #40714

Merged
merged 946 commits into from
Jan 20, 2023
Merged

*: Reorg part update table #40714

merged 946 commits into from
Jan 20, 2023

Conversation

mjonss
Copy link
Contributor

@mjonss mjonss commented Jan 18, 2023 •
edited
Loading

What problem does this PR solve?

PR on top of #40543 so looks bigger than it is.

Implementing Reorganize Partition specifics of Update Table (+ updated test)

Issue Number: close #38535

Problem Summary:

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

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

chrysan and others added 30 commits December 20, 2022 20:50
@chrysan
sessionctx: disable gc aware memory track (pingcap#40019)
806fcbf 
close pingcap#39971
@chrysan
*: make auto-analyze killable by global memory limit (pingcap#39978)
fdf335e 
ref pingcap#39971, close pingcap#39994
@mjonss
Linting
5a60f7e
@mjonss
Updated after make bazel_prepare
85257f3
@hawkingrei
cpu: fix ticker to avoid close early (pingcap#40036)
ae2d551 
ref pingcap#40029
@dependabot
build(deps): bump golang.org/x/time from 0.2.0 to 0.3.0 (pingcap#39912)
a2fa187
@tangenta
ddl: use latest ts to read record for adding index (pingcap#40081)
51cce45 
close pingcap#40074
@hawkingrei
executor: close recordset again (pingcap#40073)
2150c6b
@Reminiscent
planner: add more test cases for MPP hints (pingcap#39933)
785f515
@fzzf678
planner: support set binding status by sql digest (pingcap#39517)
08f23ef 
ref pingcap#39199
@xiongjiwei
ddl: check the limitation when creating multi-valued index (pingcap#3…
b4f500e 
…9818)

close pingcap#40086
@CbcWestwolf
sysvar: allow modifying 'tidb_allow_remove_auto_inc' when SEM is ON (p…
13e2120 
…ingcap#40083)

close pingcap#38238
@crazycs520
*: fix issue of multi-schema change with foreign key (pingcap#40042)
ad0c202 
close pingcap#40037
@qw4990
planner: check the ignore-plan-cache hint in insert-stmt (pingcap#4…
9e8a21c 
…0080)

ref pingcap#39717, close pingcap#40079
@Reminiscent
parser: support keep_order and no_keep_order hint (pingcap#39965)
fc3f04b 
ref pingcap#39964
@xiongjiwei
*: filter particularly errors when truncateAsWarning is true (pingc…
47ace08 
…ap#40078)
@mjonss
Removed CI debug log
273e7e8
@mjonss
Cleaned PR to only have the core of the data copying parts
b086421
@mjonss
Merge branch 'reorg-part-data-reorg' into reorg-part-rollback
74fa4c4
@mjonss
More cleanup of PR to only focus on reorg data copy
b0c16cd
@mjonss
Merge branch 'reorg-part-data-reorg' into reorg-part-rollback
73dcf5a
@mjonss
Added check to avoid null-pointer dereferencing
0accdd8
@mjonss
Fixed nil session context
8c914b0
@mjonss
Cleanup
0678894
@mjonss
Added Info logs for debugging in CI
3a1adcd
@mjonss
more CI debug info logs + cleared a TODO
f23a27a
@mjonss
Added one more CI debug line
ecb7f50
@hawkingrei
metrics: add metrics for ema cpu metrics and GOGC (pingcap#40049)
4e43b14 
close pingcap#40092
@zyguan
server: avoid reusing cached stmt ctx on cursor read (pingcap#40023)
0f4bd73 
close pingcap#39998
@hackersean
*: optimize mpp probe (pingcap#39932)
aeccf77 
close pingcap#39686
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 18, 2023 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • bb7133
  • tiancaiamao

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 18, 2023
This was referenced Jan 18, 2023
Closed
Closed
@mjonss mjonss changed the title *: Reorg part update table *: Reorg part update table | tidb-test=pr/2069 Jan 18, 2023
@mjonss
Copy link
Contributor Author

mjonss commented Jan 18, 2023

/retest

@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 19, 2023
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 19, 2023
bb7133
bb7133 reviewed Jan 19, 2023
View reviewed changes
ddl/backfilling.go Show resolved Hide resolved
statistics/handle/ddl.go
@@ -61,6 +61,21 @@ func (h *Handle) HandleDDLEvent(t *util.Event) error {
return err
}
}
case model.ActionReorganizePartition:
for _, def := range t.PartInfo.Definitions {
// TODO: Should we trigger analyze instead of adding 0s?
Copy link
Member

Choose a reason for hiding this comment

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

What does 'adding 0s' mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this will just write stats as all zeros / empty table/indexes. Preferably I think it should trigger stats on the new partitions, including their indexes as well as update the global stats. And also wait for all this during the DeleteReorganization phase, so stats are up-to-date when going to StatePublic where the stats will be needed.

bb7133
bb7133 approved these changes Jan 19, 2023
View reviewed changes
Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 19, 2023
@bb7133
Copy link
Member

bb7133 commented Jan 19, 2023

PTAL @wjhuang2016 @tangenta

tangenta
tangenta reviewed Jan 20, 2023
View reviewed changes
ddl/reorg.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 20, 2023
@mjonss
Copy link
Contributor Author

mjonss commented Jan 20, 2023

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 0cb356f

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 20, 2023
@mjonss mjonss changed the title *: Reorg part update table | tidb-test=pr/2069 *: Reorg part update table Jan 20, 2023
@ti-chi-bot ti-chi-bot merged commit 07a7465 into pingcap:feature/reorganize-partition Jan 20, 2023
This was referenced Jan 31, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tangenta tangenta tangenta left review comments

@tiancaiamao tiancaiamao tiancaiamao approved these changes

@bb7133 bb7133 bb7133 approved these changes

@wjhuang2016 wjhuang2016 Awaiting requested review from wjhuang2016

Assignees
No one assigned
Labels
release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.