Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#55890 closed defect (bug) (fixed)

Option 'user_count' not initalized or updated on single sites

Reported by: knutsp's profile knutsp Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.0.1 Priority: normal
Severity: normal Version: 6.0
Component: Users Keywords: needs-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Site Health Info shows "User count" on both multisite and single. In both cases it now uses the get_user_count function, but this function still emits "Doing it wrong" and returns -1 when not on multisite.

It seems in changeset [53011] it was intended to refactor get_user_count() to return actual user count also on single sites. Can't find that happened.

Change History (18)

#1 @SergeyBiryukov
2 years ago

  • Description modified (diff)

#2 in reply to: ↑ description ; follow-up: @SergeyBiryukov
2 years ago

Replying to knutsp:

Site Health Info shows "User count" on both multisite and single. In both cases it now uses the get_user_count function, but this function still emits "Doing it wrong" and returns -1 when not on multisite.

As far as I can tell, it only emits _doing_it_wrong() on single site if the $network_id parameter is passed, which does not appear to be the case in Site Health.

#3 in reply to: ↑ 2 @knutsp
2 years ago

Replying to SergeyBiryukov:

Replying to knutsp:

Site Health Info shows "User count" on both multisite and single. In both cases it now uses the get_user_count function, but this function still emits "Doing it wrong" and returns -1 when not on multisite.

As far as I can tell, it only emits _doing_it_wrong() on single site if the $network_id parameter is passed, which does not appear to be the case in Site Health.

Correct. The returned number -1 must be the default when 'user_count' option not present, and I can confirm this option is not set on the single sites I have checked so far.

It seems the option should be updated via a cron task 'wp_update_user_counts'

I can see upgrade_600() runs an intial wp_update_user_counts().

#4 @knutsp
2 years ago

  • Component changed from Site Health to Users
  • Summary changed from Site Health / Info / WordPress show User count -1 to Option 'user_count' not initalized or updated on single sites

I guess this doesn't belong under the Site Health component.
First seen on Site Health / Info / WordPress / User count as -1

#6 follow-up: @tharsheblows
2 years ago

The function previously wasn't available on a single site so using it incorrectly by passing a non-null $network_id on a single site should still be an error.

On a single site, this is working for me correctly and when I delete the user_count option, it returns a value of -1 on the site health debug screen.

There isn't any place in core that passes a parameter to the function without checking for multisite first.

#7 in reply to: ↑ 6 @knutsp
2 years ago

Replying to tharsheblows:

The function previously wasn't available on a single site so using it incorrectly by passing a non-null $network_id on a single site should still be an error.

That's ok.

On a single site, this is working for me correctly and when I delete the user_count option, it returns a value of -1 on the site health debug screen.

How long time does it take, on your site, before this option is added back (updated) automatically and the correct number is shown?

#8 follow-up: @tharsheblows
2 years ago

It schedules it for twice daily but also updates it every time a user is added -- I think the latter might be as long as it's not a "large site" which is defined as a site > 10,000 users but I didn't investigate too deeply! 😊

#9 in reply to: ↑ 8 ; follow-up: @knutsp
2 years ago

Replying to tharsheblows:

It schedules it for twice daily but also updates it every time a user is added -- I think the latter might be as long as it's not a "large site" which is defined as a site > 10,000 users but I didn't investigate too deeply! 😊

Added a user on two of my sites that shows -1. Result: Shows correct number.

This boils down to a cron job not being added/activated. The sites I have checked have no known cron issues. They were all updated to 6.0 from 5.9.3.

#10 in reply to: ↑ 9 @SergeyBiryukov
2 years ago

Replying to knutsp:

This boils down to a cron job not being added/activated.

Looks like wp_schedule_update_user_counts() is hooked to two actions:

  • admin_init
  • wp_update_user_counts

I find the latter a bit strange, as it is hooked to the very action it's supposed to schedule, and since it's a recurring event, I think this should not be necessary, unless I'm missing something.

#11 @peterwilsoncc
2 years ago

  • Milestone changed from Awaiting Review to 6.0.1

I've moved this on to the 6.0.1 milestone for further investigation.

I suspect the following needs to happen:

  • review the hook on which the scheduled task is scheduled
  • ensure it runs on update (something missed during 6.0)

cc @spacedmonkey because I think we worked on this together for the 6.0 cycle.

#12 follow-up: @spacedmonkey
2 years ago

@knutsp

Can you confirm, a couple of things?

  • Is your instance of WordPress multisite or single site.
  • Has the database upgrade been run after upgrading to 6.0?
  • Are scheduled events disabled on your site? Or are they run in a special way?

As the user count is run on database upgrade / scheduled event, then seems unlikely it would ever be -1.

Worth noting here, that a lot of the weirdness around this functionality was already found in core. It was just moved to single site context.

#13 in reply to: ↑ 12 @knutsp
2 years ago

Replying to spacedmonkey:

  • Is your instance of WordPress multisite or single site.

Single sites. This ticket is specifically reported for single site, see summary.

  • Has the database upgrade been run after upgrading to 6.0?

No reason not to think so, as no specific task for single sites.

  • Are scheduled events disabled on your site? Or are they run in a special way?

Nope. Default, no CRON constants or plugins.

As the user count is run on database upgrade / scheduled event, then seems unlikely it would ever be -1.

Well, it shows -1 on Site Health, confirmed by inspecting database, the option doesn't exist.

By adding a user, or sites that have had user registrations, the number shown is correct.

My sites have a lot of plugins, except my testing site for alpha/beta (now WP 6.1-alpha) with zero plugins. This one also showed -1 until I added a user.

My sites were all updated to major automatically by WP_AUTO_UPDATE_CORE = true.

I expected others to confirm this.

#14 follow-up: @spacedmonkey
2 years ago

Site Health Info shows "User count" on both multisite and single

I found this line a little confusing, as mentioned both single and multisite. Thanks for clearing this up.

You mention the number of plugins, you have installed. Can detail some of these. Maybe one of these is effecting the count.

Are there other instances of scheduled events not firing, like scheduled posts or the like.

For now, there is a worth around. There is a filter wp_is_large_user_count, where this new counting behaviour can be disabled.

#15 in reply to: ↑ 14 @knutsp
2 years ago

Replying to spacedmonkey:

You mention the number of plugins, you have installed. Can detail some of these. Maybe one of these is effecting the count.

I discovered this bug on a site with no plugins, little visited, but I visited it myself and waited to next day, checked again, nothing changed. Then installed "WP Crontrol" and can see task `wp_update_user_counts´ is scheduled "Twice Daily". No fatals on PHP log.

I have now inspected over 20 single (client) sites, all show -1 in Site Health, except those where user(s) have been added/registered after update to 6.0.

Clicked "Run now" in Cron Events on a site showing -1, still showing -1 after.

All sites, PHP 8.0 or 8.1.

Last edited 2 years ago by knutsp (previous) (diff)

#16 follow-up: @spacedmonkey
2 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to assigned

@SergeyBiryukov @peterwilsoncc I believe the fix for this is a simple as change the database version and ensuring that caches are flushed. This is done in [53496] that should be backported into 6.0.1. I am going to assign this to Sergey as release lead and backporter.

#17 in reply to: ↑ 16 @SergeyBiryukov
2 years ago

Replying to spacedmonkey:

I believe the fix for this is a simple as change the database version and ensuring that caches are flushed. This is done in [53496] that should be backported into 6.0.1.

Thanks! At a glance, it does seem like not bumping $wp_db_version previously in [53011] would lead to the upgrade_600() routine not running, which would cause the user_count option to be missing.

In that case, bumping the DB version in [53496] should resolve the issue.

Related: comment:32:ticket:55837

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

#18 @peterwilsoncc
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53650:

Taxonomy: Fix caching issues in WP_Term_Query class.

Introduced [52836] when passing child_of or pad_counts parameters to get_terms or WP_Term_Query class, the array
of terms received by the query, was not correctly cached. This change simplifies the logic in WP_Term_Query and
ensures terms are correctly cached. This change also, improves performance, by only caching an array of term ids where
possible.

Additionally, the database version bump included in this patch is also required for #55890 to initialize the user count
on single sites.

Props denishua, spacedmonkey, oztaser, peterwilsoncc, SergeyBiryukov, georgestephanis, jnz31, knutsp, mukesh27, costdev,
tharsheblows.
Merges [53496] to the 6.0 branch.
Fixes #55837, #55890.

--This line, and those below, will be ignored--

_M .
M src/wp-includes/class-wp-term-query.php
M src/wp-includes/version.php
M tests/phpunit/tests/term/getTerms.php

Note: See TracTickets for help on using tickets.