3 ms·

A safe MySQL upgrade that wasn't so safe

52 points by el1s7 1mo ago

Insimwytim 1mo ago
That's kinda fun (not in a fun way). Since the update is done semi-automatically by the means of AWS RDS, does AWS takes any responsibility for that? Unless this use case is warned against in migration notes, I would think they ought to.
krautburglar 1mo ago
Mixing binlog formats across replicas sounds like user error to me.
- rf15 1mo ago
  Or the use of AUTO_INCREMENT, a feature which is already a footgun if implemented badly (as it clearly was)
  - b112 1mo ago
    Agreed, and SQL is as important to fully understand, as when, for example, writing C. An inept usage of it, can lead to complete and total disaster. You wouldn't want a junior to write (unreviewed) an internet connected daemon. Or to try to meet a complex RFC spec. Yet SQL? Why not?! I think one of the greatest disservices people have done, is to abstract away SQL in frameworks. It certainly lets juniors more safely work with databases, but it really has reduced the general SQL knowledge out there. I see many senior programmers, with almost no exposure. Never touched it.
- setr 1mo ago
  Per the article, the replicas are configured the same. MIXED is just dynamically selecting the format automatically, and it seems to be the case that AUTOINCREMENT has fundamentally broken semantics under replication. And instead of erroring out when replicating, it instead chooses ROW format and plays a game of complete nonsense. The user error is in not sufficiently reading the docs, but it seems to me MySQL is going out of its way to wrap the noose
  - evanelias 1mo ago
    No, the binlog format is actually irrelevant here; the post author was just incorrect about that part. ALTER TABLE always gets replicated as a statement, regardless of the binlog format. Auto_increment only has broken replication semantics in the very specific situation the author encountered: a table has data, but no primary key (and also no alternative unique index which could serve as the clustered index key) and then an attempt is made to alter the table to add an auto_increment primary key. Basically that form of ALTER TABLE statement is telling the database to add a sequential ID to each existing row, but without providing any deterministic way that those numbers should be assigned. So each replica may choose a different numbering, causing the problem experienced by the author. It's a foot gun, but not a common one in production at any real scale where you'd have a replica in the first place. InnoDB tables really should always have primary keys, and there are various ways to ensure that happens (sql_require_primary_key option, generated invisible primary key option, external linters, etc).
    - el1s7 1mo ago
      Author here. I didn't make it clear in the article but the table in question did have a primary key, which was migrated to an unique key, and then the new auto-incremental primary key was added. As I mentioned in the article, the old ID field was then used in the update statement for the other tables. I'm updating the article now to make that part clear.
      - evanelias 1mo ago
        Interesting, I wouldn't have expected that auto_increment replication problem to occur for InnoDB tables that have a clustered index key (the old PK converted to UNIQUE), that is indeed surprising. Maybe the SQL layer does something dumb here and thinks the table doesn't have a defined ordering because it has no explicit PK -- even though internally in the storage engine it would still have the table organized by the old PK if it was a UNIQUE KEY over non-NULLable columns. But even that aside, I still say the binlog_format is irrelevant and the core problem here is 100% the ALTER to add the auto_increment: it resulted in different IDs on the replica than on the primary. That's a problem if you refer to IDs externally anywhere, regardless of whether it's 6 child tables or an external cache or logging etc. As soon as you promote a replica for any reason (not just an upgrade, any failover reason whatsoever) this would be a massive problem as all the IDs would now refer to different rows. And even before a failover event, if you do any reads from the replica for any purpose (read scaling, backups, OLAP queries), the data is going to be wrong. Essentially for the 6 child tables, it wouldn't have mattered if their UPDATEs had all used ROW or all used STATEMENT; either way you would have still had a fundamental data inconsistency between primary and replica here for the parent table. If these 6 tables' UPDATEs all used ROW, they would refer to the IDs from the primary which are locally "wrong" on the replica. Or if they all used STATEMENT then the data on the replica would be consistent locally but completely different than what's on the primary.
190n 1mo ago
I don't understand why this issue only appeared after the upgrade, not immediately after the migration to add the column?
- fipar 1mo ago
  I think it happened right after the alter table, but it was discovered after the upgrade. It's normal to have more eyes on the system after a DB upgrade, and also common to blame the DB for post-upgrade problems. Turns out this time the DB was to blame, but not because of the upgrade. If the alter table in the blog post is not a simplified version of what was executed (barring changing column names, of course), that means the table had no primary key before the migration, which is a problem on its own. To be honest, I don't even know if there's a safe way out of that situation in a replication setup, but one plan I would have tried to test in that situation is: - switch binlog_format to ROW (and never look back ...) - run a noop alter table to rebuild the table and hope that with ROW format, the rows get inserted in the same order (hope really hard please, with feeling) - run the alter table Fortunately, recent versions of MySQL have ROW as the default binlog_format.
  - evanelias 1mo ago
    I agree the root cause here is the lack of a primary key to begin with. But as far as I know, DDL is always replicated as just a statement, regardless of session binlog_format. So I believe the only real fix here is the general approach suggested in the manual [1], i.e. create a new empty table that has the auto_increment PK added and then populate it from the old table. [1] https://dev.mysql.com/doc/refman/9.7/en/replication-features-auto-increment.html https://dev.mysql.com/doc/refman/9.7/en/replication-features...
    - fipar 1mo ago
      Yeah, looking back at my first reply I did get the order wrong: first the alter, then the empty noop. But once you run the alter and have a primary key (though the author clarified the table did have a PK already), I’d just use pt-osc to run the empty noop as that would basically do what the manual says. Nice seeing you Evan! :)
mrkeen 1mo ago
Is there actually a way to 'be careful'? The moral of this story seems that you'll step on landmines. And perhaps that 'managed' databases aren't that managed.
- el1s7 1mo ago
  I agree, couldn't have said it better.
- mendapi 1mo ago
  [flagged]
exabrial 1mo ago
[dead]

52 points by el1s7 1mo ago

That's kinda fun (not in a fun way). Since the update is done semi-automatically by the means of AWS RDS, does AWS takes any responsibility for that? Unless this use case is warned against in migration notes, I would think they ought to.

krautburglar 1mo ago

Mixing binlog formats across replicas sounds like user error to me.

rf15 1mo ago
Or the use of AUTO_INCREMENT, a feature which is already a footgun if implemented badly (as it clearly was)
- b112 1mo ago
  Agreed, and SQL is as important to fully understand, as when, for example, writing C. An inept usage of it, can lead to complete and total disaster. You wouldn't want a junior to write (unreviewed) an internet connected daemon. Or to try to meet a complex RFC spec. Yet SQL? Why not?! I think one of the greatest disservices people have done, is to abstract away SQL in frameworks. It certainly lets juniors more safely work with databases, but it really has reduced the general SQL knowledge out there. I see many senior programmers, with almost no exposure. Never touched it.
setr 1mo ago
Per the article, the replicas are configured the same. MIXED is just dynamically selecting the format automatically, and it seems to be the case that AUTOINCREMENT has fundamentally broken semantics under replication. And instead of erroring out when replicating, it instead chooses ROW format and plays a game of complete nonsense. The user error is in not sufficiently reading the docs, but it seems to me MySQL is going out of its way to wrap the noose
- evanelias 1mo ago
  No, the binlog format is actually irrelevant here; the post author was just incorrect about that part. ALTER TABLE always gets replicated as a statement, regardless of the binlog format. Auto_increment only has broken replication semantics in the very specific situation the author encountered: a table has data, but no primary key (and also no alternative unique index which could serve as the clustered index key) and then an attempt is made to alter the table to add an auto_increment primary key. Basically that form of ALTER TABLE statement is telling the database to add a sequential ID to each existing row, but without providing any deterministic way that those numbers should be assigned. So each replica may choose a different numbering, causing the problem experienced by the author. It's a foot gun, but not a common one in production at any real scale where you'd have a replica in the first place. InnoDB tables really should always have primary keys, and there are various ways to ensure that happens (sql_require_primary_key option, generated invisible primary key option, external linters, etc).
  - el1s7 1mo ago
    Author here. I didn't make it clear in the article but the table in question did have a primary key, which was migrated to an unique key, and then the new auto-incremental primary key was added. As I mentioned in the article, the old ID field was then used in the update statement for the other tables. I'm updating the article now to make that part clear.
    - evanelias 1mo ago
      Interesting, I wouldn't have expected that auto_increment replication problem to occur for InnoDB tables that have a clustered index key (the old PK converted to UNIQUE), that is indeed surprising. Maybe the SQL layer does something dumb here and thinks the table doesn't have a defined ordering because it has no explicit PK -- even though internally in the storage engine it would still have the table organized by the old PK if it was a UNIQUE KEY over non-NULLable columns. But even that aside, I still say the binlog_format is irrelevant and the core problem here is 100% the ALTER to add the auto_increment: it resulted in different IDs on the replica than on the primary. That's a problem if you refer to IDs externally anywhere, regardless of whether it's 6 child tables or an external cache or logging etc. As soon as you promote a replica for any reason (not just an upgrade, any failover reason whatsoever) this would be a massive problem as all the IDs would now refer to different rows. And even before a failover event, if you do any reads from the replica for any purpose (read scaling, backups, OLAP queries), the data is going to be wrong. Essentially for the 6 child tables, it wouldn't have mattered if their UPDATEs had all used ROW or all used STATEMENT; either way you would have still had a fundamental data inconsistency between primary and replica here for the parent table. If these 6 tables' UPDATEs all used ROW, they would refer to the IDs from the primary which are locally "wrong" on the replica. Or if they all used STATEMENT then the data on the replica would be consistent locally but completely different than what's on the primary.

190n 1mo ago

I don't understand why this issue only appeared after the upgrade, not immediately after the migration to add the column?

fipar 1mo ago
I think it happened right after the alter table, but it was discovered after the upgrade. It's normal to have more eyes on the system after a DB upgrade, and also common to blame the DB for post-upgrade problems. Turns out this time the DB was to blame, but not because of the upgrade. If the alter table in the blog post is not a simplified version of what was executed (barring changing column names, of course), that means the table had no primary key before the migration, which is a problem on its own. To be honest, I don't even know if there's a safe way out of that situation in a replication setup, but one plan I would have tried to test in that situation is: - switch binlog_format to ROW (and never look back ...) - run a noop alter table to rebuild the table and hope that with ROW format, the rows get inserted in the same order (hope really hard please, with feeling) - run the alter table Fortunately, recent versions of MySQL have ROW as the default binlog_format.
- evanelias 1mo ago
  I agree the root cause here is the lack of a primary key to begin with. But as far as I know, DDL is always replicated as just a statement, regardless of session binlog_format. So I believe the only real fix here is the general approach suggested in the manual [1], i.e. create a new empty table that has the auto_increment PK added and then populate it from the old table. [1] https://dev.mysql.com/doc/refman/9.7/en/replication-features-auto-increment.html https://dev.mysql.com/doc/refman/9.7/en/replication-features...
  - fipar 1mo ago
    Yeah, looking back at my first reply I did get the order wrong: first the alter, then the empty noop. But once you run the alter and have a primary key (though the author clarified the table did have a PK already), I’d just use pt-osc to run the empty noop as that would basically do what the manual says. Nice seeing you Evan! :)

mrkeen 1mo ago

Is there actually a way to 'be careful'? The moral of this story seems that you'll step on landmines. And perhaps that 'managed' databases aren't that managed.

el1s7 1mo ago
I agree, couldn't have said it better.
mendapi 1mo ago
[flagged]

exabrial 1mo ago

[dead]