statistics: merge the partition-level stats to global-level stats #22667
Conversation
Reminiscent
changed the title
statistics/handle/handle.go
Outdated
| globalStats.Cms[i] = allCms[i][0].Copy() | ||
| for j := uint64(1); j < partitionNum; j++ { | ||
| err := globalStats.Cms[i].MergeCMSketch(allCms[i][j]) | ||
| if err != nil { | ||
| logutil.BgLogger().Debug("failed to merge the CMSketch when we merge the partition-level stats to global-level stats") | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe we can merge this part into an additional function to make here clear?
statistics/handle/handle.go
Outdated
| return | ||
| } | ||
| tableInfo := partitionTable.Meta() | ||
| partitionStats, err := h.TableStatsFromStorage(tableInfo, partitionID, false, nil) |
There was a problem hiding this comment.
We can put this in the cache. But I think it's a tradeoff. Because we just use the partition-level stats to build the global-level stats. Hardly used in other situations. This will take up a lot of cache memory to store these stats that are likely to be used only once.
There was a problem hiding this comment.
Partition-level stats can be divided into two kinds. One kind is not involved in current analyze and we just read stats from storage to participate in the merging process. The other kind is newly calculated and saved to storage in current analyze. Reading the second kind stats from storage seems unnecessary since we have the stats in memory.
There was a problem hiding this comment.
After discussion with @qw4990 , we think maybe we can do this optimization later. It is a low-frequency operation. What's your opinion @xuyifangreeneyes
qw4990
requested review from
xuyifangreeneyes
and removed request for
XuHuaiyu
There was a problem hiding this comment.
Please add some basic test cases for this PR, for example:
- create a partition table and analyze it and check if
MergePartitionStats2GlobalStatsis triggered; - generate an error when merging partition-stats and then check its statistics is not polluted(since when the error arise, all operations should be rollbacked);
…obalStats-1 � Conflicts: � statistics/handle/handle.go
|
/run-all-tests |
| err = errors.Errorf("TODO: The merge function of the histogram structure has not been implemented yet") | ||
| if err != nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
NDV should be merged here. You can add it when you merge NDV later.
|
/merge |
ti-srebot
added
the
status/can-merge
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
|
@Reminiscent merge failed. |
|
/run-unit-test |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
What problem does this PR solve?
sub-PR for the PR#22472
Problem Summary:
When we use
analyze tablestatement for the partition table indynamic-onlymode, we will try to merge partition-level stats to get the global-level stats for the partition table.What is changed and how it works?
Proposal: Partition Table Statistics (in Chinese)
What's Changed. How it Works:
Related changes
pingcap/docs/pingcap/docs-cn:Check List
Tests
Side effects
Release note