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

Add option reindexallrequired #184

Merged
merged 1 commit into from
Mar 9, 2018
Merged

Conversation

seansan
Copy link
Contributor

@seansan seansan commented Feb 25, 2017

@bob2021
Copy link
Contributor

bob2021 commented Mar 2, 2017

I think the condition on line 166 should also check that

count($process->getUnprocessedEventsCollection()) === 0

Without this, indexers would be skipped even if they require a partial update

There are also still cases where an indexer will run even if it is not required - for example if the price indexer requires a re-index then it will also run the stock indexer as it is a dependency. A simple/ugly workaround might be to add this loop at line 164 (untested):

if ($this->getArg('reindexallrequired')) {
    foreach ($processes as $process) {
        if ($process->getStatus() == Mage_Index_Model_Process::STATUS_PENDING && count($process->getUnprocessedEventsCollection()) === 0) {
            $process->setData('runed_reindexall', true);
        }
    }
}

@seansan
Copy link
Contributor Author

seansan commented Mar 2, 2017

I think the main issue is to allow for a "re-run all" and if some exotic cases are missed this is OK.

  • So missing a required re-run is bad and should be caught in this fix
  • But if an exotic case exists where a re-run occurs - I personally would be OK with this ...
    Hope it helps

@seansan
Copy link
Contributor Author

seansan commented Mar 6, 2017

testing the extra condition

@tmotyl tmotyl added the review needed Problem should be verified label Mar 22, 2017
@colinmollenhour colinmollenhour merged commit c42291e into OpenMage:1.9.3.x Mar 9, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Mar 20, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Jul 17, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Sep 19, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Feb 14, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Apr 1, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 22, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Oct 25, 2019
@sreichel sreichel added new feature shell Relates to shell scripts labels Jun 9, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature review needed Problem should be verified shell Relates to shell scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants