Skip to content

[IOTDB-1473] Fix the bug that run SessionExample.java will be failed in#3770

Merged
OneSizeFitsQuorum merged 1 commit intoapache:masterfrom
chengjianyun:bugfix/cluster-schema
Aug 19, 2021
Merged

[IOTDB-1473] Fix the bug that run SessionExample.java will be failed in#3770
OneSizeFitsQuorum merged 1 commit intoapache:masterfrom
chengjianyun:bugfix/cluster-schema

Conversation

@chengjianyun
Copy link
Copy Markdown
Contributor

@chengjianyun chengjianyun commented Aug 17, 2021

Note this is the replacement of PR: #3755

JIRA issue: https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-1473

The issue happens because auto create schema not work in cluster mode. The insertTables() tries to insert timeseries to root.sg1.d2.s1, root.sg1.d2.s2, root.sg1.d2.s3 but which aren't created until the insertion. The schema create logic fail in the execution path which cause the issue.

Actually, there are three errors auto create schema related in the cluster as far as I can know (comment out the create timeseries related code in SessionExample.java to reproduce).

    ...
    // createTimeseries();
    // createMultiTimeseries();
    // insertRecord();
    insertTablet();   // can't auto create schema
    insertTablets(); // can't auto create schema
    insertRecords(); // can't auto create schema
    selectInto();
   .....

This PR fixed the first two of situations. Has filed a JIRA issue to track the third situation which need some more discussion.

Fix logic

Rule

Put the create schema logic as low level as possible in cluster handle logic. Previous, schema auto creating logic is handle in Coordinator.java which often fails because PathNotExistException often be wrapped. And it can't handle batch insertion as well.

According to the rule, we move the schema creating logic to executeNonQueryPlan in DataGroupMember.java.

Concrete fix

insertTablet() fails because this kind of insertion will call forwardMultiSubPlan() which will return a wrapper result instead of 304 error directly which cause the error handling logic doesn't work.

For insertTablets(), it also need to apply measurements according to failed measurements for all sub insertTablet.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi, this is your first pull request in IoTDB project. Thanks for your contribution! IoTDB will be better because of you.

@jt2594838
Copy link
Copy Markdown
Contributor

The basic idea is good, but if you let the DataGroupMembers handle the schema creation and time partitioning is enabled, one tablet may be split into several smaller tablets and they are handled by more than one DataGroupMembers, so the auto-creation may be duplicated and only one of the attempts will succeed. Have you looked into such a case?

@chengjianyun
Copy link
Copy Markdown
Contributor Author

The basic idea is good, but if you let the DataGroupMembers handle the schema creation and time partitioning is enabled, one tablet may be split into several smaller tablets and they are handled by more than one DataGroupMembers, so the auto-creation may be duplicated and only one of the attempts will succeed. Have you looked into such a case?

Thanks for your reminder. For the case you mentioned, yes, it's the drawback of the solution but at the same time, it's trade off. There would be same schema creation request submit to meta group but the possibility is small. It only happens if ingestion time series records in the tablet to same storage group fall into different time partition and then different data group. This means either time partition interval is very small or time range of the tablet is widely which are corner cases. Let me test the cases to make sure things work well.

The drawback previous solution is obvious, Coordinator need to wait for the actual execution node complete then to know whether need to create schema then forward insert again which has longer latency. Let the execution node (leader of data group) handle schema creation should be a good trade off.

Thanks.

@chengjianyun
Copy link
Copy Markdown
Contributor Author

The basic idea is good, but if you let the DataGroupMembers handle the schema creation and time partitioning is enabled, one tablet may be split into several smaller tablets and they are handled by more than one DataGroupMembers, so the auto-creation may be duplicated and only one of the attempts will succeed. Have you looked into such a case?

hi, I tested the case that multi different data group submit the same schema creation request. Finally, only one schema creation plan actually executed. Everything works well in the situation.

Cluster Env: 4 nodes, replication_num=3, multi-raft-factor=2, enable-partition=true, partition_interval=604800

main test code as below.

    long timestamp = System.currentTimeMillis();

    for (int i = 0; i < 100; i++) {
      for (long row = 0; row < 10; row++) {
        int rowIndex = tablet.rowSize++;
        tablet.addTimestamp(rowIndex, row + 604900L * 1000 * i);
        for (int s = 0; s < 3; s++) {
          long value = new Random().nextLong();
          tablet.addValue(schemaList.get(s).getMeasurementId(), rowIndex, value);
        }
      }
    }

    for (long row = 0; row < 100; row++) {
      int rowIndex = tablet.rowSize++;
      tablet.addTimestamp(rowIndex, timestamp);
      for (int s = 0; s < 3; s++) {
        long value = new Random().nextLong();
        tablet.addValue(schemaList.get(s).getMeasurementId(), rowIndex, value);
      }
      if (tablet.rowSize == tablet.getMaxRowNumber()) {
        session.insertTablet(tablet, true);
        tablet.reset();
      }
      timestamp++;
    }

    session.insertTablet(tablet);
    tablet.reset();

cluster mode. Root cause is auto create schema fail in cluster mode.
@chengjianyun chengjianyun force-pushed the bugfix/cluster-schema branch from bb4ffc5 to 10e02dc Compare August 18, 2021 10:16
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.03%) to 67.285% when pulling 10e02dc on chengjianyun:bugfix/cluster-schema into 120678a on apache:master.

@OneSizeFitsQuorum OneSizeFitsQuorum added the Module - Cluster PRs for the cluster module label Aug 18, 2021
Copy link
Copy Markdown
Contributor

@OneSizeFitsQuorum OneSizeFitsQuorum left a comment

Choose a reason for hiding this comment

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

LGTM~
In fact, I really like this implementation. This will save us a round of RTT in most cases.
However, there are some cases that may need further fixing, @MrQuansy will continue to fix in PR3638 after your PR is merged. You can continue to communicate with each other.
Thank you for your contribution!

@OneSizeFitsQuorum OneSizeFitsQuorum merged commit 582e742 into apache:master Aug 19, 2021
@chengjianyun chengjianyun deleted the bugfix/cluster-schema branch August 19, 2021 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module - Cluster PRs for the cluster module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants