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.

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

executor: support read global indexes in IndexMergeReader and index join #20350

Closed

Conversation

ldeng-ustc
Copy link
Contributor

@ldeng-ustc ldeng-ustc commented Oct 6, 2020

What problem does this PR solve?

Problem Summary: support read global indexes in IndexMergeReader.

What is changed and how it works?

Proposal: #18982

When IndexMergeReader is processing first partition of partitioned table, global indexes will be read. handles of different partitions will temporarily store in different HandleMap. When certain partition processing, worker first process the handles has stored in HandleMap, then process the other handles from local indexes as before.

Add test case for index join with global index.

Check List

Tests

  • Unit test

Release note

  • None.

@ldeng-ustc ldeng-ustc requested review from a team as code owners October 6, 2020 12:24
@ldeng-ustc ldeng-ustc requested review from XuHuaiyu and removed request for a team October 6, 2020 12:24
@ti-srebot ti-srebot added the contribution This PR is from a community contributor. label Oct 6, 2020
@github-actions github-actions bot added the sig/execution SIG execution label Oct 6, 2020
@ldeng-ustc ldeng-ustc changed the title executor: support read global indexes in IndexMergeReader executor: support read global indexes in IndexMergeReader and index join Oct 9, 2020
if e.hasGlobalIndex && !e.skipGlobalIndex {
e.handleMaps = make(map[int64]*kv.HandleMap)
for _, p := range e.table.Meta().GetPartitionInfo().Definitions {
e.handleMaps[p.ID] = kv.NewHandleMap()
Copy link
Contributor

Choose a reason for hiding this comment

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

Now the maximum partitions count is 4096, so there is a potential performance regression here.

Copy link
Contributor

Choose a reason for hiding this comment

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

But if we make the may lazy initialized, maybe there would be a data race

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found that handles are unique in whole partitioned table. How about use a single global handleMap to store all handles and not clear when call nextPartition. But memory resource can't immediately recycle after processed every partitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or use two handle maps, one store handles read from global index, another store handles read from local index, so that local handle map could free after process every partition. But we must check twice for every handles.

@tiancaiamao
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 20, 2020
@tiancaiamao
Copy link
Contributor

PTAL @XuHuaiyu

@zz-jason
Copy link
Member

zz-jason commented Feb 9, 2021

I'm going to close this PR since it hasn't been updated for a long time. feel free to reopen if you want to continue with it. thank you for your contribution.

@zz-jason zz-jason closed this Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. sig/execution SIG execution status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants