Add drop trigger to the whitelisted queries.

[camilo]

What is this?

This is an attempt at solving a very punctual problem, so I'm not even sure if this is something we want to add to the library. I can alternatively create a configuration bit and put it in the hands of the users ( that might just be even worse?).

In any case, the punctual problem is; some schema change strategies rely on triggers to be able to move "live data" as the schema changes. We have observed the DROP TRIGGER command get killed which can be dangerous (or at the very least very annoying) when cleaning after the schema has changed.

[camilo]

I'll add a test if we like this approach.

[sroysen]

Just to be clear. TL/DR: semian should not kill DDLs like trigger management, but I think that there is an underlying, previous issue with these failed LHMs.

We've seen this type of entries in the logs of the lhm runner:

Errored with exception ActiveRecord::StatementInvalid: Mysql2::CircuitOpenError: [mysql_shard_17] Semian::OpenCircuitError: drop trigger if exists `lhmt_del_payments_balance_transactions` /* large hadron migration */ /*application:Shopify,pod_id:17,role:rw,job:CrossPodJobs::PoddedLhmProcessingJob,job_id:0f46c753-0bb1-4d48-b54e-08ef755a98da*/

The command that semian is killing is drop trigger if exists...

What we are seeing with LHMs failing when trying to drop a trigger is a sign of something else not working or interfering with the cleanup process. The process for an LHM is:

1. Create the transient lhm_[table] with the new schema
2. Create the triggers for the LHM
3. Start copying the data from the original table to the lhm_[table]

If the LHM code detects any issues while running 3. it will automatically try to do a cleanup, where the first step is going to be to drop the triggers. Those are the drop trigger if exists statements that we are seeing now. That means that there was a previous issue with that LHM that is making it to try perform an automatic cleanup and drop the triggers.

I think that semian should make a distinction between DDLs (and other db management commands and actions) and regular queries. It should never interfere with DDL. So, for me this is something that should be added anyway.

[camilo]

This might still be desirable, I'll discuss and bring it back if we think it makes sense.