fix(sidecar): protocol 13 — a configuration write no longer waits for its reload (F9) #18
@@ -101,11 +101,9 @@ use tracing_subscriber::EnvFilter;
|
|||||||
/// for the reload to announce itself, and **puts the old files back automatically** if it does
|
/// for the reload to announce itself, and **puts the old files back automatically** if it does
|
||||||
/// not.
|
/// not.
|
||||||
///
|
///
|
||||||
/// Two things about that reach this process. The write is the only route here that causes a write
|
/// The write is the only route here that causes a write on the game host. Since protocol 13 its
|
||||||
/// on the game host, and it is the only one whose reply routinely spends seconds rather than
|
/// reply comes back as soon as the files are on disk, marked `pending`; how the reload went follows
|
||||||
/// milliseconds — the plugin holds the correlation open across a reload and, at worst, across a
|
/// later as a `config.outcome` event, which this process files and feeds like any other.
|
||||||
/// rollback as well. `web::CONFIG_RELOAD_WINDOW` is that budget, mirrored from the plugin, and a
|
|
||||||
/// test asserts the pairing rather than trusting it.
|
|
||||||
///
|
///
|
||||||
/// # Protocol 6 — first-party clans
|
/// # Protocol 6 — first-party clans
|
||||||
///
|
///
|
||||||
@@ -165,10 +163,19 @@ use tracing_subscriber::EnvFilter;
|
|||||||
/// keeps it, and positions are asked for while somebody is looking and never touch the database
|
/// keeps it, and positions are asked for while somebody is looking and never touch the database
|
||||||
/// (D111). Which layer a viewer may see is decided on the website, never here.
|
/// (D111). Which layer a viewer may see is decided on the website, never here.
|
||||||
///
|
///
|
||||||
|
/// # Protocol 13 — the first player walk's fixes
|
||||||
|
///
|
||||||
|
/// Opened by the configuration save (the module's PLAN_FIXES.md F9, D177): `POST /config/write`
|
||||||
|
/// answers `pending` at once instead of holding the RPC across a reload, and the outcome arrives
|
||||||
|
/// as a `config.outcome` event. Nothing new to route — an event is filed and fed by its `type`,
|
||||||
|
/// whatever its kind — so what changes here is what no longer has to fit: the reload window that
|
||||||
|
/// had to sit inside [`rpc::REPLY_TIMEOUT`] is gone.
|
||||||
|
///
|
||||||
/// `docs/rust-link/PROTOCOL.md` is the specification — §8 the read path, §9 identity, §10 the
|
/// `docs/rust-link/PROTOCOL.md` is the specification — §8 the read path, §9 identity, §10 the
|
||||||
/// mirror, §11 configuration, §12 clans, §13 the raid frame, §14 the leases, §15 the world verbs,
|
/// mirror, §11 configuration, §12 clans, §13 the raid frame, §14 the leases, §15 the world verbs,
|
||||||
/// §16 the rewards, §17 the map, §18 the optional mods; this constant is one of its four declaration sites.
|
/// §16 the rewards, §17 the map, §18 the optional mods, §19 the walk's fixes; this constant is one
|
||||||
pub const PROTOCOL_VERSION: u32 = 12;
|
/// of its four declaration sites.
|
||||||
|
pub const PROTOCOL_VERSION: u32 = 13;
|
||||||
|
|
||||||
fn main() -> anyhow::Result<()> {
|
fn main() -> anyhow::Result<()> {
|
||||||
let args = match cli::parse(std::env::args().skip(1)) {
|
let args = match cli::parse(std::env::args().skip(1)) {
|
||||||
|
|||||||
@@ -102,9 +102,8 @@ pub async fn serve(addr: &str, state: AppState) -> anyhow::Result<()> {
|
|||||||
// an operator edited over SSH and the website then overwrote.
|
// an operator edited over SSH and the website then overwrote.
|
||||||
.route("/config/files", get(config_files))
|
.route("/config/files", get(config_files))
|
||||||
.route("/config/file", get(config_file))
|
.route("/config/file", get(config_file))
|
||||||
// The only route on this sidecar that causes a WRITE on the game host, and the only one
|
// The only route on this sidecar that causes a WRITE on the game host. It answers once the
|
||||||
// whose reply can take most of the RPC budget: the plugin holds it open across a reload
|
// files are on disk; the reload's outcome follows as a `config.outcome` event (protocol 13).
|
||||||
// and, at worst, across a rollback as well. See `CONFIG_RELOAD_WINDOW`.
|
|
||||||
.route("/config/write", post(config_write))
|
.route("/config/write", post(config_write))
|
||||||
// Protocol 8 (the module's PLAN.md §27): an event borrowing a value and giving it back.
|
// Protocol 8 (the module's PLAN.md §27): an event borrowing a value and giving it back.
|
||||||
// Three correlated round trips, and the sidecar knows nothing about any of them — not
|
// Three correlated round trips, and the sidecar knows nothing about any of them — not
|
||||||
@@ -694,19 +693,14 @@ async fn config_file(State(st): State<AppState>, Query(q): Query<ConfigPathQuery
|
|||||||
respond(st.rpc.call(&st.game, command, &req_id).await)
|
respond(st.rpc.call(&st.game, command, &req_id).await)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The window the plugin gives a reload to announce itself, mirrored here from
|
|
||||||
/// `ConfigReloadWindowSeconds` in `overlay/oxide/plugins/RunicGateway.cs`.
|
|
||||||
///
|
|
||||||
/// It is duplicated rather than negotiated because the two numbers are a *pairing*, like the
|
|
||||||
/// protocol version: the plugin owns the behaviour and this end owns the budget it has to fit in.
|
|
||||||
/// The test below is what keeps them honest — a worst-case write is two windows, and a
|
|
||||||
/// [`REPLY_TIMEOUT`](crate::rpc::REPLY_TIMEOUT) that does not cover both would abandon the caller
|
|
||||||
/// precisely when a rollback had just saved them, leaving the website to report a timeout over a
|
|
||||||
/// server that is perfectly healthy.
|
|
||||||
pub const CONFIG_RELOAD_WINDOW: Duration = Duration::from_secs(4);
|
|
||||||
|
|
||||||
/// Replace a set of configuration files and reload whatever owns them.
|
/// Replace a set of configuration files and reload whatever owns them.
|
||||||
///
|
///
|
||||||
|
/// Since protocol 13 the reply does not wait for the reload. The plugin answers once the files are
|
||||||
|
/// on disk (`pending: true`, a `writeId`, and the `ceilingMs` it will wait), and reports the reload
|
||||||
|
/// as a `config.outcome` event when it settles. Protocol 12 held this RPC across the reload, which
|
||||||
|
/// forced the plugin's window under [`REPLY_TIMEOUT`](crate::rpc::REPLY_TIMEOUT) — and a cold
|
||||||
|
/// compile outlasted it and had a valid edit rolled back (the module's PLAN_FIXES.md F9).
|
||||||
|
///
|
||||||
/// Opaque body, exactly as `/permissions/sync`: `cmd` and `reqId` are written over whatever the
|
/// Opaque body, exactly as `/permissions/sync`: `cmd` and `reqId` are written over whatever the
|
||||||
/// caller sent, and nothing else about the object is read here. What the website sends is whole
|
/// caller sent, and nothing else about the object is read here. What the website sends is whole
|
||||||
/// file *text* rather than a key and a value (D35), so there is nothing on this hop that could
|
/// file *text* rather than a key and a value (D35), so there is nothing on this hop that could
|
||||||
@@ -750,16 +744,15 @@ async fn config_write(State(st): State<AppState>, Json(body): Json<Value>) -> Re
|
|||||||
let result = st.rpc.call(&st.game, command, &req_id).await;
|
let result = st.rpc.call(&st.game, command, &req_id).await;
|
||||||
|
|
||||||
if matches!(result, Err(RpcError::Timeout)) {
|
if matches!(result, Err(RpcError::Timeout)) {
|
||||||
// A bare `504` on a route that writes reads as "did my change land, and is the plugin
|
// A bare `504` on a route that writes reads as "did my change land?" — and unlike every
|
||||||
// still up?" — and unlike every other timeout on this sidecar, that question has a good
|
// other timeout on this sidecar, that question has a good answer. The plugin writes a
|
||||||
// answer. The plugin writes a whole set or restores a whole set, never half of either, so
|
// whole set or none of it, and restores a whole set or none of it, so a re-read settles
|
||||||
// a re-read settles it; and a write that is still in flight is most likely inside the
|
// it. Since protocol 13 the reply no longer waits for a reload, so this is a slow host or
|
||||||
// rollback this window pays for.
|
// a busy main thread, not a rollback in progress.
|
||||||
return (
|
return (
|
||||||
StatusCode::GATEWAY_TIMEOUT,
|
StatusCode::GATEWAY_TIMEOUT,
|
||||||
Json(json!({
|
Json(json!({
|
||||||
"error": "the plugin did not report within the write budget",
|
"error": "the plugin did not report within the write budget",
|
||||||
"reloadWindowSeconds": CONFIG_RELOAD_WINDOW.as_secs(),
|
|
||||||
"hint": "re-read the files: the plugin writes the whole set or restores it",
|
"hint": "re-read the files: the plugin writes the whole set or restores it",
|
||||||
})),
|
})),
|
||||||
)
|
)
|
||||||
@@ -1037,25 +1030,6 @@ mod tests {
|
|||||||
assert_eq!(command["grants"], json!([]));
|
assert_eq!(command["grants"], json!([]));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A command larger than the game link's own line cap is refused here, where the caller learns
|
|
||||||
/// why. Forwarded, it would be discarded by both ends without a word and present as a `504`.
|
|
||||||
/// The pairing in `CONFIG_RELOAD_WINDOW`'s own words, asserted rather than trusted.
|
|
||||||
///
|
|
||||||
/// The worst path through one configuration write is two windows — wait for the edited
|
|
||||||
/// plugin's reload, give up, restore the files, reload again — plus the write, the reads and
|
|
||||||
/// a line each way. If that does not fit inside the RPC timeout, the caller is abandoned at
|
|
||||||
/// exactly the moment the rollback saved it, and the website reports a timeout over a server
|
|
||||||
/// that is healthy and has the old config back.
|
|
||||||
#[test]
|
|
||||||
fn a_rollback_fits_inside_the_rpc_budget() {
|
|
||||||
let worst = CONFIG_RELOAD_WINDOW * 2;
|
|
||||||
assert!(
|
|
||||||
worst + Duration::from_secs(1) <= crate::rpc::REPLY_TIMEOUT,
|
|
||||||
"two reload windows ({worst:?}) plus a second of slack must fit in {:?}",
|
|
||||||
crate::rpc::REPLY_TIMEOUT
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Protocol 8's routes cannot choose their own command or correlation id either, and they keep
|
/// Protocol 8's routes cannot choose their own command or correlation id either, and they keep
|
||||||
/// everything else the caller sent, unread.
|
/// everything else the caller sent, unread.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in New Issue
Block a user