refactor: data sync performance optimisation #634
Conversation
Add category resource to minimise a need to request category information for each category item, instead relying on full category tree names snapshot.
Prevent data sync from triggering in the context of `setup:upgrade` command, instead being conducted only in the context of dedicated `dotdigital:migrate` command.
Prevent pending automation processing within `setup:upgrade` command context. To be handled instead within normal automation processing context.
Minor code refactoring better adhering to methods signatures (e.g. in Consent.php try{}catch block).
Add logger in Order.php since no explicit dependency injected but used in the business logic implementation.
| @@ -39,7 +39,7 @@ public function __construct( | |||
| */ | |||
| public function apply() | |||
| { | |||
| $this->automationFactory->create()->sync(); | |||
| //$this->automationFactory->create()->sync(); | |||
There was a problem hiding this comment.
The assumption is that this model is not used by CLI/cron job and this change won't prevent automation processing.
There was a problem hiding this comment.
This patch was added when we introduced queues for automation processing. It was possible at point of upgrade that a merchant still has some residual automations in the table (up to 15 mins' worth) which would not be queued and therefore left to become stale. Without this merchants would need to remember / be reminded to do this to process remaining old-style automations.
| if (count($names)) { | ||
| return implode(',', $names); | ||
| if (!$this->connectorCategoryModel) { | ||
| $this->connectorCategoryModel = ObjectManager::getInstance()->get(ConnectorCategoryModel::class); |
There was a problem hiding this comment.
Curious why this is necessary if the class was injected in the constructor in the usual way?
There was a problem hiding this comment.
Hi @sta1r
That snippet was maintained there since the original solution was applied as a patch. You are right, DI should suffice and the use of objectManager to be deleted/discouraged.
|
@pavelshulga plenty of good improvements here which we will aim to include in the coming sprints. 👍 |
|
@pavelshulga We have released some performance improvements for the Email module in v4.28.0. These include your suggestion of preloading the category tree. The most beneficial changes came with adding caching for loaded products and other entities. Please give these a try in due course and do keep the suggestions and feedback coming. Please note, we did try out your index additions but did not find improvements in sync or query times. We have a wider audit of our indexes scheduled in 2025 so we'll come back to this. |
setup:upgradecommand, instead being conducted only in the context of dedicateddotdigital:migratecommand.setup:upgradecommand context. To be handled instead within normal automation processing context.