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 searchindex helper and console command #82

Merged
merged 4 commits into from
May 24, 2024
Merged

Conversation

nevinsm
Copy link
Contributor

@nevinsm nevinsm commented May 2, 2024

I noticed that we seem to have a lot of issues with the searchindex table causing performance issues on larger sites, or sites with data import features. This update should add a handy helper method that can be called from jobs, as well as the cli to optimize that problematic table.

@nevinsm nevinsm requested review from joshuapease and JJimmyFlynn May 2, 2024 18:53
@nevinsm nevinsm marked this pull request as ready for review May 6, 2024 17:05
Copy link
Contributor

@joshuapease joshuapease left a comment

Choose a reason for hiding this comment

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

Glad you thought about PostgreSQL!

I'm not 💯 certain the pg test will pass. But... our whole test suite is broken... so I wouldn't consider that a blocker.

@@ -28,7 +28,7 @@ services:
- XDEBUG_MODE

mysql:
image: mysql:5.7.29
image: mysql:8.0.37
Copy link
Contributor

Choose a reason for hiding this comment

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

Much thanks.

Comment on lines 29 to 33
Craft::$app->getDb()->driverName = 'pgsql';
$result = SearchIndex::optimize();

// Assert: Verify that the result is an integer (number of rows affected)
$this->assertIsInt($result);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this might throw an SQL error unless our test environment is using PostgreSQL.

I'm planning to switch our CI tests to use GitHub actions. So I might be able to make a matrix of tests (one for MySQL one for PostgreSQL).

Copy link
Contributor

Choose a reason for hiding this comment

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

I just merged an updated GitHub actions based test runner.

It uses a matrix to run tests with MySQL and Postgres.

It also switches the local dev environment to DDEV.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rebased this work and added test skipping based on db driver so the unit test works with the new matrix strategy.

@nevinsm nevinsm force-pushed the nsm/searchindex-optimization branch from c81f3db to 94e91f1 Compare May 22, 2024 14:15
@maxfenton maxfenton requested a review from ten1seven May 22, 2024 19:24
@nevinsm nevinsm merged commit fb7bc2c into v5 May 24, 2024
0 of 2 checks passed
@nevinsm nevinsm deleted the nsm/searchindex-optimization branch May 24, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants