Skip to content

[update] enhance performance on large catalog #16570

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

Merged
merged 4 commits into from
Aug 8, 2018
Merged

[update] enhance performance on large catalog #16570

merged 4 commits into from
Aug 8, 2018

Conversation

AurelienLavorel
Copy link
Member

@AurelienLavorel AurelienLavorel commented Jul 5, 2018

Description

On Magento update, if you have a big number of order (>100k for my case), you can't pass setup:upgrade step.
Instead of using Magento collection, it's a direct SQL request.

Manual testing scenarios

Upgrade Magento from 2.2.2 to 2.2.5.

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)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @AurelienLavorel. Thank you for your contribution
Here is 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@AurelienLavorel AurelienLavorel requested review from dverkade and RomaKis and removed request for dverkade July 20, 2018 09:58
@AurelienLavorel
Copy link
Member Author

@magento-engcom-team ?

@ihor-sviziev
Copy link
Contributor

Related Pull Request:
#15097

@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-2596 has been created to process this Pull Request

@ihor-sviziev
Copy link
Contributor

Result query will be following:

UPDATE sales_order_address
INNER JOIN sales_order ON sales_order_address.parent_id = sales_order.entity_id
INNER JOIN quote_address ON sales_order.quote_id = quote_address.quote_id
                            AND sales_order_address.address_type = quote_address.address_type
SET sales_order_address.quote_address_id = quote_address.address_id
WHERE sales_order_address.quote_address_id IS NULL

@magento-engcom-team
Copy link
Contributor

Hi @AurelienLavorel. Thank you for your contribution.
We will aim to release these changes as part of 2.2.7.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

@rikwillems
Copy link
Contributor

@magento-engcom-team this patch won't work with Enterprise Edition with split database setup.

@ihor-sviziev
Copy link
Contributor

Hi @rikwillems,
Thank you for your feedback! I asked @engcom-backlog-nazar to re-check it.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Oct 17, 2018

Hi @AurelienLavorel,
We verified issue reported by @rikwillems and actually change from this PR failing in Magento Commerce installation with split database setup.
image

In such configuration we have 2 split databases

  • quotes are located in "checkout" connection. Example:
    private static $connectionName = 'checkout';

    $connection = $setup->getConnection(self::$connectionName);
    $connection->modifyColumn(
    $setup->getTable('quote_address', self::$connectionName),
    'telephone',
    ['type' => Table::TYPE_TEXT, 'length' => 255]
    )->modifyColumn(
    $setup->getTable('quote_address', self::$connectionName),
    'fax',
    ['type' => Table::TYPE_TEXT, 'length' => 255]
    )->modifyColumn(
    $setup->getTable('quote_address', self::$connectionName),
    'region',
    ['type' => Table::TYPE_TEXT, 'length' => 255]
    )->modifyColumn(
    $setup->getTable('quote_address', self::$connectionName),
    'city',
    ['type' => Table::TYPE_TEXT, 'length' => 255]
    );
  • Orders and addresssed are located in "sales" connection. Example:
    private static $connectionName = 'sales';

    $connection = $installer->getConnection(self::$connectionName);
    $connection->modifyColumn(
    $installer->getTable('sales_order', self::$connectionName),
    'remote_ip',
    [
    'type' => \Magento\Framework\DB\Ddl\Table::TYPE_TEXT,
    'length' => 45
    ]
    );
    }

Could you prepare new PR that fixing this issue OR revert your changes?

@AurelienLavorel
Copy link
Member Author

Yes, I'm mounting an environment with that set up to try.

@ihor-sviziev
Copy link
Contributor

Hi @AurelienLavorel,
Thank you for working on it!

@rikwillems
Copy link
Contributor

@ihor-sviziev Please see my PR as a fix for this issue.

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

Successfully merging this pull request may close these issues.

5 participants