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

Update indexer:reset to call $indexer->invalidate() #34582

Merged
merged 6 commits into from Jun 11, 2022

Conversation

convenient
Copy link
Contributor

@convenient convenient commented Nov 8, 2021

I was recently debugging something that was invalidating the indexers.

I added some logging using a before plugin on the invalidate function

public function beforeInvalidate(\Magento\Framework\Indexer\IndexerInterface $subject)
{
    $this->logger->info(
        "Invalidating index",
        [
            'indexer_id' => $subject->getId(),
            'trace' => str_replace(PHP_EOL, '|', (new \Exception())->getTraceAsString())
        ]
    );
}

But this use case isn't caught in my logs.

It seems to me that this command should just call the function that invalidates the indexer rather than repeating the work itself. I'm sorting out my own logging / debugging internally, but this seems sensible to fix here.

/**
* Set indexer invalid
*
* @return void
*/
public function invalidate()
{
$state = $this->getState();
$state->setStatus(StateInterface::STATUS_INVALID);
$state->save();
}

Manual testing scenarios (*)

  1. Call indexer:reset and see the indexer was invalidated

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Update indexer:reset to call $indexer->invalidate() #34988: Update indexer:reset to call $indexer->invalidate()

I was recently debugging something that was invalidating the indexers. 

I added some logging using a before plugin on the `invalidate` function

https://github.com/magento/magento2/blob/227cf9229152ed4332c607796cf476055ef9ec4f/app/code/Magento/Indexer/Model/Indexer.php#L345-L355

But this use case isn't covered. It seems to me that this command should just call the function that invalidates the indexer rather than repeating the work itself.
@m2-assistant
Copy link

m2-assistant bot commented Nov 8, 2021

Hi @convenient. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE,
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

@convenient
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@convenient convenient changed the title Update indexer:reset to call indexer->invalidate() Update indexer:reset to call $indexer->invalidate() Nov 8, 2021
@m2-community-project m2-community-project bot added this to Pending Review in Pull Requests Dashboard Nov 8, 2021
@convenient convenient changed the title Update indexer:reset to call $indexer->invalidate() Update indexer:reset to call $indexer->invalidate() Nov 8, 2021
@convenient convenient changed the title Update indexer:reset to call $indexer->invalidate() WIP - Update indexer:reset to call $indexer->invalidate() Nov 8, 2021
@convenient
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@convenient
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@convenient
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@convenient
Copy link
Contributor Author

@magento run Functional Tests CE, Functional Tests EE

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@convenient convenient changed the title WIP - Update indexer:reset to call $indexer->invalidate() Update indexer:reset to call $indexer->invalidate() Nov 9, 2021
@convenient
Copy link
Contributor Author

@magento run Functional Tests B2B

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@andrewbess andrewbess self-assigned this Nov 11, 2021
@m2-community-project m2-community-project bot moved this from Pending Review to Review in Progress in Pull Requests Dashboard Nov 11, 2021
@magento-engcom-team
Copy link
Contributor

Hi @andrewbess, thank you for the review.
ENGCOM-9342 has been created to process this Pull Request
✳️ @andrewbess, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@engcom-Delta
Copy link
Contributor

Thanks for your collaboration and contribution @convenient and @andrewbess .

  1. Created before plug-in Model as shown in the below screenshot.(beforeInvalidate method code also updated as per above description)
    image
  2. Upgraded & static deployed in the local magento instance
  3. Enabled the debugging mode
  4. Ran the indexer reset command
  5. Verified in the system log & debugging logs.

Expected before PR Changes :
As per the above issue comments & descriptions we should not expect in the logs
Expected after PR Changes:
We should expect in the logs.

Observation: Not found any logs after PR changes made.

Please lets know if we have to modify anything in the above execution steps

@convenient
Copy link
Contributor Author

convenient commented Jan 18, 2022

@engcom-Delta I can see your class isn't being constructed with the logger property? so I would expect a PHP fatal error to be logged?

However if no fatal error is occurring it suggests you dont have the plugin hooked in correctly?

If you're adding a plugin you'll need to clear the relevant generated directories and caches
https://devdocs.magento.com/guides/v2.4/howdoi/php/php_clear-dirs.html

This is what I have running in production...

diff --git a/src/Plugin/Indexer.php b/src/Plugin/Indexer.php
new file mode 100644
index 0000000..9aa35d1
--- /dev/null
+++ b/src/Plugin/Indexer.php
@@ -0,0 +1,32 @@
+<?php
+namespace Ampersand\MviewIndexerLogger\Plugin;
+
+class Indexer
+{
+    /** @var \Psr\Log\LoggerInterface */
+    private $logger;
+
+    /**
+     * Action constructor.
+     * @param \Psr\Log\LoggerInterface $logger
+     */
+    public function __construct(
+        \Psr\Log\LoggerInterface $logger
+    ) {
+        $this->logger = $logger;
+    }
+
+    /**
+     * @param \Magento\Framework\Indexer\IndexerInterface $subject
+     */
+    public function beforeInvalidate(\Magento\Framework\Indexer\IndexerInterface $subject)
+    {
+        $this->logger->info(
+            "Invalidating index",
+            [
+                'indexer_id' => $subject->getId(),
+                'trace' => str_replace(PHP_EOL, '|', (new \Exception())->getTraceAsString())
+            ]
+        );
+    }
+}
diff --git a/src/etc/di.xml b/src/etc/di.xml
index 8867f14..4d07f02 100644
--- a/src/etc/di.xml
+++ b/src/etc/di.xml
@@ -4,4 +4,7 @@
     <type name="Magento\Framework\Mview\ActionInterface">
         <plugin name="Ampersand\MviewIndexerLogger\Plugin\Action" type="Ampersand\MviewIndexerLogger\Plugin\Action"/>
     </type>
+    <type name="Magento\Framework\Indexer\IndexerInterface">
+        <plugin name="log-invalidate-indexer-trace" type="Ampersand\MviewIndexerLogger\Plugin\Indexer"/>
+    </type>
 </config>

@convenient
Copy link
Contributor Author

Also @engcom-Delta the main thing that needs tested is that calling indexer:reset still actually resets the indexer.

The plugin was just a bit of background why we don't want to have duplicated code, explaining my use case, it's just gravy.

@engcom-Delta
Copy link
Contributor

✔️ QA Passed

Preconditions:

1.We should have fresh installation of Magento 2.4 Develop.
2. Magento instance should be run in debug mode

Manual testing scenario:

  1. Create before plug-in Model as shown in the below screenshot.(beforeInvalidate method code also updated as per above description)
    image
  2. Remove the temporary files by using command
    rm -rf var/* generated/* pub/static/*
  3. Perform upgrade, static deploy ,cache flush
  4. Run Indexer::reset command
  5. Verify the logs updated with before plug-in messages.

Before: ✖️ Logs were not getting updated

After: ✔️ Logs are getting updated.

Screenshot 2022-01-18 at 7 15 38 AM

Since this PR is relevant to before -plugin code fixes in order to update the log files only ,this PR has no impact any other functional issues.Hence there is no additional regression is required.

@engcom-Delta
Copy link
Contributor

@magento create issue

@ishakhsuvarov
Copy link
Contributor

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 05788bb into magento:2.4-develop Jun 11, 2022
@convenient convenient deleted the patch-1 branch June 13, 2022 14:00
@ishakhsuvarov ishakhsuvarov moved this from Merge in Progress to Recently Merged in High Priority Pull Requests Dashboard Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Indexer Partner: Ampersand partners-contribution Pull Request is created by Magento Partner Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: accept Release Line: 2.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] Update indexer:reset to call $indexer->invalidate()
8 participants