Skip to content

Commit

Permalink
fix bug that mpp_fail test cost too much time (#1591)
Browse files Browse the repository at this point in the history
* fix bug that mpp_fail test cost too much time

* add some comments

* refine

* fix clustered index related ci
  • Loading branch information
windtalker authored Mar 19, 2021
1 parent 946a897 commit 891466e
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 52 deletions.
16 changes: 6 additions & 10 deletions dbms/src/Flash/Mpp/MPPHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,11 @@ void MPPTask::cancel()
}
}
}
/// step 2. write Error msg to tunnels
writeErrToAllTunnel("MPP Task canceled because it seems hangs");
/// step 2. write Error msg and close the tunnel.
/// Here we use `closeAllTunnel` because currently, `cancel` is a query level cancel, which
/// means if this mpp task is cancelled, all the mpp tasks belonging to the same query are
/// cancelled at the same time, so there is no guarantee that the tunnel can be connected.
closeAllTunnel("MPP Task canceled because it seems hangs");
LOG_WARNING(log, "Finish cancel task: " + id.toString());
}

Expand All @@ -321,14 +324,7 @@ void MPPHandler::handleError(MPPTaskPtr task, String error)
{
if (task != nullptr)
{
/// for root task, the tunnel is only connected after DispatchMPPTask
/// finishes without error, for non-root task, tunnel can be connected
/// even if the DispatchMPPTask fails, so for non-root task, we write
/// error to all tunnels, while for root task, we just close the tunnel.
if (!task->dag_context->isRootMPPTask())
task->writeErrToAllTunnel(error);
else
task->closeAllTunnel();
task->closeAllTunnel(error);
task->unregisterTask();
}
}
Expand Down
23 changes: 15 additions & 8 deletions dbms/src/Flash/Mpp/MPPHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,19 @@ struct MPPTunnel
cv_for_finished.notify_all();
}

/// close() finishes the tunnel without checking the connect status, this function
/// should only be used when handling error if DispatchMPPTask fails for
/// root task. Because for root task, if DispatchMPPTask fails, TiDB does
/// not sending establish MPP connection request at all, it is meaningless
/// to check the connect status in this case, just finish the tunnel.
void close()
/// close() finishes the tunnel, if the tunnel is connected already, it will
/// write the error message to the tunnel, otherwise it just close the tunnel
void close(const String & reason)
{
std::unique_lock<std::mutex> lk(mu);
if (connected)
{
mpp::MPPDataPacket data;
auto err = new mpp::Error();
err->set_msg(reason);
data.set_allocated_error(err);
writer->Write(data);
}
finished = true;
cv_for_finished.notify_all();
}
Expand Down Expand Up @@ -291,13 +296,15 @@ struct MPPTask : std::enable_shared_from_this<MPPTask>, private boost::noncopyab

void cancel();

void closeAllTunnel()
/// Similar to `writeErrToAllTunnel`, but it just try to write the error message to tunnel
/// without waiting the tunnel to be connected
void closeAllTunnel(const String & reason)
{
try
{
for (auto & it : tunnel_map)
{
it.second->close();
it.second->close(reason);
}
}
catch (...)
Expand Down
34 changes: 17 additions & 17 deletions tests/clustered_index_fullstack/data_type.test
Original file line number Diff line number Diff line change
@@ -1,88 +1,88 @@

# pk_is_handle = true
mysql> drop table if exists test.t_int;
mysql> set tidb_enable_clustered_index=1;create table test.t_int (pk int(11) primary key);
mysql> create table test.t_int (pk int(11) primary key clustered);
mysql> insert into test.t_int values(1),(-1);
mysql> alter table test.t_int set tiflash replica 1;

mysql> drop table if exists test.t_uint;
mysql> set tidb_enable_clustered_index=1;create table test.t_uint (pk int(11) unsigned primary key);
mysql> create table test.t_uint (pk int(11) unsigned primary key clustered);
mysql> insert into test.t_uint values(1),(10);
mysql> alter table test.t_uint set tiflash replica 1;

# is_common_handle = true
mysql> drop table if exists test.t_float;
mysql> set tidb_enable_clustered_index=1;create table test.t_float (pk float primary key);
mysql> create table test.t_float (pk float primary key clustered);
mysql> insert into test.t_float values(1.2),(1.3);
mysql> alter table test.t_float set tiflash replica 1;

mysql> drop table if exists test.t_double;
mysql> set tidb_enable_clustered_index=1;create table test.t_double (pk double primary key);
mysql> create table test.t_double (pk double primary key clustered);
mysql> insert into test.t_double values(1.2),(1.3);
mysql> alter table test.t_double set tiflash replica 1;

mysql> drop table if exists test.t_char;
mysql> set tidb_enable_clustered_index=1;create table test.t_char (pk char(10) primary key);
mysql> create table test.t_char (pk char(10) primary key clustered);
mysql> insert into test.t_char values('1'),('10');
mysql> alter table test.t_char set tiflash replica 1;

mysql> drop table if exists test.t_varchar;
mysql> set tidb_enable_clustered_index=1;create table test.t_varchar (pk varchar(10) primary key);
mysql> create table test.t_varchar (pk varchar(10) primary key clustered);
mysql> insert into test.t_varchar values('1'),('10');
mysql> alter table test.t_varchar set tiflash replica 1;

mysql> drop table if exists test.t_date;
mysql> set tidb_enable_clustered_index=1;create table test.t_date (pk date primary key);
mysql> create table test.t_date (pk date primary key clustered);
mysql> insert into test.t_date values('2020-01-01'),('2020-10-10');
mysql> alter table test.t_date set tiflash replica 1;

mysql> drop table if exists test.t_datetime;
mysql> set tidb_enable_clustered_index=1;create table test.t_datetime (pk datetime primary key);
mysql> create table test.t_datetime (pk datetime primary key clustered);
mysql> insert into test.t_datetime values('2020-01-01 11:11:11'),('2020-10-10 22:22:22');
mysql> alter table test.t_datetime set tiflash replica 1;

mysql> drop table if exists test.t_timestamp;
mysql> set tidb_enable_clustered_index=1;create table test.t_timestamp (pk timestamp(3) primary key);
mysql> create table test.t_timestamp (pk timestamp(3) primary key clustered);
mysql> insert into test.t_timestamp values('2020-01-01 11:11:11.123'),('2020-10-10 22:22:22.234');
mysql> alter table test.t_timestamp set tiflash replica 1;

mysql> drop table if exists test.t_year;
mysql> set tidb_enable_clustered_index=1;create table test.t_year (pk year primary key);
mysql> create table test.t_year (pk year primary key clustered);
mysql> insert into test.t_year values(2020),(2013);
mysql> alter table test.t_year set tiflash replica 1;

mysql> drop table if exists test.t_time;
mysql> set tidb_enable_clustered_index=1;create table test.t_time (pk time primary key);
mysql> create table test.t_time (pk time primary key clustered);
mysql> insert into test.t_time values('11:11:11'),('22:22:22');
mysql> alter table test.t_time set tiflash replica 1;

mysql> drop table if exists test.t_decimal;
mysql> set tidb_enable_clustered_index=1;create table test.t_decimal (pk decimal(10,2) primary key);
mysql> create table test.t_decimal (pk decimal(10,2) primary key clustered);
mysql> insert into test.t_decimal values(10.10),(11.11);
mysql> alter table test.t_decimal set tiflash replica 1;

mysql> drop table if exists test.t_bit;
mysql> set tidb_enable_clustered_index=1;create table test.t_bit (pk bit(3) primary key);
mysql> create table test.t_bit (pk bit(3) primary key clustered);
mysql> insert into test.t_bit values(b'101'),(b'010');
mysql> alter table test.t_bit set tiflash replica 1;

mysql> drop table if exists test.t_enum;
mysql> set tidb_enable_clustered_index=1;create table test.t_enum (pk enum('tidb','pd','tikv','tiflash') primary key);
mysql> create table test.t_enum (pk enum('tidb','pd','tikv','tiflash') primary key clustered);
mysql> insert into test.t_enum values('tidb'),('tiflash');
mysql> alter table test.t_enum set tiflash replica 1;

mysql> drop table if exists test.t_set;
mysql> set tidb_enable_clustered_index=1;create table test.t_set (pk set('one','two') primary key, value int);
mysql> create table test.t_set (pk set('one','two') primary key clustered, value int);
mysql> insert into test.t_set values('', 1),('one,two', 2);
mysql> alter table test.t_set set tiflash replica 1;

mysql> drop table if exists test.t_prefix;
mysql> set tidb_enable_clustered_index=1;create table test.t_prefix (pk varchar(10), primary key (pk(2)));
mysql> create table test.t_prefix (pk varchar(10), primary key (pk(2)) clustered);
mysql> insert into test.t_prefix values('abc'),('cde');
mysql> alter table test.t_prefix set tiflash replica 1;

mysql> drop table if exists test.t_all;
mysql> set tidb_enable_clustered_index=1;create table test.t_all (col1 int, col2 int unsigned, col3 float, col4 double, col5 char(10), col6 varchar(10), col7 date, col8 datetime, col9 timestamp(6), col10 year, col11 time, col12 decimal(10,2), col13 bit, col14 enum('tidb','pd','tikv','tiflash'), col15 set('one','two'), col16 varchar(10), primary key (col1,col2,col3,col4,col5,col6,col7,col8,col9,col10,col11,col12,col13,col14,col15,col16(2)));
mysql> create table test.t_all (col1 int, col2 int unsigned, col3 float, col4 double, col5 char(10), col6 varchar(10), col7 date, col8 datetime, col9 timestamp(6), col10 year, col11 time, col12 decimal(10,2), col13 bit, col14 enum('tidb','pd','tikv','tiflash'), col15 set('one','two'), col16 varchar(10), primary key (col1,col2,col3,col4,col5,col6,col7,col8,col9,col10,col11,col12,col13,col14,col15,col16(2)) clustered);
mysql> insert into test.t_all values(1,1,1.2,1.2,'1','1','2020-01-01','2020-01-01 11:11:11','2020-01-01 11:11:11.123456','2020','11:11:11',12.20,b'1','tidb','','abc'),(-1,10,1.3,1.3,'10','10','2020-10-10','2020-10-10 22:22:22','2020-10-10 22:22:22.123456', '2013', '22:22:22', 13.34,b'0','tiflash','one,two','bcd');
mysql> alter table test.t_all set tiflash replica 1;
# todo add collation pk
Expand Down
4 changes: 2 additions & 2 deletions tests/clustered_index_fullstack/ddl.test
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@

## int handle
mysql> drop table if exists test.t_1
mysql> set tidb_enable_clustered_index=1;create table test.t_1(a int primary key, col int)
mysql> create table test.t_1(a int primary key clustered, col int)
mysql> insert into test.t_1 values(1,2),(2,3)
mysql> alter table test.t_1 set tiflash replica 1

# common handle
mysql> drop table if exists test.t_2
mysql> set tidb_enable_clustered_index=1;create table test.t_2(a varchar(10), b int, c int, primary key(a, b))
mysql> create table test.t_2(a varchar(10), b int, c int, primary key(a, b) clustered)
mysql> insert into test.t_2 values('1',2,3),('2',3,4)
mysql> alter table test.t_2 set tiflash replica 1

Expand Down
2 changes: 1 addition & 1 deletion tests/clustered_index_fullstack/issue_1514.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
mysql> drop table if exists test.t
mysql> set tidb_enable_clustered_index=1;create table test.t( col_20 time default '22:04:02.00' not null , col_21 smallint default 15900 , col_22 blob(273) , col_23 text not null , col_24 text(459) not null , primary key idx_12 ( col_24(5),col_21 ) , unique key idx_13 ( col_22(3),col_21 ) );
mysql> create table test.t( col_20 time default '22:04:02.00' not null , col_21 smallint default 15900 , col_22 blob(273) , col_23 text not null , col_24 text(459) not null , primary key idx_12 ( col_24(5),col_21 ) clustered, unique key idx_13 ( col_22(3),col_21 ) );
mysql> insert into test.t values ( '03:24:15.00',-27200,'XcqRrDOS','EpP','' ) ;
mysql> alter table test.t set tiflash replica 1

Expand Down
6 changes: 3 additions & 3 deletions tests/clustered_index_fullstack/query.test
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@

## int handle
mysql> drop table if exists test.t_1
mysql> set tidb_enable_clustered_index=1;create table test.t_1(a bigint primary key, col int)
mysql> create table test.t_1(a bigint primary key clustered, col int)
mysql> insert into test.t_1 values(-9223372036854775808,1),(9223372036854775807,2),(0,3)
mysql> alter table test.t_1 set tiflash replica 1

mysql> drop table if exists test.t_2
mysql> set tidb_enable_clustered_index=1;create table test.t_2(a bigint unsigned primary key, col int)
mysql> create table test.t_2(a bigint unsigned primary key clustered, col int)
mysql> insert into test.t_2 values(0,1),(18446744073709551615,2),(9223372036854775808,3)
mysql> alter table test.t_2 set tiflash replica 1

# common handle
mysql> drop table if exists test.t_3
mysql> set tidb_enable_clustered_index=1;create table test.t_3(a decimal(6,2), b bigint, c int, primary key(a, b))
mysql> create table test.t_3(a decimal(6,2), b bigint, c int, primary key(a, b) clustered)
mysql> insert into test.t_3 values(-9999.99, -9223372036854775808, 0),(9999.99, 9223372036854775807, 1),(12.21, 12, 2)
mysql> alter table test.t_3 set tiflash replica 1

Expand Down
1 change: 0 additions & 1 deletion tests/docker/config/tidb.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# TiDB Configuration.

host = "0.0.0.0"
alter-primary-key = true
3 changes: 0 additions & 3 deletions tests/docker/config/tidb_clustered_index.toml

This file was deleted.

7 changes: 0 additions & 7 deletions tests/docker/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,3 @@ wait_env dt
docker-compose -f cluster_new_collation.yaml -f tiflash-dt.yaml exec -T tiflash0 bash -c 'cd /tests ; ./run-test.sh new_collation_fullstack'
docker-compose -f cluster_new_collation.yaml -f tiflash-dt.yaml down
rm -rf ./data ./log

# run clustered index tests
docker-compose -f cluster_clustered_index.yaml -f tiflash-dt.yaml up -d
wait_env dt
docker-compose -f cluster_clustered_index.yaml -f tiflash-dt.yaml exec -T tiflash0 bash -c 'cd /tests ; ./run-test.sh clustered_index_fullstack'
docker-compose -f cluster_clustered_index.yaml -f tiflash-dt.yaml down
rm -rf ./data ./log

0 comments on commit 891466e

Please sign in to comment.