Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove catalog index price update by schedule subscription #870

Merged
merged 5 commits into from
Oct 25, 2019

Conversation

unicoder88
Copy link
Contributor

Hi there!

It saddens me to inform that I discovered a bug with update by schedule which results to a lot of reindex operations in Algolia.
In PR #666 , we subscribed to catalog_product_index_price in order to see changed catalog rule prices.
In Magento 2.3 this still works well when it comes to individual product updates, but it goes mad with full catalog_product_price reindex.

This is likely the reason behind reported issue https://github.com/algolia/algoliasearch-magento-2/issues/831 .

Steps to reproduce

  1. In Magento 2.3.1, install latest Algolia and switch all indexes to update by schedule mode.
    Initially algolia_products_cl is empty.
  2. Reindex prices, e.g. bin/magento indexer:reindex catalog_product_price
  3. Check algolia_products_cl and find that all products IDs were added - meaning full Algolia reindex!
  4. Truncate or delete records from algolia_products_cl, or note last record number.
  5. Reindex prices 2-nd time, e.g. bin/magento indexer:reindex catalog_product_price
  6. Check algolia_products_cl, and see that no new records were added.
  7. Reindex prices 3-rd time, e.g. bin/magento indexer:reindex catalog_product_price
  8. Check algolia_products_cl, and see that all product IDs were added again.

What happens:

Reason here is catalog_product_index_price_replica table.

In scheduled mode, triggers are originally created on catalog_product_index_price,
and catalog_product_index_price_replica is empty.

https://prnt.sc/pdzjdh
https://prnt.sc/pdzjl6

In this state (scenario A), full price reindex does following:

  1. truncates catalog_product_index_price_replica.
  2. fills it with fresh index data.
  3. switches catalog_product_index_price_replica vs catalog_product_index_price.
    During this switch, triggers follow renamed tables - triggers are now assigned to catalog_product_index_price_replica table.

https://prnt.sc/pdzjvw

In scenario A, no triggers are fired, even if some prices actually changed.

With switched table and triggers now assigned to _replica table, next full reindex does following:

  1. truncates catalog_product_index_price_replica. On delete triggers won't fire with TRUNCATE command.
  2. fills it with fresh index data. --- problem here!
    On insert triggers add every product in catalog to algolia_products_cl table. https://prnt.sc/pdzkhz
  3. switches catalog_product_index_price_replica vs catalog_product_index_price.
    Triggers follow renamed table and are now assigned to original catalog_product_index_price table - scneatio A.

And so with every full price reindex, scenario A - no updates at all - alternates with scneario B - all products are added to changelog.

Full price index update can happen quite often, and some modules even invalidate this index on every product save (Amasty_Shopby)!

I have not come up with a proper solution here yet, so we had to install an alternative module that makes a plugin to catalog rules reindexing and detects product price changes, and then inserts changed product IDs into algolia_products_cl table manually. But this is not generic enough.

Cleaning up

Removing subscription from mview.xml doesn't seem to delete evil triggers, so I had to come up with schema migration. Should any other unfortunate code subscribe to the same index, the triggers will get recreated on next setup:upgrade recurring run.

What do you guys think?

unicoder88 and others added 2 commits October 2, 2019 21:13
In Magento 2.3, every second full price index reindex results in reindexing of
all products in Algolia
@damcou damcou requested a review from bsuravech October 10, 2019 14:14
@damcou damcou added this to the 1.12.1 milestone Oct 10, 2019
@bsuravech bsuravech self-assigned this Oct 10, 2019
Copy link
Contributor

@bsuravech bsuravech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unicoder88 Thanks for the correction and for submitting. I've left a comment on the dropTriggers portion of the PR. It's unlikely there is another subscription on this table but we don't want to explicitly drop a trigger if there are other mview subscriptions there.

Setup/UpgradeSchema.php Outdated Show resolved Hide resolved
@bsuravech bsuravech assigned unicoder88 and unassigned bsuravech Oct 15, 2019
@bsuravech bsuravech merged commit 6167220 into algolia:develop Oct 25, 2019
vmalyk added a commit to ConvertGroupsAS/algoliasearch-magento-2 that referenced this pull request Oct 26, 2019
@peterjaap
Copy link
Contributor

@bsuravech @damcou @unicoder88 although I agree this PR in itself is good, it's not a fix in the sense that it fixes the underlying problem, right?

How are products which prices are updated because of catalog price rules now added to the queue for resyncing?

@peterjaap
Copy link
Contributor

@bsuravech still unsure about this one - it seems to me there's still no valid solution for a price rule kicking in not updating the products in Algolia, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants