Fix best practice "Asynchronous order data processing" for auto invoiced orders #36224

convenient · 2022-09-30T11:09:06Z

Description

Configuring order grid async indexing is meant to be a pretty quick win.

Configuring this value means we can still take orders and let the cron job be responsible for populating the admin tables, but unfortunately it only works when an order is not automatically invoiced during creation (so for us this breaks when used with paypal/stripe/etc)

There can be times when intensive sales on a storefront occur at the same time that Commerce is performing intensive order processing. You can configure Commerce to distinguish these two traffic patterns on the database level to avoid conflicts between read and write operations in the corresponding tables. You can store and index order data asynchronously. Orders are placed in temporary storage and moved in bulk to the Order Management grid without any collisions. You can activate this option from Stores > Settings > Configuration > Advanced > Developer > Grid Settings > Asynchronous indexing.

https://experienceleague.adobe.com/docs/commerce-operations/performance-best-practices/configuration.html#asynchronous-order-data-processing

Toggling the setting dev/grid/async_indexing=1 is meant to ensure we do not populate sales_order_grid, sales_invoice_grid, and other grid tables during the synchronous request in which the order is being placed as any failure to insert into these admin panel grids could cause the actual order to fail and rollback with an error like so

[2022-09-23 19:08:02] report.CRITICAL: Saving order ABC123 failed: SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock; try restarting transaction, query was: INSERT INTO `sales_invoice_grid` (`entity_id`, `increment_id`, `state`, `store_id`, `store_name`, `order_id`, `order_increment_id`, `order_created_at`, `customer_name`, `customer_email`, `customer_group_id`, `payment_method`, `store_currency_code`, `order_currency_code`, `base_currency_code`, `global_currency_code`, `billing_name`, `billing_address`, `shipping_address`, `shipping_information`, `subtotal`, `shipping_and_handling`, `base_grand_total`, `grand_total`, `created_at`, `updated_at`) SELECT sales_invoice.entity_id AS `entity_id`, sales_invoice.increment_id AS `increment_id`, sales_invoice.state AS `state`, sales_invoice.store_id AS `store_id`, sales_order.store_name AS `store_name`, sales_invoice.order_id AS `order_id`, sales_order.increment_id AS `order_increment_id`, sales_order.created_at AS `order_created_at`, TRIM(CONCAT_WS(' ', IF(`sales_order`.`customer_firstname` <> '', `sales_order`.`customer_firstname`, NULL), IF(`sales_order`.`customer_lastname` <> '', `sales_order`.`customer_lastname`, NULL))) AS `customer_name`, sales_order.customer_email AS `customer_email`, sales_order.customer_group_id AS `customer_group_id`, sales_order_payment.method AS `payment_method`, sales_invoice.store_currency_code AS `store_currency_code`, sales_invoice.order_currency_code AS `order_currency_code`, sales_invoice.base_currency_code AS `base_currency_code`, sales_invoice.global_currency_code AS `global_currency_code`, TRIM(CONCAT_WS(' ', IF(`sales_billing_address`.`firstname` <> '', `sales_billing_address`.`firstname`, NULL), IF(`sales_billing_address`.`lastname` <> '', `sales_billing_address`.`lastname`, NULL))) AS `billing_name`, TRIM(CONCAT_WS(',', IF(`sales_billing_address`.`company` <> '', `sales_billing_address`.`company`, NULL), IF(`sales_billing_address`.`street` <> '', `sales_billing_address`.`street`, NULL), IF(`sales_billing_address`.`city` <> '', `sales_billing_address`.`city`, NULL), IF(`sales_billing_address`.`region` <> '', `sales_billing_address`.`region`, NULL), IF(`sales_billing_address`.`postcode` <> '', `sales_billing_address`.`postcode`, NULL))) AS `billing_address`, TRIM(CONCAT_WS(',', IF(`sales_shipping_address`.`company` <> '', `sales_shipping_address`.`company`, NULL), IF(`sales_shipping_address`.`street` <> '', `sales_shipping_address`.`street`, NULL), IF(`sales_shipping_address`.`city` <> '', `sales_shipping_address`.`city`, NULL), IF(`sales_shipping_address`.`region` <> '', `sales_shipping_address`.`region`, NULL), IF(`sales_shipping_address`.`postcode` <> '', `sales_shipping_address`.`postcode`, NULL))) AS `shipping_address`, sales_order.shipping_description AS `shipping_information`, sales_invoice.base_subtotal AS `subtotal`, sales_invoice.base_shipping_amount AS `shipping_and_handling`, sales_invoice.base_grand_total AS `base_grand_total`, sales_invoice.grand_total AS `grand_total`, sales_invoice.created_at AS `created_at`, sales_invoice.updated_at AS `updated_at` FROM `sales_invoice`

I haven't traced this back fully through the git history, but I believe this has been broken for a while.

To reproduce and testing

I have reproduced this on a vanilla installation of 2.4.5, you need a payment method which automatically creates the invoice when placing the order (for us on production this is Paypal/Stripe/etc) but for these test purposes I'll use the free checkout option.

The configuration

Configure async indexing
Enable free shipping
Set the "New Order Status" for "Zero Subtotal Checkout" to be "Processing" so that an invoice is automatically created.

Also you should create a simple product that is in stock for purchasing, with a price of 0.00.

For the purposes of a quick test (and because I don't know if you have your crons running in your Magento internal test instances) I hacked up the grid reindexing code to throw an exception, we should be able to place the order and see the success page, verifying that we never try to reindex the grid during the place order action.

--- vendor/magento/module-sales/Model/ResourceModel/Grid.php    2022-09-30 10:38:38.000000000 +0100
+++ vendor/magento/module-sales/Model/ResourceModel/Grid.php    2022-09-30 10:38:44.000000000 +0100
@@ -102,6 +102,8 @@
      */
     public function refresh($value, $field = null)
     {
+        throw new \Exception('Luker - triggering indexer refresh');
+
         $select = $this->getGridOriginSelect()
             ->where(($field ?: $this->mainTableName . '.entity_id') . ' = ?', $value);
         $sql = $this->getConnection()

To reproduce the issue

Add your free product to basket
Proceed through the checkout
Place a free order by picking free shipping

Expected result:

See the success page
no entries in sales_order_grid or sales_invoice_grid for your order

Actual result:

I see my exception, meaning we tried to reindex the grids during the placing of the order which should never happen

screen-recording-2022-09-30-at-120351_t3skb0cQ.mp4

When you take this approach you can see in the logs a stack trace showing the piece of code that is attempting to synchronously reindex the grid tables despite the flag being set.

[2022-09-30T10:04:08.293258+00:00] main.CRITICAL: Saving order 000000004 failed: Luker - triggering indexer refresh [] []
[2022-09-30T10:04:09.334806+00:00] main.CRITICAL: Exception: Luker - triggering indexer refresh in /2.4.5/vendor/magento/module-sales/Model/ResourceModel/Grid.php:105
Stack trace:
#0 /2.4.5/vendor/magento/module-sales/Model/ResourceModel/GridPool.php(38): Magento\Sales\Model\ResourceModel\Grid->refresh('4', 'sales_order.ent...')
#1 /2.4.5/vendor/magento/module-sales/Model/Order/Invoice/Plugin/AddressUpdate.php(73): Magento\Sales\Model\ResourceModel\GridPool->refreshByOrderId('4')
#2 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(146): Magento\Sales\Model\Order\Invoice\Plugin\AddressUpdate->afterProcess(Object(Magento\Sales\Model\ResourceModel\Order\Handler\Address\Interceptor), Object(Magento\Sales\Model\ResourceModel\Order\Handler\Address\Interceptor), Object(Magento\Sales\Model\Order\Interceptor))
#3 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Sales\Model\ResourceModel\Order\Handler\Address\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Sales\Model\Order\Interceptor))
#4 /2.4.5/generated/code/Magento/Sales/Model/ResourceModel/Order/Handler/Address/Interceptor.php(32): Magento\Sales\Model\ResourceModel\Order\Handler\Address\Interceptor->___callPlugins('process', Array, Array)
#5 /2.4.5/vendor/magento/module-sales/Model/ResourceModel/Order/Relation.php(98): Magento\Sales\Model\ResourceModel\Order\Handler\Address\Interceptor->process(Object(Magento\Sales\Model\Order\Interceptor))
#6 /2.4.5/vendor/magento/framework/Model/ResourceModel/Db/VersionControl/RelationComposite.php(48): Magento\Sales\Model\ResourceModel\Order\Relation->processRelation(Object(Magento\Sales\Model\Order\Interceptor))
#7 /2.4.5/vendor/magento/framework/Model/ResourceModel/Db/VersionControl/AbstractDb.php(57): Magento\Framework\Model\ResourceModel\Db\VersionControl\RelationComposite->processRelations(Object(Magento\Sales\Model\Order\Interceptor))
#8 /2.4.5/vendor/magento/framework/Model/ResourceModel/Db/AbstractDb.php(402): Magento\Framework\Model\ResourceModel\Db\VersionControl\AbstractDb->processAfterSaves(Object(Magento\Sales\Model\Order\Interceptor))
#9 /2.4.5/vendor/magento/module-sales/Model/ResourceModel/Order.php(180): Magento\Framework\Model\ResourceModel\Db\AbstractDb->save(Object(Magento\Sales\Model\Order\Interceptor))
#10 /2.4.5/generated/code/Magento/Sales/Model/ResourceModel/Order/Interceptor.php(32): Magento\Sales\Model\ResourceModel\Order->save(Object(Magento\Sales\Model\Order\Interceptor))
#11 /2.4.5/vendor/magento/module-sales/Model/OrderRepository.php(282): Magento\Sales\Model\ResourceModel\Order\Interceptor->save(Object(Magento\Sales\Model\Order\Interceptor))
#12 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Sales\Model\OrderRepository->save(Object(Magento\Sales\Model\Order\Interceptor))
#13 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Sales\Model\OrderRepository\Interceptor->___callParent('save', Array)
#14 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Sales\Model\OrderRepository\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Sales\Model\Order\Interceptor))
#15 /2.4.5/generated/code/Magento/Sales/Model/OrderRepository/Interceptor.php(59): Magento\Sales\Model\OrderRepository\Interceptor->___callPlugins('save', Array, Array)
#16 /2.4.5/vendor/magento/module-sales/Model/Service/OrderService.php(214): Magento\Sales\Model\OrderRepository\Interceptor->save(Object(Magento\Sales\Model\Order\Interceptor))
#17 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Sales\Model\Service\OrderService->place(Object(Magento\Sales\Model\Order\Interceptor))
#18 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Sales\Model\Service\OrderService\Interceptor->___callParent('place', Array)
#19 /2.4.5/vendor/magento/module-inventory-sales/Plugin/Sales/OrderManagement/AppendReservationsAfterOrderPlacementPlugin.php(195): Magento\Sales\Model\Service\OrderService\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Sales\Model\Order\Interceptor))
#20 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(135): Magento\InventorySales\Plugin\Sales\OrderManagement\AppendReservationsAfterOrderPlacementPlugin->aroundPlace(Object(Magento\Sales\Model\Service\OrderService\Interceptor), Object(Closure), Object(Magento\Sales\Model\Order\Interceptor))
#21 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Sales\Model\Service\OrderService\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Sales\Model\Order\Interceptor))
#22 /2.4.5/generated/code/Magento/Sales/Model/Service/OrderService/Interceptor.php(86): Magento\Sales\Model\Service\OrderService\Interceptor->___callPlugins('place', Array, Array)
#23 /2.4.5/vendor/magento/module-quote/Model/QuoteManagement.php(603): Magento\Sales\Model\Service\OrderService\Interceptor->place(Object(Magento\Sales\Model\Order\Interceptor))
#24 /2.4.5/vendor/magento/module-quote/Model/QuoteManagement.php(483): Magento\Quote\Model\QuoteManagement->submitQuote(Object(Magento\Quote\Model\Quote\Interceptor), Array)
#25 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Quote\Model\QuoteManagement->submit(Object(Magento\Quote\Model\Quote\Interceptor), Array)
#26 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Quote\Model\QuoteManagement\Interceptor->___callParent('submit', Array)
#27 /2.4.5/vendor/magento/module-sales-rule/Plugin/CouponUsagesIncrement.php(54): Magento\Quote\Model\QuoteManagement\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Quote\Model\Quote\Interceptor), Array)
#28 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(135): Magento\SalesRule\Plugin\CouponUsagesIncrement->aroundSubmit(Object(Magento\Quote\Model\QuoteManagement\Interceptor), Object(Closure), Object(Magento\Quote\Model\Quote\Interceptor))
#29 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Quote\Model\QuoteManagement\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Quote\Model\Quote\Interceptor))
#30 /2.4.5/generated/code/Magento/Quote/Model/QuoteManagement/Interceptor.php(68): Magento\Quote\Model\QuoteManagement\Interceptor->___callPlugins('submit', Array, NULL)
#31 /2.4.5/vendor/magento/module-quote/Model/QuoteManagement.php(441): Magento\Quote\Model\QuoteManagement\Interceptor->submit(Object(Magento\Quote\Model\Quote\Interceptor))
#32 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Quote\Model\QuoteManagement->placeOrder('2', NULL)
#33 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Quote\Model\QuoteManagement\Interceptor->___callParent('placeOrder', Array)
#34 /2.4.5/vendor/paypal/module-braintree-core/Plugin/OrderCancellation.php(63): Magento\Quote\Model\QuoteManagement\Interceptor->Magento\Framework\Interception\{closure}('2', NULL)
#35 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(135): PayPal\Braintree\Plugin\OrderCancellation->aroundPlaceOrder(Object(Magento\Quote\Model\QuoteManagement\Interceptor), Object(Closure), '2', NULL)
#36 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Quote\Model\QuoteManagement\Interceptor->Magento\Framework\Interception\{closure}('2', NULL)
#37 /2.4.5/generated/code/Magento/Quote/Model/QuoteManagement/Interceptor.php(50): Magento\Quote\Model\QuoteManagement\Interceptor->___callPlugins('placeOrder', Array, NULL)
#38 /2.4.5/vendor/magento/module-quote/Model/GuestCart/GuestCartManagement.php(87): Magento\Quote\Model\QuoteManagement\Interceptor->placeOrder('2', NULL)
#39 /2.4.5/generated/code/Magento/Quote/Model/GuestCart/GuestCartManagement/Interceptor.php(41): Magento\Quote\Model\GuestCart\GuestCartManagement->placeOrder('XoEhuqGJGhBcmT7...', NULL)
#40 /2.4.5/vendor/magento/module-checkout/Model/GuestPaymentInformationManagement.php(127): Magento\Quote\Model\GuestCart\GuestCartManagement\Interceptor->placeOrder('XoEhuqGJGhBcmT7...')
#41 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Checkout\Model\GuestPaymentInformationManagement->savePaymentInformationAndPlaceOrder('XoEhuqGJGhBcmT7...', 'example@example...', Object(Magento\Quote\Model\Quote\Payment), Object(Magento\Quote\Model\Quote\Address\Interceptor))
#42 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Checkout\Model\GuestPaymentInformationManagement\Interceptor->___callParent('savePaymentInfo...', Array)
#43 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Checkout\Model\GuestPaymentInformationManagement\Interceptor->Magento\Framework\Interception\{closure}('XoEhuqGJGhBcmT7...', 'example@example...', Object(Magento\Quote\Model\Quote\Payment), Object(Magento\Quote\Model\Quote\Address\Interceptor))
#44 /2.4.5/generated/code/Magento/Checkout/Model/GuestPaymentInformationManagement/Interceptor.php(23): Magento\Checkout\Model\GuestPaymentInformationManagement\Interceptor->___callPlugins('savePaymentInfo...', Array, Array)
#45 [internal function]: Magento\Checkout\Model\GuestPaymentInformationManagement\Interceptor->savePaymentInformationAndPlaceOrder('XoEhuqGJGhBcmT7...', 'example@example...', Object(Magento\Quote\Model\Quote\Payment), Object(Magento\Quote\Model\Quote\Address\Interceptor))
#46 /2.4.5/vendor/magento/module-webapi/Controller/Rest/SynchronousRequestProcessor.php(95): call_user_func_array(Array, Array)
#47 /2.4.5/vendor/magento/module-webapi/Controller/Rest.php(195): Magento\Webapi\Controller\Rest\SynchronousRequestProcessor->process(Object(Magento\Framework\Webapi\Rest\Request\Proxy))
#48 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Webapi\Controller\Rest->dispatch(Object(Magento\Framework\App\Request\Http))
#49 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Webapi\Controller\Rest\Interceptor->___callParent('dispatch', Array)
#50 /2.4.5/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Webapi\Controller\Rest\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Framework\App\Request\Http))
#51 /2.4.5/generated/code/Magento/Webapi/Controller/Rest/Interceptor.php(23): Magento\Webapi\Controller\Rest\Interceptor->___callPlugins('dispatch', Array, Array)
#52 /2.4.5/vendor/magento/framework/App/Http.php(116): Magento\Webapi\Controller\Rest\Interceptor->dispatch(Object(Magento\Framework\App\Request\Http))
#53 /2.4.5/generated/code/Magento/Framework/App/Http/Interceptor.php(23): Magento\Framework\App\Http->launch()
#54 /2.4.5/vendor/magento/framework/App/Bootstrap.php(264): Magento\Framework\App\Http\Interceptor->launch()
#55 /2.4.5/pub/index.php(30): Magento\Framework\App\Bootstrap->run(Object(Magento\Framework\App\Http\Interceptor))
#56 {main} [] []

The issue is this plugin which attempts to attach addresses to newly created invoices, and also reindex the grid.

magento2/app/code/Magento/Sales/Model/Order/Invoice/Plugin/AddressUpdate.php

Lines 52 to 74 in d6ff7b7

    
           foreach ($order->getInvoiceCollection()->getItems() as $invoice) { 
        
               $invoiceAttributesForSave = []; 
        
               if (!$invoice->getBillingAddressId() && $billingAddress) { 
        
                   $invoice->setBillingAddressId($billingAddress->getId()); 
        
                   $invoiceAttributesForSave[] = 'billing_address_id'; 
        
                   $orderInvoiceHasChanges = true; 
        
               } 
        
               if (!$invoice->getShippingAddressId() && $shippingAddress) { 
        
                   $invoice->setShippingAddressId($shippingAddress->getId()); 
        
                   $invoiceAttributesForSave[] = 'shipping_address_id'; 
        
                   $orderInvoiceHasChanges = true; 
        
               } 
        
               if (!empty($invoiceAttributesForSave)) { 
        
                   $this->attribute->saveAttribute($invoice, $invoiceAttributesForSave); 
        
               } 
        
           } 
        
           if ($orderInvoiceHasChanges) { 
        
               $this->gridPool->refreshByOrderId($order->getId()); 
        
           }

Seeing as we're creating the invoice during the same time as we're creating the order, this causes the dev/grid/async_indexing flag to not be respected.

Proposed Solution

I believe using the same kind of logic that exists in Magento\Sales\Model\GridAsyncInsert and Magento\Sales\Model\GridSyncInsertObserver should suffice, which is what i've put in this pull request.

I've not yet chased down the static/unit/integration testing etc, as I wanted to get this out there and get a discussion going before burning any more time on this, if this solution looks acceptable I'll tidy thing up a bit.

Any input / comments / criticisms? I'd really like this patched in before we get into Black Friday / Christmas and any feedback is appreciated.

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Fix best practice "Asynchronous order data processing" for auto invoiced orders #36334: Fix best practice "Asynchronous order data processing" for auto invoiced orders

m2-assistant · 2022-09-30T11:09:09Z

Hi @convenient. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

convenient · 2022-09-30T11:09:30Z

@magento run all tests

magento-automated-testing · 2022-09-30T11:09:36Z