Magento-lts: All tables should have Primary Keys for Group replication in SQL

Created on 8 Sep 2020  路  6Comments  路  Source: OpenMage/magento-lts

Summary (*)

When hosting OpenMage on Digital Ocean you get sent the following email daily (which is really annoying).

Hello,

Our systems have indicated that your MySQL cluster, db-mysql-fra1-xxx, has tables without primary keys. We have identified that MySQL tables without primary keys can lead to service replication issues that jeopardize performance and availability. If primary keys are not present for database tables exceeding 5,000 rows, data-loss can occur.

Examples (*)


The following tables don't have primary keys:

api2_acl_user
api_session
catalog_category_anc_categs_index_idx
catalog_category_anc_categs_index_tmp
catalog_category_anc_products_index_idx
catalog_category_anc_products_index_tmp
catalog_category_product_index_enbl_idx
catalog_category_product_index_enbl_tmp
catalog_category_product_index_idx
catalog_category_product_index_tmp
log_url
m_db
oauth_nonce
weee_discount
widget_instance_page_layout

Proposed solution

1.) Identify unique combinations of columns and create PKs on them
2.) Where uniqueness cannot be guaranteed create an update script that adds new PK column.
3.) Test and alter all CRUD operations to be aware of the new PK especially if they are new columns rather than keys simply added to existing columns.

Most helpful comment

I wonder why catalog_category_product_index_idx doesn't have a unique key. Its only index covers product_id, category_id, and store_id. I can't see that as having any additional columns or multiple values. Changing that to a unique might make it perform better.

EDIT: it's empty for me... so, I won't see any performance improvements. Not sure what it does since it is empty.

All 6 comments

I did a naive trial run on this problem after adding the fake_pk columns and reindexing we get the following error

Insert value list does not match column list: 1136 Column count doesn't match value count at row 1, query was: INSERT INTO catalog_category_product_index (category_id, product_id, position, is_parent, store_id, visibility) SELECT catalog_category_product_index_idx.category_id, catalog_category_product_index_idx.product_id, catalog_category_product_index_idx.position, catalog_category_product_index_idx.is_parent, catalog_category_product_index_idx.store_id, catalog_category_product_index_idx.visibility, catalog_category_product_index_idx.fake_pk FROM catalog_category_product_index_idx ON DUPLICATE KEY UPDATE category_id = VALUES(category_id), product_id = VALUES(product_id), position = VALUES(position), is_parent = VALUES(is_parent), store_id = VALUES(store_id), visibility = VALUES(visibility)

I notice that catalog_category_product_index has a three column product key but the *_idx and *_tmp tables that the index are writing into ie catalog_category_product_index_idx and catalog_category_product_index_tmp have the three needed columns index set to non-unique making them unsuitable for a primary key in those temp tables, so this requires further thought.

Looking into this further I've found that whilst MySQL tables do not necessarily need a primary key[1], if you are using group replication it is needed [2]. So Digital Ocean are right to keep warning... :(

[1] https://dev.mysql.com/doc/refman/5.7/en/create-table.html#create-table-indexes-keys
[2] https://dev.mysql.com/doc/refman/5.7/en/group-replication-requirements.html

image
this can just be changed from a unique to a PK. Check each for that. Then I'd split this up so that you can do those that have just a change from unique keys to PKs and those that need more work.

I wonder why catalog_category_product_index_idx doesn't have a unique key. Its only index covers product_id, category_id, and store_id. I can't see that as having any additional columns or multiple values. Changing that to a unique might make it perform better.

EDIT: it's empty for me... so, I won't see any performance improvements. Not sure what it does since it is empty.

Sounds like a great addition, when doing this changes, please take the Branch 20.0 as target

@Flyingmana assign it to me and I'll crack on with it... 馃憤

Was this page helpful?
0 / 5 - 0 ratings