Skip to content

Conversation

@outdooracorn
Copy link
Member

Remove the conditions that doesn't save metrics if the Wiki is deleted. A check for whether metrics are equal is enough to prevent duplicate data from being stored.

Bug: T415034

@outdooracorn outdooracorn self-assigned this Jan 20, 2026
Copy link
Contributor

@tarrow tarrow left a comment

Choose a reason for hiding this comment

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

Seems fine to me and happy with the tests but I think we probably ought to add one more.

I'm also sadly not that confident in this huge class now so I would suggest that on deployment of the new code we manually trigger it and carefully, critically evaluate the results while we have the context mostly in our heads.

Honestly I'm still not sure I can really answer the question: "why are people convinced that user count is behaving differently to page count?" because to me they appear to be being treated equally.

@outdooracorn
Copy link
Member Author

outdooracorn commented Jan 21, 2026

Honestly I'm still not sure I can really answer the question: "why are people convinced that user count is behaving differently to page count?" because to me they appear to be being treated equally.

The page count has been collected from the start, so when the job ran after the wiki was deleted it a record was added with the last known pages and with is_deleted = 1 (true). For every subsequent run, no new record would be created because the most recent record in wiki_daily_metrics with is_deleted = 1.

The user count was added later. For any wiki deleted after the user count was introduced, I believe the previous logic would have worked as above. For already deleted wikis, as there is a record in wiki_daily_metrics with is_deleted = 1, no new record will be added with a value in the total_user_count column.

What we probably should have done when the user count was introduced, is for all deleted wikis set total_user_count to the value in wiki_site_stats.users rather than null. We could still do that instead/as well as, if you think it is worth the effort?

@tarrow
Copy link
Contributor

tarrow commented Jan 21, 2026

The user count was added later. For any wiki deleted after the user count was introduced, I believe the previous logic would have worked as above.

OK! I got it I think!

@tarrow
Copy link
Contributor

tarrow commented Jan 21, 2026

What we probably should have done when the user count was introduced, is for all deleted wikis set total_user_count to the value in wiki_site_stats.users rather than null. We could still do that instead/as well as, if you think it is worth the effort?

I reckon there's no real need unless the PM comes back to us.

I do wonder if we're going to be asked to backfill wiki_site_stats.users too but let's also cross that bridge as and when.

I'm happy to approve post rebase

Remove the conditions that doesn't save metrics if the Wiki is deleted. A check for whether metrics are equal is enough to prevent duplicate data from being stored.

Bug: T415034
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.

3 participants