Skip to content

Rebased: Suspend calling wp_cache_set_posts_last_changed() when touching post meta in WP_Sync_Post_Meta_Storage #11320

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.

Sign up for GitHub

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

Closed
chriszarate wants to merge 6 commits into from
+286 −3
Closed

Rebased: Suspend calling wp_cache_set_posts_last_changed() when touching post meta in WP_Sync_Post_Meta_Storage #11320

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9311711
Prevent calling wp_cache_set_posts_last_changed() when touching post …
westonruter Feb 23, 2026
fba4b2a
Suspend via a special global
chriszarate Mar 20, 2026
4498e29
Inline global setting
chriszarate Mar 20, 2026
98b851a
Use counter approach
chriszarate Mar 20, 2026
dc85bf7
Add unit tests for wp_cache_set_last_changed
chriszarate Mar 20, 2026
178a599
Merge remote-tracking branch 'origin/trunk' into fix/rtc-suspended-po…
chriszarate Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions src/wp-includes/collaboration/class-wp-sync-post-meta-storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ public function add_update( string $room, $update ): bool {
return false;
}

$meta_id = add_post_meta( $post_id, self::SYNC_UPDATE_META_KEY, $update, false );

return (bool) $meta_id;
return $this->with_suspended_posts_last_changed_update(
fn() => (bool) add_post_meta( $post_id, self::SYNC_UPDATE_META_KEY, $update, false )
);
}

/**
Expand Down Expand Up @@ -123,7 +123,10 @@ public function set_awareness_state( string $room, array $awareness ): bool {
}

// update_post_meta returns false if the value is the same as the existing value.
update_post_meta( $post_id, wp_slash( self::AWARENESS_META_KEY ), wp_slash( $awareness ) );
$this->with_suspended_posts_last_changed_update(
fn() => update_post_meta( $post_id, self::AWARENESS_META_KEY, wp_slash( $awareness ) )
Copy link
Contributor

@adamziel adamziel Mar 20, 2026

Choose a reason for hiding this comment

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

Do we need that private method actually? Or should we just inline it here?

Copy link
Author

@chriszarate chriszarate Mar 20, 2026

Choose a reason for hiding this comment

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

If we don't inline it, then we can avoid the need for try/catch and a counter approach, so probably worth it.

Copy link
Author

@chriszarate chriszarate Mar 20, 2026

Choose a reason for hiding this comment

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

the try/catch is extra safe, though, so i'll keep it

Copy link
Contributor

@adamziel adamziel Mar 20, 2026

Choose a reason for hiding this comment

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

I'm not sure if I follow how could we avoid try/catch and a counter – do you mean we can avoid duplicating that code in two places? Yeah that sounds good to me. Or do you mean we could avoid that entirely? In which case I'm super curious.

Copy link
Author

@chriszarate chriszarate Mar 20, 2026

Choose a reason for hiding this comment

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

Ignore this, I was misunderstanding what you meant by "call inside of a call"

);

return true;
}

Expand Down Expand Up @@ -299,4 +302,25 @@ public function remove_updates_before_cursor( string $room, int $cursor ): bool

return true;
}

/**
* Invokes the provided callback while the suspending setting the posts
* last_changed cache key via a special global flag.
*
* @since 7.0.0
* @see wp_cache_set_posts_last_changed()
*
* @template T
* @param Closure(): T $callback Callback.
* @return T Return value from the callback.
*/
private function with_suspended_posts_last_changed_update( Closure $callback ) {
$GLOBALS['__suspend_posts_last_changed_update'] = true;
Copy link
Contributor

@adamziel adamziel Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Nice! I'd just use a counter here, such as

Suggested change
$GLOBALS['__suspend_posts_last_changed_update'] = true;
++$GLOBALS['__suspend_posts_last_changed_update'];

and later on

		--$GLOBALS['__suspend_posts_last_changed_update'];

with a check such as

		0 === $GLOBALS['__suspend_posts_last_changed_update']

just in case there's a call inside a call for whatever weird reason.


try {
return $callback();
} finally {
$GLOBALS['__suspend_posts_last_changed_update'] = false;
Copy link
Member

@tyxla tyxla Mar 20, 2026

Choose a reason for hiding this comment

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

We should also update this one if we end up using it as a counter

}
}
}
4 changes: 4 additions & 0 deletions src/wp-includes/post.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8443,6 +8443,10 @@ function wp_add_trashed_suffix_to_post_name_for_post( $post ) {
* @since 5.0.0
*/
function wp_cache_set_posts_last_changed() {
if ( isset( $GLOBALS['__suspend_posts_last_changed_update'] ) && true === $GLOBALS['__suspend_posts_last_changed_update'] ) {
return;
}

wp_cache_set_last_changed( 'posts' );
}

Expand Down
Loading