Improve post-checkout indexing

[bob2021]

This is part of a group of PRs containing the changes discussed in issue #152

There are a few issues with Mage_CatalogInventory_Model_Observer::reindexQuoteInventory():

• Exceptions aren't caught

  • Throwing an exception here will roll back the inventory, but it is too late as the order has already been placed. EDIT: No longer true as Safer DB transactions #187 has been merged
  • Creates a poor user experience - it makes no difference to the customer if indexing failed

• Stock & price indexer resource models are accessed directly

  • No locking - causes duplicate insert errors
  • No mode check - the indexers run even if in manual mode

• Stock item models are saved for low/OOS stock items

  • Stock data is loaded before quote conversion - if a payment authorisation takes 5 seconds, then the inventory data being saved is at least 5 seconds old.
  • Triggers re-indexing that isn't always necessary
  • Can cause a deadlock on index_* tables

This PR:

• Catches & logs exceptions
• Adds indexer locking
• Adds indexer mode check
• Only runs indexers when necessary
• Updates inventory data using Mage_CatalogInventory_Model_Resource_Stock

Possible breaking changes:

• Stock item model is not saved, so the related events are not fired
• These indexers no longer run: catalog_product_attribute, catalogsearch_fulltext
  • To hide an OOS product on the frontend it only needs to be removed from the price index. The other indexes will be cleaned up when saving the stock item model or running a full reindex.

I don't really like the way this solution duplicates the logic from the stock item model and accesses so many resource methods, but it gives maximum control over the indexing that occurs, and ensures the inventory data is kept consistent.

[bob2021]

There are some problems with this PR, I'll post an update in the next few days.

[bob2021]

Actually I think its ok, but I've updated the description to clarify exactly what has changed and the potential issues.

[andreasemer]

Currently i see a problem in the function reindexQuoteInventory with other extensions for example for FPC or Varnish, without hacking they don't flush the specific product cache.

I have tested it with https://github.com/GordonLesti/Lesti_Fpc

[googlygoo]

Looking at what is done with Index'ing in you work. I have a concern that the data on the store with be completely out of sync therefore giving the customers the wrong information if the index mode is either Mage_Index_Model_Process::MODE_MANUAL or Mage_Index_Model_Process::MODE_SCHEDULE. Therefor I feel that no matter what the index mode is set too, in this case is should always run as MODE_REAL_TIME to give the customer the best experience.

[bob2021]

I think that's a valid argument, the main reason it ended up in this PR is that while I was trying to debug some of the other problems mentioned, I found that some of the indexers were still running even though I thought I had disabled them by setting them to manual mode - so from a developers perspective, the behaviour is inconsistent.

From a customer/store owners perspective (and for backwards compatibility) it probably does make sense to force the indexing to take place.

Though I think there are still benefits from the other features in this PR (indexer locking and conditional price indexing)

[nachores]

Hi there,

I'm testing this solution and I have a doubt. If I switch stock and price indexes to manual mode, using your approach, reindex is not done during checkout, ok. My concern is that the stock index doesn't show "Update is required" in the admin. Then, I cancel that order and this time the index shows that it needs to be updated.

Even setting the index to update on save, the index doesn't show "update is required". Does it make sense to you guys?

[bob2021]

During checkout the indexer resource methods are called directly, which bypasses the index event system that normally triggers the "update is required" status.

Cancelling an order doesn't bypass the index event system, so the "update is required" status is triggered as normal.

However, for the reasons @googlygoo mentioned, it is not usually desirable to disable the real time stock and price indexing.

[nachores]

Yes and I see a problem in that bypass. I have chosen Manual Mode so it will not run, good, but at least you should be aware of it ("Update is required"), Don't you think so? If you are aware of it, you can just update it manually or in my case, with a partial scheduled reindex tool. I'll be working on that.

I agree with @googlygoo, but as you said, for those who have configured manual mode, this is very confusing. Moreover, not saving the stock item model has also side effects, as you also mention, in user experience (layered navigation , search, ...).

[bob2021]

I am no longer involved with Magento development and I will not be contributing any further changes to this PR so I am closing it.

[addison74]

I understand the situation but a simple statement that you are no longer involved in the project was enough without closing the PR, leaving it to the appreciation of others who are still active if your work so far can be useful OpenMage.

[fballiano]

since there were many doubts in the comments of this PR, the author is no longer in the openmage world, there's a conflict and it would need a rebase too... I'll close it for the time being.