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

Extract Coprocessor DAG #5155

Merged
merged 18 commits into from Aug 2, 2019

Conversation

@breeswish
Copy link
Member

commented Jul 30, 2019

What have you changed? (mandatory)

Extract Coprocessor DAG into a standalone workspace called tidb_query (TiDB query engine). Also renamed current cop_xxx workspaces into tidb_query_xxx.

  • Merge #5152 first.
  • Add failpoint tests for region error while scanning, lock error and outdated error.
  • Update makefile for make expression.
  • Check backtrace behaviour: currently we won't output additional backtrace for these errors
  • Remove unknown_err!. Enhanced other_err! to behave the same as box_err.
  • Update grafana to remove unused deadline waiting metrics. pingcap/tidb-ansible#859

What are the type of the changes? (mandatory)

  • Engineering (engineering change which doesn't change any feature or fix any issue)

How has this PR been tested? (mandatory)

Existing tests.

Refer to a related PR or issue link (optional)

#4180

breeswish added some commits Jul 30, 2019

Extract Coprocessor DAG into tidb_qe
Signed-off-by: Breezewish <breezewish@pingcap.com>
Merge commit 'b1096aad8479a8b0411af3ada3e908cd72283275' into __cop_ex…
…tract/6

Signed-off-by: Breezewish <breezewish@pingcap.com>

@breeswish breeswish added the S: DNM label Jul 30, 2019

Merge remote-tracking branch 'origin/master' into __cop_extract/6
Signed-off-by: Breezewish <breezewish@pingcap.com>

@breeswish breeswish force-pushed the breeswish:__cop_extract/6 branch from ec142a6 to c2d9924 Jul 30, 2019

Supply more failpoint tests
Signed-off-by: Breezewish <breezewish@pingcap.com>

@breeswish breeswish force-pushed the breeswish:__cop_extract/6 branch from c2d9924 to b5fcd8a Jul 30, 2019

breeswish added some commits Jul 30, 2019

Update make expression
Signed-off-by: Breezewish <breezewish@pingcap.com>
change to warning
Signed-off-by: Breezewish <breezewish@pingcap.com>
Make unknown_err behaves the same as box_err
Signed-off-by: Breezewish <breezewish@pingcap.com>

@breeswish breeswish removed the S: DNM label Jul 31, 2019

@lonng lonng added the C: Copr label Aug 1, 2019

Show resolved Hide resolved Makefile Outdated
pub fn code(&self) -> i32 {
match self {
// TODO: We should assign our own error code
EvaluateError::MaxExecuteTimeExceeded => 1105,

This comment has been minimized.

Copy link
@lonng

lonng Aug 1, 2019

Contributor

I think we should organize all error codes in an individual module and assign a constant for them.

This comment has been minimized.

Copy link
@breeswish

breeswish Aug 1, 2019

Author Member

This is the only place need these codes so looks like separating codes does not help readability, flexibility or reduce repeats.

use crate::storage::IntervalRange;
use crate::Result;

const SINGLE_GROUP: &[u8] = b"SingleGroup";

This comment has been minimized.

Copy link
@lonng

lonng Aug 1, 2019

Contributor

Should we give a shorter key for this?

This comment has been minimized.

Copy link
@breeswish

breeswish Aug 1, 2019

Author Member

We can keep it untouched for now. It shall be removed in DAG v2 to implement a TiDB compatible schema.

use protobuf::Message;
use tipb::executor::{self, ExecType, ExecutorExecutionSummary};
use tipb::select::{Chunk, DAGRequest, SelectResponse, StreamResponse};

This comment has been minimized.

Copy link
@lonng

lonng Aug 1, 2019

Contributor

Remove this extra blank line.

This comment has been minimized.

Copy link
@breeswish

breeswish Aug 1, 2019

Author Member

The blank line is intended, for separating external dependencies and project dependencies.

use super::time::{Duration, Instant};

#[derive(Debug, Copy, Clone)]
pub struct DeadlineError;

This comment has been minimized.

Copy link
@lonng

lonng Aug 1, 2019

Contributor

Why not derive from Fail which can implement description automatically.

This comment has been minimized.

Copy link
@breeswish

breeswish Aug 1, 2019

Author Member

Currently there is no fail in tikv_util and I don't want to import it for this simple purpose.

@siddontang

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

I don't like the name of tidb_qe, please use a better name.

breeswish added some commits Aug 1, 2019

Address comments
Signed-off-by: Breezewish <breezewish@pingcap.com>
Rename tidb_qe to tidb_query
Signed-off-by: Breezewish <breezewish@pingcap.com>

@breeswish breeswish force-pushed the breeswish:__cop_extract/6 branch from 3bb7390 to 45423e7 Aug 1, 2019

More reasonable error code
Signed-off-by: Breezewish <breezewish@pingcap.com>
futures = "0.1"

This comment has been minimized.

Copy link
@lonng

lonng Aug 1, 2019

Contributor

Does this blank line have some special intention?

This comment has been minimized.

Copy link
@breeswish

breeswish Aug 1, 2019

Author Member

For separating workspace dependencies and 3rd-party dependencies.

Show resolved Hide resolved Makefile Outdated
Fix makefile
Signed-off-by: Breezewish <breezewish@pingcap.com>

breeswish added some commits Aug 1, 2019

Rearrange imports
Signed-off-by: Breezewish <breezewish@pingcap.com>
Apply some missing change from box_err to other_err
Signed-off-by: Breezewish <breezewish@pingcap.com>
@breeswish

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2019

/run-all-tests

1 similar comment
@mahjonp

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2019

/run-all-tests

@lonng

lonng approved these changes Aug 2, 2019

@lonng lonng merged commit 99a9225 into tikv:master Aug 2, 2019

5 checks passed

DCO All commits are signed off!
Details
idc-jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details

@breeswish breeswish deleted the breeswish:__cop_extract/6 branch Aug 2, 2019

@breeswish breeswish referenced this pull request Aug 2, 2019

Closed

Extract Coprocessor DAG #4180

@breeswish breeswish referenced this pull request Aug 15, 2019

Closed

Extract Coprocessor DAG #4707

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.