← Back to team overview

maria-developers team mailing list archive

Re: fc42991: MDEV-7901: re-implement analyze table for low impact


Hi, Sergei!

On 03.08.2016 12:30, Sergei Golubchik wrote:
Hi, Oleksandr!

Looks mostly ok, see few comments below

On Aug 01, Oleksandr Byelkin wrote:
revision-id: fc42991720838e165ad448b6707602cded92faa4 (mariadb-10.2.1-9-gfc42991)
parent(s): 08683a726773f8cdf16a4a3dfb3920e5f7842481
committer: Oleksandr Byelkin
timestamp: 2016-08-01 19:25:45 +0200

MDEV-7901: re-implement analyze table for low impact

Table before collecting engine independent statistics now is reopened in read mode,
InnoDB allow write operations in this case.

  mysql-test/r/stat_tables_innodb_debug.result |  27 +++
  mysql-test/t/stat_tables_innodb_debug.test   |  37 ++++
  sql/sql_admin.cc                             | 295 +++++++++++++++------------
  storage/innobase/handler/ha_innodb.cc        |   1 +
  storage/xtradb/handler/ha_innodb.cc          |   2 +-
  5 files changed, 227 insertions(+), 135 deletions(-)

diff --git a/mysql-test/r/stat_tables_innodb_debug.result b/mysql-test/r/stat_tables_innodb_debug.result
new file mode 100644
index 0000000..b02848d
--- /dev/null
+++ b/mysql-test/r/stat_tables_innodb_debug.result
@@ -0,0 +1,27 @@
+# MDEV-7901: re-implement analyze table for low impact
+create table t1(a int primary key) engine=innodb;
+insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
you use t1 only to populate t2. You don't need to create a table for
thatr, use seq_0_to_9 - https://mariadb.com/kb/en/mariadb/sequence/
It adds dependance on presence of the engine, but I hope it is not a big limitation (done).

+create table t2 (a int, b int, c int, key IDX1(a), key IDX2(a,b)) engine=innodb;
+insert into t2 select a/10, a/2, a from test.t1;
+SET DEBUG_SYNC='statistics_collection_start1 SIGNAL analyzing WAIT_FOR written';
+analyze table t2 persistent for all;
+connect  con1, localhost, root,,;
+connection con1;
+SET DEBUG_SYNC= 'now WAIT_FOR analyzing';
+select count(*) from t2;
+insert into t2 values (333,333,333);
+update t2 set a=1;
+SET DEBUG_SYNC= 'now SIGNAL written';
+connection default;
+Table	Op	Msg_type	Msg_text
+test.t2	analyze	status	Engine-independent statistics collected
+test.t2	analyze	status	OK
+select count(*) from t2;
+set debug_sync='RESET';
+drop table t1,t2;
good direct test

diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
index 578b78d..933f3d0 100644
--- a/sql/sql_admin.cc
+++ b/sql/sql_admin.cc
@@ -812,7 +773,73 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
if (compl_result_code == HA_ADMIN_OK && collect_eis)
ok, I've quickly checked the code and I think (not sure) that there is
no need to reopen for innodb. But as you do, please, show the benefits
of it - add a test case for MyISAM too (remember, TL_READ allows
concurrent inserts).

besides, this reopening code needs a comment explaining why you're doing
Test & comment added.

+      enum_sql_command save_sql_command= lex->sql_command;
+      trans_commit_stmt(thd);
+      trans_commit(thd);
+      thd->open_options|= extra_open_options;
+      close_thread_tables(thd);
+      table->table= NULL;
+      thd->mdl_context.release_transactional_locks();
+      lex->sql_command= save_sql_command;
where is lex->sql_command changed?
no, I tought I removed it, but it is done now.

+      table->mdl_request.init(MDL_key::TABLE, table->db, table->table_name,
+                              MDL_SHARED_NO_READ_WRITE, MDL_TRANSACTION);
+      table->mdl_request.set_type(MDL_SHARED_READ);
+      table->lock_type= TL_READ;
+      open_error= open_only_one_table(thd, table,
+                                      repair_table_use_frm,
+                                      (view_operator_func != NULL));
can one analyze a view? how does that work?
No. actually it is prohibited before we open table first time. I put DBUG_ASSERT here just in case and changed the call parameter.

+      thd->open_options&= ~extra_open_options;

Chief Architect MariaDB
and security@xxxxxxxxxxx

Mailing list: https://launchpad.net/~maria-developers
Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp