search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.55k stars 9.32k forks source link

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

Closed m2-assistant[bot] closed 2 years ago

m2-assistant[bot] commented 2 years ago

This issue is automatically created based on existing pull request: magento/magento2#36224: Fix best practice "Asynchronous order data processing" for auto invoiced orders

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.

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

Screenshot 2022-09-30 at 10 53 42 Screenshot 2022-09-30 at 10 53 55 Screenshot 2022-09-30 at 10 54 13

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

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

Expected result:

Actual result:

https://user-images.githubusercontent.com/600190/193256993-d8b41219-0221-4cce-a245-7e64ab251679.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.

https://github.com/magento/magento2/blob/d6ff7b706e25d9ca2ffd7ad66ebf29a6eda8c686/app/code/Magento/Sales/Model/Order/Invoice/Plugin/AddressUpdate.php#L52-L74

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 (*)

github-jira-sync-bot commented 2 years ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-6908 is successfully created for this GitHub issue.

m2-assistant[bot] commented 2 years ago

:white_check_mark: Confirmed by @engcom-Lima. Thank you for verifying the issue.
Issue Available: @engcom-Lima, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

Winfle commented 1 year ago

This is really detailed issue, thanks