search

pingcap / tidb

TiDB is an open-source, cloud-native, distributed, MySQL-Compatible database for elastic scale and real-time analytics. Try AI-powered Chat2Query free at : https://www.pingcap.com/tidb-serverless/
https://pingcap.com
Apache License 2.0
36.32k stars 5.72k forks source link

context: Add method to detach the `tableReaderExecutor` from the session context #53337

Open YangKeao opened 2 weeks ago

YangKeao commented 2 weeks ago

What problem does this PR solve?

Issue Number: close #53336

Problem Summary:

It's not safe to call Next (or other method) of an executor after the session is executing the next statement. We have split out a lot of Context and it's clear that the context depended by the TableReaderExecutor is safe to be re-used (after proper configuration) while executing another statement is running on the session.

What changed and how does it work?

This PR adds IntoStatic method for several contexts. After calling this method, the context can be safe to be used while the session continues to run the next statements.

Check List

Tests

TODO:

Side effects

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None
ti-chi-bot[bot] commented 2 weeks ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

YangKeao commented 2 weeks ago

/retest

tiprow[bot] commented 2 weeks ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

YangKeao commented 2 weeks ago

/retest

YangKeao commented 2 weeks ago

/retest

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 15.15152% with 84 lines in your changes are missing coverage. Please review.

Project coverage is 74.6350%. Comparing base (0afe54d) to head (740ccd3). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #53337 +/- ## ================================================ + Coverage 72.5179% 74.6350% +2.1171% ================================================ Files 1505 1506 +1 Lines 429866 429972 +106 ================================================ + Hits 311730 320910 +9180 + Misses 98820 89222 -9598 - Partials 19316 19840 +524 ``` | [Flag](https://app.codecov.io/gh/pingcap/tidb/pull/53337/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/pingcap/tidb/pull/53337/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | `49.1553% <15.1515%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap#carryforward-flags-in-the-pull-request-comment) to find out more. | [Components](https://app.codecov.io/gh/pingcap/tidb/pull/53337/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | Coverage Δ | | |---|---|---| | [dumpling](https://app.codecov.io/gh/pingcap/tidb/pull/53337/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | `53.9957% <ø> (ø)` | | | [parser](https://app.codecov.io/gh/pingcap/tidb/pull/53337/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | `∅ <ø> (∅)` | | | [br](https://app.codecov.io/gh/pingcap/tidb/pull/53337/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pingcap) | `50.4334% <ø> (+9.0600%)` | :arrow_up: |
YangKeao commented 1 week ago

/retest

ti-chi-bot[bot] commented 1 week ago

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-tests-checked label, please finished the tests then check the finished items in description.

For example:

Tests

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

:open_book: For more info, you can check the "Contribute Code" section in the development guide.

YangKeao commented 1 week ago

/retest

ti-chi-bot[bot] commented 1 week ago

[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 assign windtalker, winoros, zimulala for approval, 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: - **[OWNERS](https://github.com/pingcap/tidb/blob/master/OWNERS)** - **[pkg/ddl/OWNERS](https://github.com/pingcap/tidb/blob/master/pkg/ddl/OWNERS)** - **[pkg/distsql/OWNERS](https://github.com/pingcap/tidb/blob/master/pkg/distsql/OWNERS)** - **[pkg/expression/OWNERS](https://github.com/pingcap/tidb/blob/master/pkg/expression/OWNERS)** - **[pkg/planner/OWNERS](https://github.com/pingcap/tidb/blob/master/pkg/planner/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
tiprow[bot] commented 1 week ago

@YangKeao: 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
fast_test_tiprow 740ccd3d37fe4a662ac131750d37008f0b280200 link true /test fast_test_tiprow

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
ti-chi-bot[bot] commented 1 week ago

@YangKeao: 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
idc-jenkins-ci-tidb/unit-test 740ccd3d37fe4a662ac131750d37008f0b280200 link true /test unit-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).