[buddypress-trac] [BuddyPress Trac] #5128: Move user and group last_activity out of wp_usermeta

[buddypress-trac]

#5128: Move user and group last_activity out of wp_usermeta
-------------------------------------+------------------
 Reporter:  boonebgorges             |       Owner:
     Type:  enhancement              |      Status:  new
 Priority:  high                     |   Milestone:  2.0
Component:  Core                     |     Version:
 Severity:  major                    |  Resolution:
 Keywords:  has-patch needs-testing  |
-------------------------------------+------------------
Changes (by boonebgorges):

 * keywords:   => has-patch needs-testing


Comment:

 5128.patch is a first attempt at moving 'last_activity' out of usermeta.
 It's a complicated issue, so bear with me while I break down the strategy.
 Line numbers below refer to the *right-hand* column in
 https://buddypress.trac.wordpress.org/attachment/ticket/5128/5128.patch.

 '''Performance'''

 The change requires a number of changes that make direct comparisons
 difficult, but here's a summary of the relevant queries (on an
 installation with about 100,000 activity items and 25,000 users):

 1. Trunk

 {{{
 SELECT umeta_id FROM wp_usermeta WHERE meta_key = 'last_activity' AND
 user_id = 1 (0.2ms)

 SELECT u.user_id as id FROM wp_usermeta u WHERE u.meta_key =
 'last_activity' ORDER BY u.meta_value DESC LIMIT 0, 20 (57.3ms)

 SELECT COUNT(u.user_id) FROM wp_usermeta u WHERE u.meta_key =
 'last_activity' (28.3ms)
 }}}

 2. With 5128.patch applied:

 {{{
 SELECT id, user_id, date_recorded FROM wp_bp_activity WHERE component =
 'members' AND type = 'last_activity' AND user_id IN (1) LIMIT 1 (0.2ms)

 SELECT u.user_id as id FROM wp_bp_activity u WHERE u.component = 'members'
 AND u.type = 'last_activity' ORDER BY u.date_recorded DESC LIMIT 0, 20
 (0.2ms)

 SELECT COUNT(u.user_id) FROM wp_bp_activity u WHERE u.component =
 'members' AND u.type = 'last_activity' (30.4ms)

 SELECT id, user_id, date_recorded FROM wp_bp_activity WHERE component =
 'members' AND type = 'last_activity' AND user_id IN
 (1,26138,23108,18770,24279,24090,7478,5470,15216,2957,23229,3037,10118,17063,29675,28554,3776,10632,13248,16357)
 LIMIT 20 (0.3ms)
 }}}

 Basically, the main query went from about 57ms to .3ms. Roughly two orders
 of magnitude improvement. This will become more dramatic on larger sites
 (usermeta's performance will degrade in a greater-than-linear fashion),
 especially where usermeta is filled with lots of other junk from other
 plugins.

 In short, the performance implications are very significant, and we should
 move forward with this fix.

 '''Implementation Details'''

 1. I opted to use wp_bp_activity for storage. If I'd created my own table,
 it would've had the following columns: id, user_id, date_recorded, with
 all three columns indexed. The activity table already has these columns,
 and already has the indexes, so I thought it made sense to use it.
 Moreover, it might one day be useful to query for last_activity data
 *alongside other kinds of activity*; storing them in one table makes this
 possible. The performance implications of using wp_bp_activity, instead of
 a dedicated table, are very minimal.

 2. In order to use the activity table even when activity is disabled, I do
 the following:
     a. Register `$bp->table_prefix . 'activity'` as
 `$bp->members->table_name_last_activity` (so we're not dependent on
 `$bp->activity`)
     b. Install the activity tables even when the Activity component is
 disabled
     c. Provide my own custom database methods for querying the data (see
 `BP_Core_User::get_last_activity()` etc)

 In other words, I'm using the activity *table*, but I'm not using any of
 the activity component.

 3. I've built a migration tool (see bp-core/bp-core-update.php). For the
 moment, the tool leaves the usermeta data in place (this makes comparative
 benchmarking easier).

 4. I've swapped out most of the references to the 'last_activity' usermeta
 throughout the codebase.

 '''Further questions'''

 There are a few issues I wanted to get feedback on before moving forward.

 a. I haven't altered any of the half-dozen or so methods in `BP_Core_User`
 that make direct queries to wp_usermeta for the 'last_activity' data.
 These methods are no longer used in BuddyPress. On the other hand, they
 may still be in use in plugins etc, and the proposed changes will probably
 break them.

 b. Related: I've left the old usermeta data in place. Existing plugins may
 be relying on it. It may be wise not only to keep the old usermeta data
 there, but to mirror the new 'last_activity' data in usermeta going
 forward. That way, plugins that are reading this data in usermeta will
 continue to be able to do so (though it would be unwise for plugins to
 *update* this data). I'm torn on this issue. I'm leaning towards
 mirroring, but throwing big deprecated notices for anyone who attempts to
 query that data from usermeta.

 c. I haven't touched group or blog 'last_activity'. If we're satisfied
 with my treatment, I'll port those over too.

 d. I haven't implemented any caching yet. I'll do so once the first pass
 has been committed to trunk.

 Overall, I'm pretty satisfied with this patch - I think it's a huge
 improvement over what we've got, and I think it covers most backward
 compatibility/migration cases well - but would like feedback on the
 general strategy and the points above before committing. Thanks.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5128#comment:7>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac