-
Notifications
You must be signed in to change notification settings - Fork 3.3k
RTC: Transients for awareness and JIT meta cache flushing #11348
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.
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
25d0548
f217c2c
312f20d
e490c61
0347cff
cfe2768
bc58e2a
c4f6264
4ca7b2c
b8c0f4c
e13f24d
c809192
8eb0a0d
6d83d4e
4127634
4d680f5
f4f606f
fa16d93
e096bae
c97b742
835cce7
6cfb99b
f849007
dc57f84
2a4e730
f182903
242894b
d77d1c6
cbb9120
6d3a4e4
8b78349
470fa90
4a39f9a
0d67af4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2473,6 +2473,20 @@ public function get_posts() { | |||||||||||||||||
| $clauses = $this->meta_query->get_sql( 'post', $wpdb->posts, 'ID', $this ); | ||||||||||||||||||
| $join .= $clauses['join']; | ||||||||||||||||||
| $where .= $clauses['where']; | ||||||||||||||||||
|
|
||||||||||||||||||
| /* | ||||||||||||||||||
| * Determine if meta queries require just-in-time cache invalidation. | ||||||||||||||||||
| * | ||||||||||||||||||
| * If the most recent modification of post meta (addition, update or deletion) was of a meta key | ||||||||||||||||||
| * that uses jit cache invalidation, then the meta query cache will be considered stale and the | ||||||||||||||||||
| * post-query cache group flushed by changing the posts' last changed time. | ||||||||||||||||||
| * | ||||||||||||||||||
| * The cache is considered up to date if the `wp_query_meta_query_updated` cache key in the `post_meta` | ||||||||||||||||||
| * group returns `true`, otherwise the cache will be considered stale. | ||||||||||||||||||
| */ | ||||||||||||||||||
| if ( ! wp_cache_get( 'wp_query_meta_query_updated', 'post_meta' ) ) { | ||||||||||||||||||
peterwilsoncc marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+2483
to
+2487
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about expressing that last paragraph with code?
Suggested change
|
||||||||||||||||||
| wp_cache_set_posts_last_changed(); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| $rand = ( isset( $query_vars['orderby'] ) && 'rand' === $query_vars['orderby'] ); | ||||||||||||||||||
|
|
||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is storing things in both post-meta and transients, should it (and the class) have it's name changed? It also means that if a new DB table is added later, this class can be continue to be used as the interface for extenders. It could also be made it clear in a devnote (or inline comment?) that interacting with sync related data should be done through this class, not through any hooks called as a result of how it implements the storage. i.e. don't count on update_postmeta firing forever. This feels inline with the comment on the class being "Core class that provides an interface for storing and retrieving sync updates and awareness data during a collaborative session."
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // @todo pending everyone's feedback just in case the rename unlinks the comments. |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -30,7 +30,7 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage { | |||
| * @since 7.0.0 | ||||
| * @var string | ||||
| */ | ||||
| const AWARENESS_META_KEY = 'wp_sync_awareness_state'; | ||||
| const AWARENESS_TRANSIENT_PREFIX = 'wp_sync_awareness'; | ||||
|
|
||||
| /** | ||||
| * Meta key for sync updates. | ||||
|
|
@@ -69,73 +69,39 @@ class WP_Sync_Post_Meta_Storage implements WP_Sync_Storage { | |||
| * | ||||
| * @since 7.0.0 | ||||
| * | ||||
| * @global wpdb $wpdb WordPress database abstraction object. | ||||
| * | ||||
| * @param string $room Room identifier. | ||||
| * @param mixed $update Sync update. | ||||
| * @return bool True on success, false on failure. | ||||
| */ | ||||
| public function add_update( string $room, $update ): bool { | ||||
| global $wpdb; | ||||
|
|
||||
| $post_id = $this->get_storage_post_id( $room ); | ||||
| if ( null === $post_id ) { | ||||
| return false; | ||||
| } | ||||
|
|
||||
| // Use direct database operation to avoid cache invalidation performed by | ||||
| // post meta functions (`wp_cache_set_posts_last_changed()` and direct | ||||
| // `wp_cache_delete()` calls). | ||||
| return (bool) $wpdb->insert( | ||||
| $wpdb->postmeta, | ||||
| array( | ||||
| 'post_id' => $post_id, | ||||
| 'meta_key' => self::SYNC_UPDATE_META_KEY, | ||||
| 'meta_value' => wp_json_encode( $update ), | ||||
| ), | ||||
| array( '%d', '%s', '%s' ) | ||||
| ); | ||||
| $meta_id = add_post_meta( $post_id, wp_slash( self::SYNC_UPDATE_META_KEY ), wp_slash( $update ), false ); | ||||
|
|
||||
| return (bool) $meta_id; | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Gets awareness state for a given room. | ||||
| * | ||||
| * @since 7.0.0 | ||||
| * | ||||
| * @global wpdb $wpdb WordPress database abstraction object. | ||||
| * | ||||
| * @param string $room Room identifier. | ||||
| * @return array<int, mixed> Awareness state. | ||||
| */ | ||||
| public function get_awareness_state( string $room ): array { | ||||
| global $wpdb; | ||||
|
|
||||
| $post_id = $this->get_storage_post_id( $room ); | ||||
| if ( null === $post_id ) { | ||||
| return array(); | ||||
| } | ||||
|
|
||||
| // Use direct database operation to avoid updating the post meta cache. | ||||
| // ORDER BY meta_id DESC ensures the latest row wins if duplicates exist | ||||
| // from a past race condition in set_awareness_state(). | ||||
| $meta_value = $wpdb->get_var( | ||||
| $wpdb->prepare( | ||||
| "SELECT meta_value FROM $wpdb->postmeta WHERE post_id = %d AND meta_key = %s ORDER BY meta_id DESC LIMIT 1", | ||||
| $post_id, | ||||
| self::AWARENESS_META_KEY | ||||
| ) | ||||
| ); | ||||
|
|
||||
| if ( null === $meta_value ) { | ||||
| return array(); | ||||
| } | ||||
|
|
||||
| $awareness = json_decode( $meta_value, true ); | ||||
| $room_hash = md5( $room ); // Not used for cryptographic purposes. | ||||
peterwilsoncc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
| $awareness = get_transient( self::AWARENESS_TRANSIENT_PREFIX . ":{$room_hash}" ); | ||||
|
|
||||
| if ( ! is_array( $awareness ) ) { | ||||
| return array(); | ||||
| } | ||||
|
|
||||
| // Deterministic ordering of transient data. | ||||
| $awareness = wp_list_sort( $awareness, 'client_id' ); | ||||
| return array_values( $awareness ); | ||||
| } | ||||
|
|
||||
|
|
@@ -144,54 +110,24 @@ public function get_awareness_state( string $room ): array { | |||
| * | ||||
| * @since 7.0.0 | ||||
| * | ||||
| * @global wpdb $wpdb WordPress database abstraction object. | ||||
| * | ||||
| * @param string $room Room identifier. | ||||
| * @param array<int, mixed> $awareness Serializable awareness state. | ||||
| * @return bool True on success, false on failure. | ||||
| */ | ||||
| public function set_awareness_state( string $room, array $awareness ): bool { | ||||
| global $wpdb; | ||||
|
|
||||
| $post_id = $this->get_storage_post_id( $room ); | ||||
| if ( null === $post_id ) { | ||||
| return false; | ||||
| } | ||||
|
|
||||
| // Use direct database operation to avoid cache invalidation performed by | ||||
| // post meta functions (`wp_cache_set_posts_last_changed()` and direct | ||||
| // `wp_cache_delete()` calls). | ||||
| // | ||||
| // If two concurrent requests both see no row and both INSERT, the | ||||
| // duplicate is harmless: get_awareness_state() reads the latest row | ||||
| // (ORDER BY meta_id DESC). | ||||
| $meta_id = $wpdb->get_var( | ||||
| $wpdb->prepare( | ||||
| "SELECT meta_id FROM $wpdb->postmeta WHERE post_id = %d AND meta_key = %s ORDER BY meta_id DESC LIMIT 1", | ||||
| $post_id, | ||||
| self::AWARENESS_META_KEY | ||||
| ) | ||||
| ); | ||||
|
|
||||
| if ( $meta_id ) { | ||||
| return (bool) $wpdb->update( | ||||
| $wpdb->postmeta, | ||||
| array( 'meta_value' => wp_json_encode( $awareness ) ), | ||||
| array( 'meta_id' => $meta_id ), | ||||
| array( '%s' ), | ||||
| array( '%d' ) | ||||
| ); | ||||
| } | ||||
|
|
||||
| return (bool) $wpdb->insert( | ||||
| $wpdb->postmeta, | ||||
| array( | ||||
| 'post_id' => $post_id, | ||||
| 'meta_key' => self::AWARENESS_META_KEY, | ||||
| 'meta_value' => wp_json_encode( $awareness ), | ||||
| ), | ||||
| array( '%d', '%s', '%s' ) | ||||
| ); | ||||
| $room_hash = md5( $room ); // Not used for cryptographic purposes. | ||||
| // Deterministic ordering of transient data. | ||||
| $awareness = wp_list_sort( $awareness, 'client_id' ); | ||||
|
|
||||
| /* | ||||
| * Maintain transient for longer than awareness. | ||||
| * | ||||
| * Recently used rooms are more likely to be used again soon. Maintaining the | ||||
| * transient longer than the awareness can avoid adding new entries in the options | ||||
| * table unnecessarily. | ||||
| */ | ||||
| set_transient( self::AWARENESS_TRANSIENT_PREFIX . ":{$room_hash}", $awareness, DAY_IN_SECONDS ); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the race description you are talking about https://core.trac.wordpress.org/ticket/63450 or something different?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamziel wordpress-develop/src/wp-includes/option.php Line 1140 in 85cc3de
I've done some testing on what happens if the transient disappears using
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @peterwilsoncc ah, I've missed that |
||||
| return true; | ||||
| } | ||||
|
|
||||
| /** | ||||
|
|
@@ -281,8 +217,6 @@ public function get_update_count( string $room ): int { | |||
| * | ||||
| * @since 7.0.0 | ||||
| * | ||||
| * @global wpdb $wpdb WordPress database abstraction object. | ||||
| * | ||||
| * @param string $room Room identifier. | ||||
| * @param int $cursor Return updates after this cursor (meta_id). | ||||
| * @return array<int, mixed> Sync updates. | ||||
|
|
@@ -332,10 +266,7 @@ public function get_updates_after_cursor( string $room, int $cursor ): array { | |||
|
|
||||
| $updates = array(); | ||||
| foreach ( $rows as $row ) { | ||||
| $decoded = json_decode( $row->meta_value, true ); | ||||
| if ( null !== $decoded ) { | ||||
| $updates[] = $decoded; | ||||
| } | ||||
| $updates[] = maybe_unserialize( $row->meta_value ); | ||||
| } | ||||
|
|
||||
| return $updates; | ||||
|
|
@@ -346,8 +277,6 @@ public function get_updates_after_cursor( string $room, int $cursor ): array { | |||
| * | ||||
| * @since 7.0.0 | ||||
| * | ||||
| * @global wpdb $wpdb WordPress database abstraction object. | ||||
| * | ||||
| * @param string $room Room identifier. | ||||
| * @param int $cursor Remove updates with meta_id < this cursor. | ||||
| * @return bool True on success, false on failure. | ||||
|
|
@@ -373,6 +302,7 @@ public function remove_updates_before_cursor( string $room, int $cursor ): bool | |||
| return false; | ||||
| } | ||||
|
|
||||
| wp_cache_maybe_set_posts_last_changed_following_post_meta_update( array(), $post_id, self::SYNC_UPDATE_META_KEY ); | ||||
| return true; | ||||
| } | ||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1396,6 +1396,7 @@ function sanitize_meta( $meta_key, $meta_value, $object_type, $object_subtype = | |
| * @since 5.5.0 The `$default` argument was added to the arguments array. | ||
| * @since 6.4.0 The `$revisions_enabled` argument was added to the arguments array. | ||
| * @since 6.7.0 The `label` argument was added to the arguments array. | ||
| * @since 7.0.0 The `$jit_cache_invalidation` argument was added to the arguments array. | ||
| * | ||
| * @global array $wp_meta_keys Global registry for meta keys. | ||
| * | ||
|
|
@@ -1405,27 +1406,30 @@ function sanitize_meta( $meta_key, $meta_value, $object_type, $object_subtype = | |
| * @param array $args { | ||
| * Data used to describe the meta key when registered. | ||
| * | ||
| * @type string $object_subtype A subtype; e.g. if the object type is "post", the post type. If left empty, | ||
| * the meta key will be registered on the entire object type. Default empty. | ||
| * @type string $type The type of data associated with this meta key. | ||
| * Valid values are 'string', 'boolean', 'integer', 'number', 'array', and 'object'. | ||
| * @type string $label A human-readable label of the data attached to this meta key. | ||
| * @type string $description A description of the data attached to this meta key. | ||
| * @type bool $single Whether the meta key has one value per object, or an array of values per object. | ||
| * @type mixed $default The default value returned from get_metadata() if no value has been set yet. | ||
| * When using a non-single meta key, the default value is for the first entry. | ||
| * In other words, when calling get_metadata() with `$single` set to `false`, | ||
| * the default value given here will be wrapped in an array. | ||
| * @type callable $sanitize_callback A function or method to call when sanitizing `$meta_key` data. | ||
| * @type callable $auth_callback Optional. A function or method to call when performing edit_post_meta, | ||
| * add_post_meta, and delete_post_meta capability checks. | ||
| * @type bool|array $show_in_rest Whether data associated with this meta key can be considered public and | ||
| * should be accessible via the REST API. A custom post type must also declare | ||
| * support for custom fields for registered meta to be accessible via REST. | ||
| * When registering complex meta values this argument may optionally be an | ||
| * array with 'schema' or 'prepare_callback' keys instead of a boolean. | ||
| * @type bool $revisions_enabled Whether to enable revisions support for this meta_key. Can only be used when the | ||
| * object type is 'post'. | ||
| * @type string $object_subtype A subtype; e.g. if the object type is "post", the post type. If left empty, | ||
| * the meta key will be registered on the entire object type. Default empty. | ||
| * @type string $type The type of data associated with this meta key. | ||
| * Valid values are 'string', 'boolean', 'integer', 'number', 'array', and 'object'. | ||
| * @type string $label A human-readable label of the data attached to this meta key. | ||
| * @type string $description A description of the data attached to this meta key. | ||
| * @type bool $single Whether the meta key has one value per object, or an array of values per object. | ||
| * @type mixed $default The default value returned from get_metadata() if no value has been set yet. | ||
| * When using a non-single meta key, the default value is for the first entry. | ||
| * In other words, when calling get_metadata() with `$single` set to `false`, | ||
| * the default value given here will be wrapped in an array. | ||
| * @type callable $sanitize_callback A function or method to call when sanitizing `$meta_key` data. | ||
| * @type callable $auth_callback Optional. A function or method to call when performing edit_post_meta, | ||
| * add_post_meta, and delete_post_meta capability checks. | ||
| * @type bool|array $show_in_rest Whether data associated with this meta key can be considered public and | ||
| * should be accessible via the REST API. A custom post type must also declare | ||
| * support for custom fields for registered meta to be accessible via REST. | ||
| * When registering complex meta values this argument may optionally be an | ||
| * array with 'schema' or 'prepare_callback' keys instead of a boolean. | ||
| * @type bool $revisions_enabled Whether to enable revisions support for this meta_key. Can only be used when the | ||
| * object type is 'post'. | ||
| * @type bool $jit_cache_invalidation Whether to enable just-in-time cache invalidation for this meta key. When enabled, the cache | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kittenkamala: This addition to post_meta should likely get a mention in the misc changes devnote. |
||
| * for WP_Query will be invalidated the next time a meta query is run. Can only be used when the | ||
| * object type is 'post'. | ||
| * } | ||
| * @param string|array $deprecated Deprecated. Use `$args` instead. | ||
| * @return bool True if the meta key was successfully registered in the global array, false if not. | ||
|
|
@@ -1450,6 +1454,7 @@ function register_meta( $object_type, $meta_key, $args, $deprecated = null ) { | |
| 'auth_callback' => null, | ||
| 'show_in_rest' => false, | ||
| 'revisions_enabled' => false, | ||
| 'jit_cache_invalidation' => false, | ||
| ); | ||
|
|
||
| // There used to be individual args for sanitize and auth callbacks. | ||
|
|
@@ -1508,6 +1513,12 @@ function register_meta( $object_type, $meta_key, $args, $deprecated = null ) { | |
| } | ||
| } | ||
|
|
||
| if ( $args['jit_cache_invalidation'] && 'post' !== $object_type ) { | ||
| _doing_it_wrong( __FUNCTION__, __( 'Just-in-time cache invalidation for meta keys is only supported with the "post" object type.' ), '7.0.0' ); | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| // If `auth_callback` is not provided, fall back to `is_protected_meta()`. | ||
| if ( empty( $args['auth_callback'] ) ) { | ||
| if ( is_protected_meta( $meta_key, $object_type ) ) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about the following rewrite? It makes things click for my by providing the larger context: