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

Trigger auto-analyze more frequently for guestbook estimates #8972

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Sep 12, 2022

What this PR does / why we need it: Decreases the interval between table analyses for the guestbookresponse table which improves the download estimate. Prior to this PR, installations were seeing inaccuracies of several %, e.g. undercounts of 3000+/90K. With this added constraint, observed differences have so far been ~100 times smaller.

Which issue(s) this PR closes:

Closes #8840

Special notes for your reviewer: See issue for more discussion. Autovacuum is a background process. Running it includes 'analyzing' the table which updates the reltuples and relpages entries that the download estimate is based on. It looks like this requires a table scan (which was what the old direct select count(*) from guestbookresponse call was doing) and, since guestbookresponse is ~write-only, vacuuming (recovering disk space from deleted rows) itself isn't needed and autovacuum is presumably not doing much beyond scanning the table and updating the reltuples/relpages stats.

FWIW: The sql call in the flyway script is something that installations can further change, i.e. if it is problematic the default behavior can be restored. So this PR is primarily to provide a better default.

Also note that this PR may still not help that much when there are few downloads (for which you'd want an even smaller update % and/or would want to drop the +50 rows part of the calculation). An alternate/additional solution to use direct counting up to some threshold (or via a setting) has also been proposed.

Also also note - the original reporter of the issue is waiting for this change to be recommended and/or included before they are allowed to apply it to their production instance.

Suggestions on how to test this: By default, autovacuum is triggered when table size increases by 10% + 50 rows and this PR changes that to 1%+50 rows. I don't know if it is really necessary to test that specifically. Presumably one could trigger 50+1% downloads in a test instance and confirm that autovacuum triggers but it might be easier to just put the change on demo and watch over a few days to see when it happens there.
Alternately, to just confirm that vacuuming helps, one could look at the current download estimate (visible in the UI or you can run the estimate query )versus select count(*) from guestbookresponse; and then manually run vacuum (VERBOSE, ANALYZE) guestbookresponse; and confirm that the estimate improves.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?: I added one - not sure it is needed.

Additional documentation:

@mreekie mreekie added the bk2211 label Nov 1, 2022
@donsizemore
Copy link
Contributor

@qqmyers Good morning - will this FlyWay script also apply to new installations, or would it only get applied at time of upgrade to the next release?

@qqmyers
Copy link
Member Author

qqmyers commented Nov 9, 2022

both - flyway applies all missing updates upon start so new installs will get all of them, upgrades will just get the new ones since the last update.

@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 13, 2022
@mreekie
Copy link

mreekie commented Dec 13, 2022

This is a one line database update.
Minimal code to review and minimal to test.
size = 3

@mreekie
Copy link

mreekie commented Dec 14, 2022

added to sprint Dec 15, 2022

Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

👍

@kcondon kcondon self-assigned this Dec 20, 2022
@kcondon kcondon merged commit 3bdc075 into IQSS:develop Dec 21, 2022
@pdurbin pdurbin added this to the 5.13 milestone Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Issue: download counts dropped between 5.8 to 5.9?
6 participants