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

Disable DISABLE KEYS statements #188

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

bob2021
Copy link
Contributor

@bob2021 bob2021 commented Feb 27, 2017

This is part of a group of PRs containing the changes discussed in issue #152

Indexers that extend Mage_Index_Model_Resource_Abstract run an ALTER TABLE ... DISABLE|ENABLE KEYS statement before and after inserting data.

The statement is only supported by MyISAM tables so it is useless for modern magento installations, but it still requires a table metadata lock even if MyISAM is not used.

The catalogsearch index does still use MyISAM, but it doesn't extend Mage_Index_Model_Resource_Abstract so it isn't affected.

This PR disables the statements by default, but leaves the functionality in place if still required.

It also removes calls to Mage_Index_Model_Resource_Abstract::useDisableKeys. In most cases it was called by mistake and had no effect (it should have been Mage_Index_Model_Resource_Abstract::disableTableKeys), but now they aren't required at all.

Copy link
Contributor

@cieslix cieslix left a comment

Choose a reason for hiding this comment

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

+1
looks ok, I have no idea who is using MYISAM tables with Magento anyway.

I was testing it with Magento and sample data and no issue with reindexing.

@colinmollenhour
Copy link
Member

Would you mind rebasing or resubmitting on 1.9.3.x branch? Otherwise I approve.

@tmotyl tmotyl changed the base branch from 1.9.2.4 to 1.9.3.x March 6, 2017 08:10
@tmotyl tmotyl changed the base branch from 1.9.3.x to 1.9.2.4 March 6, 2017 08:11
@bob2021 bob2021 changed the base branch from 1.9.2.4 to 1.9.3.x March 6, 2017 10:19
@bob2021
Copy link
Contributor Author

bob2021 commented Mar 6, 2017

Rebased on 1.9.3.x

@tmotyl
Copy link
Contributor

tmotyl commented Mar 22, 2017

@colinmollenhour would you mind merging it once you approve?

@tmotyl tmotyl added the review needed Problem should be verified label Mar 22, 2017
@colinmollenhour colinmollenhour merged commit f63b9a0 into OpenMage:1.9.3.x Mar 22, 2017
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Dec 6, 2017
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Dec 6, 2017
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
@sreichel sreichel added enhancement and removed review needed Problem should be verified labels Jan 11, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Feb 28, 2018
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Mar 20, 2018
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Jul 17, 2018
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Sep 19, 2018
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Feb 14, 2019
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Apr 1, 2019
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 22, 2019
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Oct 25, 2019
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
disable 'ALTER TABLE ... DISABLE KEYS' statements by default,
and remove useless calls to useDisableKeys()
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