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

statistics: fix sync load fails after disabling lite init stats #54531

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

statistics: fix sync load fails after disabling lite init stats #54531

wants to merge 7 commits into from
+106 −38

Conversation

hawkingrei
Copy link
Member

@hawkingrei hawkingrei commented Jul 9, 2024 •
edited
Loading

What problem does this PR solve?

Issue Number: close #54532

Problem Summary:

What changed and how does it work?

after #53399, we cannot load the common column's stats into the cache. so it also cannot init the infoschema in the stats structure. so it will lead the problem, which cannot sync load

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

  • 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-linked-issue do-not-merge/needs-tests-checked release-note-none sig/planner SIG: Planner size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 9, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 51.66667% with 29 lines in your changes missing coverage. Please review.

Project coverage is 50.1448%. Comparing base (419139e) to head (fdc34e1).
Report is 6 commits behind head on master.

❗ Current head fdc34e1 differs from pull request most recent head 1e4c7f0

Please upload reports for the commit 1e4c7f0 to get more accurate results.

Additional details and impacted files 
@@                Coverage Diff                @@
##             master     #54531         +/-   ##
=================================================
- Coverage   74.7209%   50.1448%   -24.5761%     
=================================================
  Files          1557       1673        +116     
  Lines        363550     601631     +238081     
=================================================
+ Hits         271648     301687      +30039     
- Misses        72247     276899     +204652     
- Partials      19655      23045       +3390
Flag Coverage Δ
integration 18.7155% <51.6666%> (?)

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

Components Coverage Δ
dumpling 55.1994% <ø> (ø)
parser ∅ <ø> (∅)
br 48.4455% <ø> (+0.7597%) ⬆️
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Jul 9, 2024
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from 7a80598 to 6f18b1f Compare July 10, 2024 02:16
@hawkingrei
Copy link
Member Author

/retest
1 similar comment
@hawkingrei
Copy link
Member Author

/retest
@hawkingrei hawkingrei requested a review from winoros July 10, 2024 06:46
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch 2 times, most recently from 9923a4e to 35a2237 Compare July 10, 2024 07:04
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from dec0852 to 3fd13fd Compare July 10, 2024 10:13
@ti-chi-bot ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 10, 2024
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from 4925b62 to 3fd13fd Compare July 10, 2024 15:12
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 10, 2024
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from 84b0d15 to 9617a46 Compare July 10, 2024 16:03
@hawkingrei
Copy link
Member Author

/retest
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch 2 times, most recently from 714295c to ee73a33 Compare July 11, 2024 00:56
@hawkingrei hawkingrei changed the title statistics: fix sync load fails after disabling lite init stats Jul 11, 2024
@hawkingrei
Copy link
Member Author

/retest
1 similar comment
@hawkingrei
Copy link
Member Author

/retest
@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 15, 2024
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Jul 15, 2024

PR needs rebase.

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/test-infra repository.
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from e5842da to a2db608 Compare July 17, 2024 09:34
@hawkingrei hawkingrei changed the title statistics: fix sync load fails after disabling lite init stats [WIP] Jul 17, 2024
@hawkingrei hawkingrei removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2024
@hawkingrei
Copy link
Member Author

/retest
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from e23d8e5 to b69d810 Compare July 17, 2024 15:26
@hawkingrei
Copy link
Member Author

/retest
@hawkingrei hawkingrei changed the title statistics: fix sync load fails after disabling lite init stats Jul 18, 2024
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch 2 times, most recently from 6e27269 to d7478fb Compare July 19, 2024 06:44
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Jul 19, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from hawkingrei and additionally assign bornchanger for approval(Please ensuring that each of them provides their approval before proceeding). For more information see the Code Review Process.

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

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
@hawkingrei hawkingrei changed the title statistics: fix sync load fails after disabling lite init stats [WIP] Jul 19, 2024
winoros
winoros reviewed Jul 21, 2024
View reviewed changes
pkg/statistics/handle/syncload/stats_syncload.go
Comment on lines +565 to +569
if tbl.ColAndIdxExistenceMap.IsColEmpty() {
for _, col := range tbleInfo.Meta().Columns {
tbl.ColAndIdxExistenceMap.InsertCol(col.ID, col, colHist.StatsAvailable())
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Only update the column itself. Otherwise we need to read all column from mysql.stats_histograms.
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from 54da511 to fdc34e1 Compare July 23, 2024 09:52
@hawkingrei
statistics: fix sync load fails after disabling lite init stats
316a770
@hawkingrei
update
4e6664f
@hawkingrei
update
565a2d0 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
tmp
da4df26 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
tmp
8b97085 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
tmp
f59eac6 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei
update
33d9b24 
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@hawkingrei hawkingrei force-pushed the fix_cannot_sync_load_close_lite branch from 1e4c7f0 to 33d9b24 Compare July 25, 2024 09:59
@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Jul 25, 2024

@hawkingrei: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
idc-jenkins-ci-tidb/check_dev_2 33d9b24 link true /test check-dev2
idc-jenkins-ci-tidb/check_dev 33d9b24 link true /test check-dev

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/test-infra repository. I understand the commands that are listed here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
2 participants
@hawkingrei @winoros