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

Probabilistic counting of pages is not working correctly #336

Closed
brendanheywood opened this issue Apr 11, 2024 · 2 comments · Fixed by #339
Closed

Probabilistic counting of pages is not working correctly #336

brendanheywood opened this issue Apr 11, 2024 · 2 comments · Fixed by #339
Assignees
Labels
bug Something isn't working

Comments

@brendanheywood
Copy link
Contributor

brendanheywood commented Apr 11, 2024

If I load a single page:

delete from mdl_tool_excimer_page_groups;

purge caches

ab -c 1 -n 4 https://master.localhost/admin/tool/heartbeat/croncheck.php

This loads 4 pages, in serial so there shouldn't be any issues with concurrency

This results in an approx count of 2 - 4 which is about right. If I repeat those same commands another 3 times then I'd expect 8-16 on average but its still on 2-4.

If I run 64 of them then still nothing:

ab -c 1 -n 64 https://master.localhost/admin/tool/heartbeat/croncheck.php

I think refactor approximate_increment so that it takes 2 values which is the current value and the increment value so we don't need to call it in a loop.

For example if the current value is 4, which means we have roughly 2^4 "things" = 16 and we add an increment of 16 seconds then we know for sure that we should increment to 2^5 = 32. We don't need to call the increment 16 times.

We should also write tests for this and have the increment function also optionally accept a random number so we can make the unit tests deterministic.

@brendanheywood brendanheywood added the bug Something isn't working label Apr 11, 2024
@brendanheywood
Copy link
Contributor Author

There is probably a couple bugs with this, at minimum the cache is not being saved when the increment happens, so it is resaving over the old, lower value, on top of the new incremented value so its not ever going up

@bwalkerl
Copy link
Contributor

The main bug here appears to be $month having a string type in get_page_group(string $name, string $month). This is serialized to create a key, but outside of this function $month is stored as an int, so the cachekeys this function creates don't match the cachekey getting updated.

bwalkerl added a commit that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants