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 additional indexes to speed up report #66

Open
jwalits opened this issue Apr 6, 2022 · 3 comments
Open

Add additional indexes to speed up report #66

jwalits opened this issue Apr 6, 2022 · 3 comments

Comments

@jwalits
Copy link

jwalits commented Apr 6, 2022

Whilst running the report, I found with around 4 million records in the local_csp table, the report would fall over at times.

Drilling down further into the error and sql query - found this one was causing some grief
https://github.com/catalyst/moodle-local_csp/blob/master/classes/table/csp_report.php#L178-L179

It would be good to add an index for both violateddirective and blockeddomain columns to speed up the query

Tested locally, Postgres was fine and was able to add index to those fields. However, when trying to add an index to those fields in MySQL came across this error
Specified key was too long; max key length is 3072 bytes.

After a chat with Brendan, a thought was converting the column to text to add an index.

I need to re-import the data again locally from prod and will test converting the columns to text and adding an index. Will create a PR with that shortly.

@brendanheywood
Copy link
Contributor

Or the other option is adding a derived column which has a hash or similar in it, and it is indexed and we adjust the sql so we are leveraging the derived column instead. Which is almost like a poor mans index in the table.

@danmarsden
Copy link
Member

Surely we don't need to keep 4 million records of csp report data? Isn't that a sign we aren't cleaning up?

@jwalits
Copy link
Author

jwalits commented Apr 6, 2022

That was caused by a previous bug in the plugin (I believe it was this one: #59) The data has since been removed on the client site and rebuilding it currently, but with those many rows the report stopped execution.

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

No branches or pull requests

3 participants