fix(#2851): skip the state PUT when unchanged — it still emits an event
set_room_state PUT unconditionally and its doc called that idempotent. It is, one level too high: a PUT of identical content is a no-op on the room's STATE and the homeserver still appends an event to the TIMELINE. Downstream an event is unread activity, then a todo, then a turn — for every agent in the room. The provisioning sweep re-wiring the hive Space's child link therefore woke the whole hive on the sweep's cadence, forever, for a link that never changed (~1801s between events, measured across eleven consecutive intervals). Read the current content first and return early when it matches. The lookup fails open — an unreadable state means write, because the re-apply exists to repair a missing link and "don't know" must not be treated as "fine". Only the steady state goes quiet.
This commit is contained in:
parent
462f353c10
commit
81292f14b6
1 changed files with 93 additions and 7 deletions
|
|
@ -1218,8 +1218,47 @@ async fn find_chat_room_by_name(client: &reqwest::Client, admin_token: &str) ->
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
/// PUT a state event into `room_id` using the admin token. Idempotent —
|
/// Whether the state event still has to be written.
|
||||||
/// re-sending identical content is a no-op on the homeserver.
|
///
|
||||||
|
/// `current` is the room's existing content for this `(type, state_key)`,
|
||||||
|
/// or `None` when it could not be read. **Unreadable means write**: the
|
||||||
|
/// re-apply exists so a missing link gets repaired, and one redundant
|
||||||
|
/// event is cheaper than a hierarchy that never reconverges.
|
||||||
|
///
|
||||||
|
/// Comparison is `serde_json::Value` equality, which compares objects as
|
||||||
|
/// maps — key order in the response does not matter.
|
||||||
|
fn state_needs_write(current: Option<&serde_json::Value>, desired: &serde_json::Value) -> bool {
|
||||||
|
current != Some(desired)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Read the room's current content for one state event. `None` on any
|
||||||
|
/// failure — absent state, transport error, or an unparseable body are
|
||||||
|
/// all "we don't know", and [`state_needs_write`] turns that into a write.
|
||||||
|
async fn current_room_state(
|
||||||
|
client: &reqwest::Client,
|
||||||
|
admin_token: &str,
|
||||||
|
url: &str,
|
||||||
|
) -> Option<serde_json::Value> {
|
||||||
|
let resp = client.get(url).bearer_auth(admin_token).send().await.ok()?;
|
||||||
|
if !resp.status().is_success() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
resp.json::<serde_json::Value>().await.ok()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// PUT a state event into `room_id` using the admin token, **skipping the
|
||||||
|
/// write when the room already carries identical content**.
|
||||||
|
///
|
||||||
|
/// The read is not an optimisation. A PUT of identical content is a no-op
|
||||||
|
/// on the room's *state*, and the homeserver still appends an event to the
|
||||||
|
/// *timeline* — so "idempotent" was true one level too high. Downstream,
|
||||||
|
/// an event is unread activity, which is a todo, which is a turn: a caller
|
||||||
|
/// re-applying a link on a periodic sweep wakes every agent in the room on
|
||||||
|
/// that sweep's cadence, forever. Measured at ~30 minutes per wake per
|
||||||
|
/// agent before this guard existed.
|
||||||
|
///
|
||||||
|
/// The self-healing property the re-apply exists for is unaffected: a
|
||||||
|
/// missing or divergent link still gets written.
|
||||||
async fn set_room_state(
|
async fn set_room_state(
|
||||||
client: &reqwest::Client,
|
client: &reqwest::Client,
|
||||||
admin_token: &str,
|
admin_token: &str,
|
||||||
|
|
@ -1233,6 +1272,15 @@ async fn set_room_state(
|
||||||
let encoded_key = encode_room_id_for_url(state_key);
|
let encoded_key = encode_room_id_for_url(state_key);
|
||||||
let url =
|
let url =
|
||||||
format!("{base}/_matrix/client/v3/rooms/{encoded_room}/state/{event_type}/{encoded_key}");
|
format!("{base}/_matrix/client/v3/rooms/{encoded_room}/state/{event_type}/{encoded_key}");
|
||||||
|
let current = current_room_state(client, admin_token, &url).await;
|
||||||
|
if !state_needs_write(current.as_ref(), content) {
|
||||||
|
tracing::debug!(
|
||||||
|
%room_id,
|
||||||
|
event_type,
|
||||||
|
"matrix: state already current, skipping PUT (no timeline event)"
|
||||||
|
);
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
let resp = client
|
let resp = client
|
||||||
.put(&url)
|
.put(&url)
|
||||||
.bearer_auth(admin_token)
|
.bearer_auth(admin_token)
|
||||||
|
|
@ -1261,8 +1309,9 @@ async fn set_room_state(
|
||||||
/// join instead of an empty Space.
|
/// join instead of an empty Space.
|
||||||
///
|
///
|
||||||
/// Dedup mirrors [`ensure_hive_space`]: persisted id wins, else rediscover
|
/// Dedup mirrors [`ensure_hive_space`]: persisted id wins, else rediscover
|
||||||
/// by name, else create. The space-child link is re-applied on every call
|
/// by name, else create. The space-child link is re-checked on every call
|
||||||
/// (idempotent PUT) so a recovered room reconverges its hierarchy link.
|
/// so a recovered room reconverges its hierarchy link — and written only
|
||||||
|
/// when it differs, see [`set_room_state`].
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
|
|
@ -1347,9 +1396,9 @@ pub async fn ensure_hive_chat_room(
|
||||||
id
|
id
|
||||||
};
|
};
|
||||||
|
|
||||||
// Wire the Space → room child link (idempotent). Without an
|
// Wire the Space → room child link. Without an `m.space.child` carrying
|
||||||
// `m.space.child` carrying a `via`, the room won't surface in the Space
|
// a `via`, the room won't surface in the Space hierarchy. `suggested`
|
||||||
// hierarchy. `suggested` hints clients to surface it prominently.
|
// hints clients to surface it prominently.
|
||||||
let child_content = serde_json::json!({
|
let child_content = serde_json::json!({
|
||||||
"via": [server_name],
|
"via": [server_name],
|
||||||
"suggested": true,
|
"suggested": true,
|
||||||
|
|
@ -1700,6 +1749,43 @@ mod tests {
|
||||||
assert!(h.chars().all(|c| c.is_ascii_hexdigit()));
|
assert!(h.chars().all(|c| c.is_ascii_hexdigit()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The steady state. This is the whole point of the guard: the sweep
|
||||||
|
/// runs forever, and every write it makes is a wake for every agent
|
||||||
|
/// in the room.
|
||||||
|
#[test]
|
||||||
|
fn state_matching_the_room_is_not_rewritten() {
|
||||||
|
let desired = serde_json::json!({ "via": ["example.org"], "suggested": true });
|
||||||
|
let current = desired.clone();
|
||||||
|
assert!(!state_needs_write(Some(¤t), &desired));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Key order in the homeserver's response must not force a write —
|
||||||
|
/// the guard leans on `serde_json::Value` comparing objects as maps,
|
||||||
|
/// and a byte- or order-sensitive comparison would silently degrade
|
||||||
|
/// to writing every time while still looking correct.
|
||||||
|
#[test]
|
||||||
|
fn state_matching_but_reordered_is_not_rewritten() {
|
||||||
|
let desired = serde_json::json!({ "via": ["example.org"], "suggested": true });
|
||||||
|
let current = serde_json::json!({ "suggested": true, "via": ["example.org"] });
|
||||||
|
assert!(!state_needs_write(Some(¤t), &desired));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn diverged_state_is_rewritten() {
|
||||||
|
let desired = serde_json::json!({ "via": ["example.org"], "suggested": true });
|
||||||
|
let current = serde_json::json!({ "via": ["old.example.org"], "suggested": true });
|
||||||
|
assert!(state_needs_write(Some(¤t), &desired));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Fail-open, and deliberately so: an unreadable current state must
|
||||||
|
/// write. The re-apply exists to repair a missing link, so "we don't
|
||||||
|
/// know" has to behave like "it's missing", not like "it's fine".
|
||||||
|
#[test]
|
||||||
|
fn unknown_state_is_written() {
|
||||||
|
let desired = serde_json::json!({ "via": ["example.org"], "suggested": true });
|
||||||
|
assert!(state_needs_write(None, &desired));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn random_hex_two_calls_differ() {
|
fn random_hex_two_calls_differ() {
|
||||||
// Sanity check — not a statistical claim, just guards
|
// Sanity check — not a statistical claim, just guards
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue