SchumacherFM / Magento-FastIndexer

Makes indexing of your Magento store around x times faster! ‼️ Maintainers wanted!
Apache License 2.0
80 stars 21 forks source link

Default file based locks don't work #3

Open TivoSoho opened 8 years ago

TivoSoho commented 8 years ago

You have:

        $userModel = Mage::getStoreConfig('fastindexer/indexer/lock_model');
        if (true === empty($userModel) || false === $this->getFiHelper()->isEnabled()) {
            $this->_lockInstance = false;
            return false;
        }

And since the default file lock has no value (empty check returns true), it will not work - Fatal error: Call to a member function isLockExists() on a non-object in ...app\code\core\Mage\Index\Model\Process.php on line 480

TivoSoho commented 8 years ago

I understand it was meant to be like that, since you do have this:

        if (false === $this->getLockInstance()) {
            return parent::isLocked();
        }

What you fail to realize here is that you have overridden the parent variable for false - so of course this will fail as you cannot call isLockExists() on false. :)

SchumacherFM commented 8 years ago

Thanks for spotting that! Never relealized it because of having no tests 8-)

Would you mind sending me a PR?

TivoSoho commented 8 years ago

Sorry, don't know what PR is/means.

Btw, also due to the value "false", disabling it also breaks everything as again - cannot call functions on false.

TivoSoho commented 8 years ago

Some background as to why we need it: we have a huge price index table - over 4 million records that it inserts. And of course as such we have a lot of failed transactions during indexing. I had a similar idea to yours on how to minimize that time to mere fractions. So if your code works that would be perfect - it is better, much more advanced and it would take me forever to code my own idea. :) We are using Magento 1.9.1.0

SchumacherFM commented 8 years ago

PR means Pull Request. https://help.github.com/articles/using-pull-requests/

The fast indexer is only useful for a full reindex. partial reindexing can never be supported.

I would be happy to merge your changes into this repository.

ross-ritchey commented 8 years ago

Don't want to go through the process of setting up a branch to create a pull request for this one....

This issue is fixed by removing the:

$this->_lockInstance = false;`

From:

$userModel = Mage::getStoreConfig('fastindexer/indexer/lock_model'); if (true === empty($userModel) || false === $this->getFiHelper()->isEnabled()) { $this->_lockInstance = false; return false; }

This change needs tested - but on my 1.9.2.4 setup everything seems to be working good.

SchumacherFM commented 8 years ago

I'm not sure if that fixes it. There needs to be also some compatibility with older versions.

gjhilderink commented 8 years ago

When i disable the:

$this->_lockInstance = false;

I get a new error on line 136, Call to undefined method Mage_Index_Model_Lock::isLocked()

I am also using magento 1.9.2.4

mverstegen commented 7 years ago

I had the same problem, you need to look in the core_config_data table on path "fastindexer/indexer/lock_model" and set it to "db". After that it is working perfect for me so far...

@SchumacherFM i'm also the dnd path url extension, is your module overruling the complete rewrite logic or is it using the one already exist?

SchumacherFM commented 7 years ago

No idea :-(