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

server: implement spill disk for cursorFetch result #44730

Closed
wants to merge 12 commits into from

Conversation

YangKeao
Copy link
Member

@YangKeao YangKeao commented Jun 16, 2023

What problem does this PR solve?

Issue Number: close #43233

What is changed and how it works?

  1. Use the rowContainer to store the rows after execution.
  2. Reset the memory/disk tracker when the statement is closed.

TODO:

Check List

Tests

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

I used the following codes to do some basic tests. The preloaded data is 5M rows sysbench ./oltp_insert.lua :

public static void tidbBigSelectTest() throws SQLException, InterruptedException {
    // the following are test codes for MySQL
    ConnectionImpl conn = (ConnectionImpl) DriverManager.getConnection("jdbc:mysql://127.0.0.1:4000/test?useCursorFetch=true&defaultFetchSize=1000", "root", "");
    conn.prepareStatement("set @@tidb_mem_quota_query = 8 << 20\n").execute();
    conn.prepareStatement("begin").execute();

    String sql = "SELECT * FROM test.sbtest1";
    PreparedStatement stmt = conn.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
    stmt.setFetchSize(1000);
    Instant beforeExecuteTime = Clock.systemUTC().instant();
    ResultSet rs = stmt.executeQuery();
    Instant afterExecuteTime = Clock.systemUTC().instant();
    System.out.println("It takes " + (Duration.between(beforeExecuteTime, afterExecuteTime).toString()) + " to EXECUTE QUERY");

    ResultSetMetaData metaData = rs.getMetaData();
    int columnCount = metaData.getColumnCount();
    int batchSize = stmt.getFetchSize();
    int rowCount = 0;
    int totalCount = 0;
    while (rs.next()) {
        rowCount++;
        totalCount++;
        Object[] columns = new Object[columnCount];
        for (int i = 1; i <= columnCount; i++) {
            columns[i - 1] = rs.getObject(i);
        }
        Row row = new Row(columns);
        if (rowCount == batchSize) {
            System.out.println("Fetched " + batchSize + " rows");
            System.out.println("allocated memory is " + (Runtime.getRuntime().totalMemory()-Runtime.getRuntime().freeMemory()));
            System.out.println("TotalCount " + totalCount);
            rowCount = 0;
        }
        if (totalCount == 4999999) {
            System.out.println("here");
        }
    }
    if (rowCount > 0) {
        System.out.println("allocated memory is " + (Runtime.getRuntime().totalMemory()-Runtime.getRuntime().freeMemory()));
        System.out.println("Fetched " + rowCount + " rows");
        System.out.println("TotalCount " + totalCount);
    }

    conn.prepareStatement("commit").execute();
}

Release note

Automatically spill the memory exceeding `tidb_mem_quota_query` into the disk

@YangKeao YangKeao added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 16, 2023
@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jun 16, 2023
@YangKeao YangKeao marked this pull request as draft June 16, 2023 07:38
@ti-chi-bot ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 16, 2023
@YangKeao YangKeao changed the title server: implement basic statement memory spill server: implement spill disk for cursorFetch result Jun 16, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch 3 times, most recently from d5ba21f to 2bd9b83 Compare June 19, 2023 09:18
@ti-chi-bot ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 19, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch 2 times, most recently from fca0c6f to 481b15b Compare June 21, 2023 07:36
@YangKeao YangKeao marked this pull request as ready for review June 21, 2023 07:48
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 21, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch from a265725 to f338924 Compare June 21, 2023 08:50
@YangKeao YangKeao requested review from tiancaiamao and XuHuaiyu June 21, 2023 08:52
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch 2 times, most recently from 1fc0d53 to 3bce133 Compare June 26, 2023 07:33
@YangKeao
Copy link
Member Author

/test tiprow_fast_test

@ti-chi-bot
Copy link

ti-chi-bot bot commented Jun 26, 2023

@YangKeao: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test build
  • /test canary-scan-security
  • /test check-dev
  • /test check-dev2
  • /test mysql-test
  • /test pingcap/tidb/canary_ghpr_unit_test
  • /test pull-integration-br-test
  • /test pull-integration-mysql-test
  • /test unit-test

Use /test all to run the following jobs that were automatically triggered:

  • pingcap/tidb/ghpr_build
  • pingcap/tidb/ghpr_check
  • pingcap/tidb/ghpr_check2
  • pingcap/tidb/ghpr_mysql_test
  • pingcap/tidb/ghpr_unit_test

In response to this:

/test tiprow_fast_test

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.

@ti-chi-bot ti-chi-bot bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 27, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch from e90f142 to 8cb0124 Compare June 27, 2023 08:16
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch from bce5f3d to 1e9e54f Compare July 3, 2023 03:51
@YangKeao YangKeao marked this pull request as draft July 3, 2023 03:51
@ti-chi-bot ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2023
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch 2 times, most recently from 778a790 to 6795df9 Compare July 3, 2023 07:35
@YangKeao YangKeao force-pushed the cursor-fetch-persist branch from 6795df9 to 76d2951 Compare July 3, 2023 07:39
@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 4, 2023
@ti-chi-bot
Copy link

ti-chi-bot bot commented Jul 4, 2023

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.

@YangKeao YangKeao marked this pull request as ready for review July 5, 2023 05:43
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 5, 2023
@YangKeao
Copy link
Member Author

YangKeao commented Jul 5, 2023

Github doesn't update this PR after I rebase and solve the conflict. It's really weird. The content of this PR is different from the content in my branch: https://github.com/YangKeao/tidb/tree/cursor-fetch-persist (e.g. the file /util/chunk/row_container_reader.go).

I'll try to close and reopen this PR to see whether it'll update 🤦 .

@YangKeao YangKeao closed this Jul 5, 2023
@YangKeao
Copy link
Member Author

YangKeao commented Jul 5, 2023

Good. Then it doesn't allow me to reopen it 😮‍💨 . I have to create a new one.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiDB should track, limit (and maybe spill) the temporary memory used by CursorFetch
3 participants