Skip to content

fix(entry-query): remove get_all_entries_asc#937

Open
faisalahammad wants to merge 1 commit into
Automattic:developfrom
faisalahammad:fix/591-remove-get-all-entries-asc
Open

fix(entry-query): remove get_all_entries_asc#937
faisalahammad wants to merge 1 commit into
Automattic:developfrom
faisalahammad:fix/591-remove-get-all-entries-asc

Conversation

@faisalahammad

Copy link
Copy Markdown

Summary

get_all_entries_asc() cached a Liveblog post's entire entry list under one cache key. On posts with a lot of entries that blob can exceed the memcache 1MB object size limit, so the write silently fails and every request falls back to a full uncached DB read - exactly the scaling problem this was meant to avoid. This replaces it with direct, date-bounded DB queries and a small cached count for pagination.

Fixes #591

Changes

classes/class-wpcom-liveblog-entry-query.php

Before:

public function get_between_timestamps( $start_timestamp, $end_timestamp ) {
	$all_entries = $this->get_all_entries_asc();
	return $this->find_between_timestamps( $all_entries, $start_timestamp, $end_timestamp );
}

public function get_all_entries_asc() {
	$cached_entries_asc_key = $this->key . '_entries_asc_' . $this->post_id;
	$cached_entries_asc     = wp_cache_get( $cached_entries_asc_key, 'liveblog' );
	if ( false !== $cached_entries_asc ) {
		return $cached_entries_asc;
	}
	$all_entries_asc = $this->get( array( 'order' => 'ASC' ) );
	wp_cache_set( $cached_entries_asc_key, $all_entries_asc, 'liveblog' );
	return $all_entries_asc;
}

After:

public function get_between_timestamps( $start_timestamp, $end_timestamp ) {
	$args = array(
		'order'      => 'ASC',
		'date_query' => array(
			'column'    => 'comment_date_gmt',
			'after'     => gmdate( 'Y-m-d H:i:s', $start_timestamp ),
			'before'    => gmdate( 'Y-m-d H:i:s', $end_timestamp ),
			'inclusive' => true,
		),
	);
	$entries = $this->get( $args );
	return self::remove_replaced_entries( $entries );
}

public function count_entries() {
	// keys a small cached count off the highest comment ID (not timestamp,
	// so same-second inserts can't collide), excludes deletions and
	// original entries that have since been replaced by an edit or delete
	...
}

Why: queries the requested time range directly from the DB instead of loading and caching the whole entry list. count_entries() gives get_entries_by_time() a correctly deduplicated pagination count without needing the full list either.

liveblog.php

Before:

$all_entries = self::$entry_query->get_all_entries_asc();
$entries     = self::$entry_query->find_between_timestamps( $all_entries, $start_timestamp, $end_timestamp );
...
$pages = ceil( count( self::flatten_entries( $all_entries ) ) / $per_page );

After:

$entries = self::$entry_query->get_between_timestamps( $start_timestamp, $end_timestamp );
...
$pages = ceil( self::$entry_query->count_entries() / $per_page );

Why: get_entries_by_time() is hit on every polling request, so it's the method that actually blows past the cache size limit on large liveblogs. get_entries_paged() was changed similarly, just querying the full ASC list directly instead of caching it as one object - its pagination math is unchanged.

Testing

Test 1: date-bounded query returns the right entries

  1. Added entries in tests/Integration/EntryQueryTest.php covering get_between_timestamps() against the new date_query path (mid-range, border timestamps)
    Result: passes, same coverage as before plus new count_entries() tests for inserts, deletions, and edits.

Test 2: pagination unaffected

  1. Added tests/Integration/RestApiTest.php coverage for get_entries_paged() - page slicing, empty state, deleted entries excluded from total
    Result: passes

Test 3: full suite

  1. composer test:unit - 7 tests, composer test:integration - 166 tests, composer cs on changed files, composer lint
    Result: all pass, 0 PHPCS errors/warnings on changed files

Screenshots

N/A, no UI changes.

- get_entries_by_time() now queries entries directly with a date-bounded
  DB query instead of caching and slicing the full entry list
- get_entries_paged() queries the full list directly instead of caching
  it as one object
- add count_entries() for cheap, correctly deduplicated pagination counts

get_all_entries_asc() cached a Liveblog post's whole entry list under one
cache key. On posts with a lot of entries that blob can exceed the
memcache 1MB object size limit, so the write silently fails and every
request falls back to a full uncached DB read, which is exactly the
scaling problem this was meant to avoid.

Fixes Automattic#591
@faisalahammad faisalahammad requested a review from a team as a code owner July 4, 2026 09:06
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.

Remove WPCOM_Liveblog_Entry_Query::get_all_entries_asc()

1 participant