Performance problem & memory leak in Mage_Index_Model_Process

[tmotyl]

around line 205 of Mage_Index_Model_Process::reindexAll() there is code like this:

 $unprocessedEvents = $eventResource->getUnprocessedEvents($this);
            if ($this->getMode() == self::MODE_MANUAL && (count($unprocessedEvents) > 0)) {
                $this->_getResource()->updateStatus($this, self::STATUS_REQUIRE_REINDEX);

similar code is also around line 233 in the same file

which means that the whole collection of index/event for given process and status is fetched, just to call "count()" to check if records existis in db...

the second problem is that this query and count() are executed even if process is in MODE_MANUAL -> the if statement for mode should be checked before executing query

Magento should make a query which returns 0 or 1 if records are still there like this
SELECT IF(COUNT() > 0, 1, 0) FROM ... WHERE ...
or
SELECT count() FROM (SELECT 1 FROM ...WHERE .... LIMIT 1)

[magento-team]

Hi,

Thank you for your post.

Unfortunately, I am not sure I understand issues you reported. Could you please clarify following:
What is the actual performance problem? Do you have any time measurement you can provide along with a test case in regards to code performance? What exactly is the performance problem?
In regards to 'memory leak' did you mean memory consumption?
Do you have a code change suggestion along with a test case that would show performance and memory consumption improvement I could use to move this ticket into development stage?

The code you posted as problematic is actually in place by design, at this point I do not see any misbehavior.
Additionally, Indexers have been refactored as of 1.13 (Performance Release) and the code you posted is still in place by design.
Please explain your concerns more by answering my questions above.

Thank you,
Anton Kosyriev

[tmotyl]

Hi Anton,
I haven't tested the new indexer stuff yet, but if the code I mentioned is still in place, the problem is still valid.
Let's imagine you have few thousands of unprocessed records in the index_event table.
Then the line:

 $unprocessedEvents = $eventResource->getUnprocessedEvents($this);

will fetch all of them, and load into memory.
Magento will do it just for checking if the collection of unprocessed events is greated than 0:
(count($unprocessedEvents) > 0))

So the problem is that Magento will use a lot of memory just for checking the count on the collection... Not to mention that loading all these objects also take a lot of time.

I haven't done performance test for this problem, so I can not share any numbers with you. But please, the issue here is quite obvious, and easy to spot just from reading the code.

My suggestion here is to refactor the code, by adding additional method to $eventResource, which will test whether there are any unprocessed events.
On mysql level magento should do a query like:
Magento should make a query which returns 0 or 1 if records are still there like this
SELECT IF(COUNT() > 0, 1, 0) FROM ... WHERE ...
or
SELECT count() FROM (SELECT 1 FROM ...WHERE .... LIMIT 1)

to assure, that mysql will not do a full scan, but just check whether there are more then one unprocessed events in the table.

Please let me know if you have any questions.
Cheers
Tymoteusz Motylewski

[magento-team]

Hi, This issue has been fixed and will be published with next update.