Make pt-osc compatible with AFTER triggers
General
Escalation
General
Escalation
Description
Environment
None
Smart Checklist
Activity
Sveta Smirnova July 28, 2017 at 6:57 PM
Works fine for me:
// Session 1
mysql> create table t1(id int not null auto_increment primary key, f1 int);
Query OK, 0 rows affected (0.01 sec)
mysql> insert into t1(f1) values(1),(2),(3),(4),(5),(6),(7),(8),(9),(0);
Query OK, 10 rows affected (0.01 sec)
Records: 10 Duplicates: 0 Warnings: 0
mysql> create trigger t1ai for each row \c
mysql> create table t1_log(f1 int, f1_old int);
Query OK, 0 rows affected (0.01 sec)
mysql> create trigger t1ai for each row insert into t1_log values(NULL, NEW.f1);
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'for each row insert into t1_log values(NULL, NEW.f1)' at line 1
mysql> create trigger t1ai after insert on t1 for each row insert into t1_log values(NULL, NEW.f1);
Query OK, 0 rows affected (0.02 sec)
mysql> create table t1_log2(f1 int, f1_old int);
Query OK, 0 rows affected (0.01 sec)
mysql> create trigger t1bi before insert on t1 for each row insert into t1_log2 values(NULL, NEW.f1);
Query OK, 0 rows affected (0.02 sec)
mysql> create trigger t1au after insert on t1 for each row insert into t1_log values(OLD.f1, NEW.f1);
ERROR 1363 (HY000): There is no OLD row in on INSERT trigger
mysql> create trigger t1au after update on t1 for each row insert into t1_log values(OLD.f1, NEW.f1);
Query OK, 0 rows affected (0.02 sec)
mysql> create trigger t1bu before update on t1 for each row insert into t1_log2 values(OLD.f1, NEW.f1);
Query OK, 0 rows affected (0.01 sec)
mysql> create trigger t1ad after delete on t1 for each row insert into t1_log values(OLD.f1, NULL);
Query OK, 0 rows affected (0.02 sec)
mysql> create trigger t1bd before delete on t1 for each row insert into t1_log2 values(OLD.f1, NULL);
Query OK, 0 rows affected (0.02 sec)
mysql> insert into t1(f1) values(10);
Query OK, 1 row affected (0.00 sec)
mysql> select * from t1;
+----+------+
| id | f1 |
+----+------+
| 1 | 1 |
| 2 | 2 |
| 3 | 3 |
| 4 | 4 |
| 5 | 5 |
| 6 | 6 |
| 7 | 7 |
| 8 | 8 |
| 9 | 9 |
| 10 | 0 |
| 11 | 10 |
+----+------+
11 rows in set (0.00 sec)
mysql> update t1 set f1=20 where id=11;
Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0
mysql> delete from t1 where id=11;
Query OK, 1 row affected (0.01 sec)
mysql> select * from t1_LOG;
ERROR 1146 (42S02): Table 'test.t1_LOG' doesn't exist
mysql> select * from t1_log;
+------+--------+
| f1 | f1_old |
+------+--------+
| NULL | 10 |
| 10 | 20 |
| 20 | NULL |
+------+--------+
3 rows in set (0.00 sec)
mysql> select * from t1_log2;
+------+--------+
| f1 | f1_old |
+------+--------+
| NULL | 10 |
| 10 | 20 |
| 20 | NULL |
+------+--------+
3 rows in set (0.00 sec)
// Session 2
sveta@delly:~/src/percona-toolkit$ ~/build/percona-toolkit/bin/pt-online-schema-change --preserve-triggers --alter="add column f2 int" h=127.0.0.1,P=13000,u=root,D=test,t=t1 --execute
No slaves found. See --recursion-method if host delly has slaves.
Not checking slave lag because no slaves were found and --check-slave-lag was not specified.
Operation, tries, wait:
analyze_table, 10, 1
copy_rows, 10, 0.25
create_triggers, 10, 1
drop_triggers, 10, 1
swap_tables, 10, 1
update_foreign_keys, 10, 1
Altering `test`.`t1`...
Creating new table...
Created new table test._t1_new OK.
Altering new table...
Altered `test`.`_t1_new` OK.
2017-07-28T21:54:12 Creating triggers...
2017-07-28T21:54:12 Created triggers OK.
2017-07-28T21:54:12 Copying approximately 10 rows...
2017-07-28T21:54:12 Copied rows OK.
2017-07-28T21:54:12 Adding original triggers to new table.
2017-07-28T21:54:13 Analyzing new table...
2017-07-28T21:54:13 Swapping tables...
2017-07-28T21:54:13 Swapped original and new tables OK.
2017-07-28T21:54:13 Dropping old table...
2017-07-28T21:54:13 Dropped old table `test`.`_t1_old` OK.
2017-07-28T21:54:13 Dropping triggers...
2017-07-28T21:54:13 Dropped triggers OK.
Successfully altered `test`.`t1`.
// Session 1
mysql> show triggers;
+---------+--------+-------+--------------------------------------------+--------+------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------+----------------------+----------------------+--------------------+
| Trigger | Event | Table | Statement | Timing | Created | sql_mode | Definer | character_set_client | collation_connection | Database Collation |
+---------+--------+-------+--------------------------------------------+--------+------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------+----------------------+----------------------+--------------------+
| t1bi | INSERT | t1 | insert into t1_log2 values(NULL, NEW.f1) | BEFORE | 2017-07-28 21:54:13.04 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
| t1ai | INSERT | t1 | insert into t1_log values(NULL, NEW.f1) | AFTER | 2017-07-28 21:54:12.94 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
| t1bu | UPDATE | t1 | insert into t1_log2 values(OLD.f1, NEW.f1) | BEFORE | 2017-07-28 21:54:13.00 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
| t1au | UPDATE | t1 | insert into t1_log values(OLD.f1, NEW.f1) | AFTER | 2017-07-28 21:54:12.92 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
| t1bd | DELETE | t1 | insert into t1_log2 values(OLD.f1, NULL) | BEFORE | 2017-07-28 21:54:12.96 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
| t1ad | DELETE | t1 | insert into t1_log values(OLD.f1, NULL) | AFTER | 2017-07-28 21:54:12.89 | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION | root@localhost | utf8 | utf8_general_ci | latin1_swedish_ci |
+---------+--------+-------+--------------------------------------------+--------+------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------+----------------------+----------------------+--------------------+
6 rows in set (0.01 sec)
mysql>
mysql> show create table t1;
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table |
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| t1 | CREATE TABLE `t1` (
`id` int(11) NOT NULL AUTO_INCREMENT,
`f1` int(11) DEFAULT NULL,
`f2` int(11) DEFAULT NULL,
PRIMARY KEY (`id`)
) ENGINE=MyISAM AUTO_INCREMENT=12 DEFAULT CHARSET=latin1 |
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.01 sec)Carlos Salguero July 17, 2017 at 6:13 PM
Since we still have time before releasing this, I'll ask some DBAs to also test it.
Carlos Salguero July 6, 2017 at 4:02 AM
I have these tests:
ok 136 - Basic --preserve-triggers exit 0
ok 137 - Basic --preserve-triggers tables
ok 138 - Basic --preserve-triggers rows
ok 139 - Basic --preserve-triggers triggers still exist
ok 140 - Basic --preserve-triggers MAX(pk_col)
ok 141 - Basic --preserve-triggers AUTO_INCREMENT=20000
ok 142 - Basic --preserve-triggers foo added
ok 143 - --preserve-triggers: after triggers exit 0
ok 144 - --preserve-triggers: after triggers tables
ok 145 - --preserve-triggers: after triggers rows
ok 146 - --preserve-triggers: after triggers triggers still exist
ok 147 - --preserve-triggers: after triggers MAX(pk_col)
ok 148 - --preserve-triggers: after triggers AUTO_INCREMENT=9
ok 149 - --preserve-triggers: after triggers foo3 added
ok 150 - --preserve-triggers cannot drop column used by trigger
ok 151 - --preserve-triggers: message if try to drop a field used by triggers
ok 152 - --preserve-triggers --no-swap-tables exit status
ok 153 - --preserve-triggers --no-drop-old-table exit status
ok 154 - --preserve-triggers --no-drop-old-table original & new tables still exists
ok 155 - --preserve-triggers cannot be used --no-drop-triggersAnything missing?
Kenny Gryp July 4, 2017 at 8:53 PM
@Carlos Salguero: on your updated acceptance criteria.
Why would --no-drop-old-table not work? We can keep the old table and only remove the trigger.
As extra feature I would also maybe print out the existing trigger definition that was removed, so when the old table remains and needs to be brought back, the user can manually copy paste the trigger definition and put it back.
Vadim Tkachenko June 27, 2017 at 10:17 PM
I agree to make this only 5.7 feature. It is very complicated and problematic to support 5.6
Done
Created March 3, 2017 at 2:55 AM
Updated December 12, 2017 at 12:47 PM
Resolved July 17, 2017 at 6:13 PM
Currently pt-online-schema-change cannot run if table already has AFTER triggers. But it can, if add an option which instruct the tool to do following (see pseudo-code):
The workflow to add triggers would be as follows (from our own experiences doing this with trigger-based tools):
lock the table in question (standard mysql lock tables write)
pull the existing trigger code, call that block 1
log trigger code into temporary file to avoid trigger definition loss in case of pt-osc failure
generate the trigger code for pt-osc, call that block 2
drop the existing trigger
create a new trigger with block 1 and block 2 appended, wrap them in /* BLOCK 1 START */ comments or similar if that makes it easier to parse out
repeat for other events (cover all of ins,upd,del)
unlock table
To remove triggers, the reverse:
lock the table with write lock
pull the trigger code and parse out block 1
drop the trigger
create the trigger with only code from block 1
repeat for other event
unlock table
Do not make this option default and add a warning about database can be in inconsistent state if the tool or MySQL Server is killed during execution.
Launchpad: [https://bugs.launchpad.net/percona-toolkit/+bug/1491133
]
Acceptance Criteria for params:
Scenario The --preserve-triggers feature needs to delete the old table and triggers and recreate them into for new table. Since it is not possible to have duplicated trigger names, the old table and triggers MUST be deleted and recreated Feature: --preserve and --no-drop-old-table params When --preserve-triggers param is specified And --no-drop-old-table was specified Then Exit with an error message Feature: --preserve triggers and --no-swap-tables params When --preserve-triggers param is specified And --no-swap-tables was specified Then exit with an error message Feature: --preserve triggers and --no-drop-triggers params When --preserve-triggers param is specified And --no-drop-triggers was specified Then exit with an error message