Skip to content
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

Old inactive quotes are now actually purged from the database #1489

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

FredericMartinez
Copy link
Contributor

Description (*)

Since the beggining of Magento, only quotes converted to orders can be deleted by sales_clean_quotes cron because quote collection filter on is_active=0


Show quotes group by year

select YEAR(updated_at), count('*')
from sales_flat_quote
GROUP BY YEAR(updated_at)
YEAR(updated_at) count('*')
2018 1014080
2019 404537
2020 159526
2021 46765

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added the Component: Sales Relates to Mage_Sales label Mar 8, 2021
@rvelhote
Copy link
Contributor

rvelhote commented Mar 9, 2021

I was thinking about this issue last week.

Our sales_flat_quote table is at ~2.15 million records and there is a situation where the trigger_recollect flag might be modified for all quotes due to a catalog rule or price modification (maybe more reasons?).

I believe that this was the cause for the dreaded SQLSTATE[HY000]: General error: 1205 Lock wait timeout exceeded; try restarting transaction, query was: XYZ we experienced last week. The table took 3 to 5 minutes fo fully update which stopped customers from adding products to the cart during that timeframe - it's possible/likely, however, that it was executed multiple times in succession rather than just once.

I'm still in the process of investigating this issue and mitigation and reducing the table would have a very positive effect.

fballiano
fballiano previously approved these changes Apr 2, 2021
@joshua-bn
Copy link
Contributor

joshua-bn commented Apr 2, 2021

Yikes. If checkout/cart/delete_quote_after is empty for whatever reason, it will cause all active quotes to break. Please, whatever you do, put a default on that.

I just checked the value I have. It's 0. That would be very, very bad if that ever ran on production for me.

@fballiano
Copy link
Contributor

what are we doing on this @FredericMartinez?

@FredericMartinez
Copy link
Contributor Author

@fballiano I removed the filter, old active non-active quotes are now purged.
Already in production since years, I have never saw a breaking change

kiatng
kiatng previously approved these changes May 3, 2022
@luigifab
Copy link
Contributor

luigifab commented May 4, 2022

If checkout/cart/delete_quote_after is empty for whatever reason, it will cause all active quotes to break.

A check is needed no? For example, if empty, keep is_active filter?

@kiatng kiatng self-requested a review May 5, 2022 02:54
@kiatng
Copy link
Contributor

kiatng commented May 5, 2022

If checkout/cart/delete_quote_after is empty for whatever reason, it will cause all active quotes to break.

A check is needed no? For example, if empty, keep is_active filter?

We cannot merge this until this is addressed: all active quotes will be deleted if checkout/cart/delete_quote_after is empty.

A suggestion:

    public function cleanExpiredQuotes($schedule)
    {
        Mage::dispatchEvent('clear_expired_quotes_before', array('sales_observer' => $this));
        $lifetimes = Mage::getConfig()->getStoresConfigByPath('checkout/cart/delete_quote_after');
        foreach ($lifetimes as $storeId => $day) {
            $day = (int) $day;
            $lifetime = 86400 * $day;
            /** @var Mage_Sales_Model_Mysql4_Quote_Collection $quotes */
            $quotes = Mage::getModel('sales/quote')->getCollection();

            $quotes->addFieldToFilter('store_id', $storeId);
            $quotes->addFieldToFilter('updated_at', array('to'=>date("Y-m-d", time()-$lifetime)));
            if ($day === 0) {
                $quotes->addFieldToFilter('is_active', 0);
            }
            // ...

Also add a note in the backend, something like:

image

Please suggest better English.

@luigifab
Copy link
Contributor

With the comment, I also suggest to add in system.xml: <validate>validate-not-negative-number</validate>

Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've to take a look at what @kiatng is saying

@luigifab
Copy link
Contributor

luigifab commented Nov 17, 2022

I updated and merged with Github web editor.

@github-actions github-actions bot added the Component: Checkout Relates to Mage_Checkout label Nov 17, 2022
luigifab
luigifab previously approved these changes Nov 18, 2022
luigifab
luigifab previously approved these changes Jan 20, 2023
@luigifab luigifab requested a review from fballiano January 20, 2023 15:51
@elidrissidev elidrissidev self-requested a review January 20, 2023 19:50
@fballiano fballiano changed the base branch from 1.9.4.x to main May 12, 2023 17:09
@fballiano fballiano dismissed luigifab’s stale review May 12, 2023 17:09

The base branch was changed.

@fballiano
Copy link
Contributor

I've rebased it and I think it's ready to be merged

@kyrena
Copy link
Contributor

kyrena commented Jun 16, 2023

Ping @kiatng and @FredericMartinez for latest PR update 😸.

@fballiano fballiano changed the title Really purge old quotes Old inactive quotes are now actually purged from the database Jul 3, 2023
@fballiano fballiano merged commit e7e10cf into OpenMage:main Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Checkout Relates to Mage_Checkout Component: Sales Relates to Mage_Sales
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants