statstics: reuse fmsketch

[hawkingrei]

What problem does this PR solve?

Issue Number: close #47071

Problem Summary:

What is changed and how it works?

Check List

Tests

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

before:

after:

• 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

[codecov]

Codecov Report

Merging #47070 (179343e) into master (4450ae4) will decrease coverage by 0.3519%.
Report is 2 commits behind head on master.
The diff coverage is 100.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47070        +/-   ##
================================================
- Coverage   72.9633%   72.6115%   -0.3519%     
================================================
  Files          1337       1358        +21     
  Lines        399007     405580      +6573     
================================================
+ Hits         291129     294498      +3369     
- Misses        89075      92380      +3305     
+ Partials      18803      18702       -101     

Flag	Coverage Δ
integration	30.6066% <75.9259%> (?)
unit	72.9540% <100.0000%> (-0.0094%)	⬇️

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

Components	Coverage Δ
dumpling	53.9913% <ø> (ø)
parser	84.9620% <ø> (-0.0108%)	⬇️
br	48.5758% <ø> (-4.3639%)	⬇️

[time-and-fate]

I think the logic in this PR is very fragile.
In this PR:

1. We put back the allFms in the pool in MergePartitionStats2GlobalStats
2. We put back the GlobalStats.Fms in the pool in handleGlobalStats and updateGlobalStats, which is returned from MergePartitionStats2GlobalStats
3. We put back the tableAllPartitionStats.Column.FMSketch in the pool in handleGlobalStats, which is also created in MergePartitionStats2GlobalStats.

The assumptions are very fragile, if we share structs in any two of the above three places, there will be bugs. And it's hard to realize this, because we put them back in the pool in totally different places.

[time-and-fate]

And we are handling the logic inconsistently.
When we call MergePartitionStats2GlobalStats from updateGlobalStats, we don't reuse the FM sketch in the allPartitionStats, but when we call MergePartitionStats2GlobalStats from handleGlobalStats, we reuse them.
Even in handleGlobalStats, we only reuse FM sketches from Column, but not reuse those from Index.

[time-and-fate]

My suggestion is to do all the Put() in MergePartitionStats2GlobalStats. And it looks like we can even remove Fms from the struct GlobalStats.

[hawkingrei]

I think the logic in this PR is very fragile. In this PR:

1. We put back the `allFms` in the pool in `MergePartitionStats2GlobalStats`

2. We put back the `GlobalStats.Fms` in the pool in `handleGlobalStats` and `updateGlobalStats`, which is returned from `MergePartitionStats2GlobalStats`

3. We put back the `tableAllPartitionStats.Column.FMSketch` in the pool in `handleGlobalStats`, which is also created in `MergePartitionStats2GlobalStats`.

The assumptions are very fragile, if we share structs in any two of the above three places, there will be bugs. And it's hard to realize this, because we put them back in the pool in totally different places.

In fact, we optimize the two places.

1、create FM sketch from the NewFmSketch
2、Create from the proto

all the implements create Fmsketch from the pool and release Fmsketch at the end of the lift.

the root cause is at the tableAllPartitionStats which the life of FMsketch is out of MergePartitionStats2GlobalStats.

[hawkingrei]

My suggestion is to do all the Put() in MergePartitionStats2GlobalStats. And it looks like we can even remove Fms from the struct GlobalStats.

I have refactor this code. Now we have an external cache in the mergeGlobalStatus. if it is not from the external, we will release it in the mergeGlobalStatus.

[time-and-fate]

Looks like we don't need this. We initiate globalStats below.

[hawkingrei]

Done

[time-and-fate]

I think it's safer to set FMSketch to nil after putting to the pool.

[time-and-fate]

Ok to me to improve them in another PR or in another way at your convenience.

[hawkingrei]

Done，use map.clear to clean it.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qw4990, time-and-fate

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 [qw4990,time-and-fate]

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]

[LGTM Timeline notifier]

Timeline:

• 2023-09-19 02:05:06.020853202 +0000 UTC m=+569471.988441252: ☑️ agreed by qw4990.
• 2023-09-19 13:37:16.094453453 +0000 UTC m=+611002.062041506: ☑️ agreed by time-and-fate.

[tiprow]

@hawkingrei: 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
tiprow_fast_test	179343e	link	true	/test tiprow_fast_test

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.

[hawkingrei]

/retest

[hawkingrei]

/cherrypick release-6.5

[ti-chi-bot]

@hawkingrei: new pull request created to branch release-6.5: #49573.

In response to this:

/cherrypick release-6.5

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 ti-community-infra/tichi repository.