Skip to content

Commit a6119af

Browse files
authored
The lane reads with the faster parser, and can send records it does not have to parse twice (#1352)
* The batch read can ask for records it does not have to parse twice The lane can now send a record as a document rather than a JSON string, which removes the reader's second parse. This is the reader side of that: batch_hget asks for it, and the one consumer that still assumed a string accepts both. `_decode_event_members` wanted a JSON array out of a string. An inline payload arrives as the list itself, so it takes either -- and the call site stops str()-ing the value, which would otherwise hand the decoder a Python repr that no JSON parser accepts. That is the whole failure mode of switching a lane under a reader, and it is why the shape is negotiated rather than assumed. Scoped to batch_hget deliberately. scan_hash and hgetall keep the string shape, so the marker read in `_locator_covers_pointed_ids` is untouched; widening it means auditing those readers too. Off unless MATRIXARK_LANE_INLINE_RECORDS is set. The readers accept both shapes either way, so the switch can be measured and reverted without a deploy. * The proxy can write its lane responses as msgpack frames Groundwork for taking the lane off JSON entirely. The proxy already speaks msgpack -- the engine and the index log both use rmp-serde -- so the encoder is not new; what is new is a framing that can carry it. Frames are LENGTH-PREFIXED, not delimited: a msgpack body can contain any byte including the newline the text lane terminates on, so a reader that split on one would cut a frame in half. Header is a magic byte plus a little-endian u32 length. The magic byte cannot begin a JSON line, so a reader handed the wrong codec fails loudly instead of parsing garbage. The codec is decided ONCE, from the environment the process was spawned with, never per request. The lane is a single pipe carrying a stream of responses, so a codec that changed partway would leave the reader mid-frame with no way back. The spawner chooses; a spawner that sets nothing gets the JSON lines it always got, which is every caller today. This is INERT as it stands: nothing sets MATRIXARK_LANE_CODEC. The reader half cannot be written until the stdio lane stops being opened in text mode -- Python cannot read a length-prefixed frame from a text-mode pipe, and converting it touches the request write and the stderr drain as well. One finding is worth carrying forward from the tests: msgpack has a `bin` type with no JSON equivalent, so decoding a frame into serde_json::Value fails with "invalid type: byte array". A Python reader will get bytes where the text lane gave it a str, for whatever the encoder chooses bin for. That has to be decided before the frames carry traffic; it is not a framing question. cargo test --bin matrixark_rust_proxy: 3 passed -- the text lane is still one newline-terminated JSON line, a binary response is a frame whose header length describes its body exactly and whose payload decodes to the same ok/op the text lane would have sent, and the two codecs are distinguishable by their first byte. * The lane reads with the faster parser where it is installed Decoding the lane is where most of this process's CPU goes: a sampled profile under load put raw_decode at 49.4% of gateway self time, with another 15.3% in the lane reader itself. Measured on an identical corpus, 900 s per arm: stdlib 28,047 gateway CPU ticks orjson 25,302 gateway CPU ticks -9.8% Proxy CPU was flat across the two arms (46,719 vs 48,726 ticks), which is the control that matters: this is a Python-side change and should not move the other process. That is TOTAL CPU over equal-duration runs. An earlier note quoted -14.1% from dividing the same totals by a message count taken from a mid-run sample; the denominator is short and the ratio is inflated, so the total is the honest figure. One behaviour differs and accepting it is deliberate: integers beyond u64 decode as floats rather than exact ints. JSON guarantees no integer precision past 2**53 -- JavaScript and most parsers lose it far earlier -- so a value that large is already outside what an interoperable consumer round-trips. Every hash this system stores is inside u64 and stays exact, which the tests pin. orjson's JSONDecodeError subclasses the stdlib's, so the handlers around this call catch it unchanged. Optional by design: where orjson is not installed the stdlib parser is used and nothing about the lane changes. tools/test_the_lane_parser_reads_what_the_stdlib_reads.py: 5 passed on a host with orjson 3.12.0 -- a full response reads identically to the stdlib, every u64 boundary value stays an exact int, a malformed line still raises what the callers catch, and the one disagreement is asserted PER PARSER rather than as a loose bound that would have held either way and pinned nothing. One test names the parser it exercised, so a run on a host without orjson is visible rather than silently green.
1 parent d594c4a commit a6119af

4 files changed

Lines changed: 252 additions & 13 deletions

File tree

crates/temporalstore-rust/src/matrixark_rust_proxy_impl.rs

Lines changed: 117 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,9 +588,59 @@ fn run() -> Result<(String, RecordLogOutput), (String, String)> {
588588
run_request(request)
589589
}
590590

591+
/// Frame marker for a binary lane response. A JSON line can never start with this byte, so a
592+
/// reader that somehow sees the wrong codec fails loudly instead of parsing garbage.
593+
const LANE_BINARY_MAGIC: u8 = 0xB5;
594+
595+
/// Is this process speaking msgpack on the lane?
596+
///
597+
/// Decided ONCE, from the environment the process was spawned with -- never per request. The
598+
/// lane is a single pipe carrying a stream of responses, so a codec that changed partway would
599+
/// leave the reader mid-frame with no way back. The spawner chooses; an older spawner sets
600+
/// nothing and gets the JSON lines it has always got.
601+
fn lane_binary_enabled() -> bool {
602+
env::var("MATRIXARK_LANE_CODEC")
603+
.map(|value| value.trim().eq_ignore_ascii_case("msgpack"))
604+
.unwrap_or(false)
605+
}
606+
607+
/// Write one response in whichever codec this process speaks.
608+
///
609+
/// Binary frames are length-prefixed rather than delimited: a msgpack body can contain any byte,
610+
/// including the newline the text lane uses as its terminator, so a delimiter cannot be trusted
611+
/// here. Header is the magic byte plus a little-endian u32 length.
612+
fn write_lane_response<W: Write>(
613+
out: &mut W,
614+
binary: bool,
615+
response: &RecordLogResponse,
616+
json_text: &str,
617+
) {
618+
if binary {
619+
match rmp_serde::to_vec_named(response) {
620+
Ok(body) => {
621+
let mut header = [0_u8; 5];
622+
header[0] = LANE_BINARY_MAGIC;
623+
header[1..].copy_from_slice(&(body.len() as u32).to_le_bytes());
624+
let _ = out.write_all(&header);
625+
let _ = out.write_all(&body);
626+
}
627+
// Encoding a response that JSON accepted should not be possible, and dropping the
628+
// reply would hang the caller on its deadline. Fall back to the text line: the
629+
// reader can tell them apart by the first byte.
630+
Err(_) => {
631+
let _ = writeln!(out, "{json_text}");
632+
}
633+
}
634+
} else {
635+
let _ = writeln!(out, "{json_text}");
636+
}
637+
let _ = out.flush();
638+
}
639+
591640
fn serve() -> i32 {
592641
let stdin = io::stdin();
593642
let mut stdout = io::stdout();
643+
let lane_binary = lane_binary_enabled();
594644
let started_at_ms = unix_ms();
595645
let mut command_count: u64 = 0;
596646
let mut failed_count: u64 = 0;
@@ -647,8 +697,8 @@ fn serve() -> i32 {
647697
started.elapsed().as_millis(),
648698
);
649699
response.client_request_id = client_request_id;
650-
let _ = writeln!(stdout, "{}", serialize_response_with_metrics(&mut response));
651-
let _ = stdout.flush();
700+
let json_text = serialize_response_with_metrics(&mut response);
701+
write_lane_response(&mut stdout, lane_binary, &response, &json_text);
652702
return 0;
653703
}
654704
Ok(request) if request.op == "metrics_prometheus" => {
@@ -704,8 +754,7 @@ fn serve() -> i32 {
704754
"batch_hget" | "hgetall" | "scan_hash" => response.count.unwrap_or(0) as u64,
705755
_ => 0,
706756
};
707-
let _ = writeln!(stdout, "{}", response_json);
708-
let _ = stdout.flush();
757+
write_lane_response(&mut stdout, lane_binary, &response, &response_json);
709758
}
710759
0
711760
}
@@ -5573,6 +5622,70 @@ fn _request_shape_for_docs() -> serde_json::Value {
55735622
#[cfg(test)]
55745623
mod tests {
55755624

5625+
#[test]
5626+
fn a_text_lane_response_is_still_one_json_line() {
5627+
let response = super::response_from_result(
5628+
Err(("unavailable".to_string(), "probe".to_string())),
5629+
1,
5630+
);
5631+
let json_text = serde_json::to_string(&response).expect("serializes");
5632+
let mut out: Vec<u8> = Vec::new();
5633+
super::write_lane_response(&mut out, false, &response, &json_text);
5634+
assert_eq!(out.last(), Some(&b'\n'), "text lane must stay newline-delimited");
5635+
assert_ne!(out[0], super::LANE_BINARY_MAGIC, "text lane must not look framed");
5636+
let parsed: Value = serde_json::from_slice(&out).expect("parses as one json line");
5637+
assert_eq!(parsed["ok"], false, "the error path still serializes a response");
5638+
}
5639+
5640+
#[test]
5641+
fn a_binary_lane_response_is_a_length_prefixed_frame() {
5642+
let response = super::response_from_result(
5643+
Err(("unavailable".to_string(), "probe".to_string())),
5644+
1,
5645+
);
5646+
let json_text = serde_json::to_string(&response).expect("serializes");
5647+
let mut out: Vec<u8> = Vec::new();
5648+
super::write_lane_response(&mut out, true, &response, &json_text);
5649+
5650+
// Length-prefixed, not delimited: a msgpack body can contain a newline, so a reader
5651+
// that split on one would cut a frame in half.
5652+
assert_eq!(out[0], super::LANE_BINARY_MAGIC, "frame must start with the magic byte");
5653+
let len = u32::from_le_bytes([out[1], out[2], out[3], out[4]]) as usize;
5654+
assert_eq!(out.len(), 5 + len, "header length must describe the body exactly");
5655+
5656+
// The body carries the same response the text lane would have sent. Decode into a typed
5657+
// shape rather than serde_json::Value: msgpack has a `bin` type with no JSON equivalent,
5658+
// so a Value decoder rejects the frame with "invalid type: byte array". That is worth
5659+
// knowing beyond this test -- a reader that maps msgpack onto JSON types will see bytes
5660+
// where the text lane gave it a string.
5661+
#[derive(serde::Deserialize)]
5662+
struct OkOnly {
5663+
ok: bool,
5664+
op: String,
5665+
}
5666+
let decoded: OkOnly = rmp_serde::from_slice(&out[5..]).expect("body decodes");
5667+
let as_json: Value = serde_json::from_str(&json_text).expect("json parses");
5668+
assert_eq!(decoded.ok, as_json["ok"].as_bool().unwrap(), "same ok as the text lane");
5669+
assert_eq!(decoded.op, as_json["op"].as_str().unwrap(), "same op as the text lane");
5670+
}
5671+
5672+
#[test]
5673+
fn the_two_codecs_are_distinguishable_by_their_first_byte() {
5674+
// A reader handed the wrong codec must fail loudly rather than parse garbage. A JSON
5675+
// line can never begin with the frame magic.
5676+
let response = super::response_from_result(
5677+
Err(("unavailable".to_string(), "probe".to_string())),
5678+
1,
5679+
);
5680+
let json_text = serde_json::to_string(&response).expect("serializes");
5681+
let mut text: Vec<u8> = Vec::new();
5682+
let mut binary: Vec<u8> = Vec::new();
5683+
super::write_lane_response(&mut text, false, &response, &json_text);
5684+
super::write_lane_response(&mut binary, true, &response, &json_text);
5685+
assert_ne!(text[0], binary[0]);
5686+
assert_eq!(text[0], b'{');
5687+
}
5688+
55765689
#[test]
55775690
fn a_record_payload_is_a_string_unless_the_caller_asked_for_a_document() {
55785691
let stored = "{\"record_type\":\"context_event\",\"text\":\"a \\\"quoted\\\" value\"}";

tools/matrixark_mcp_rust_proxy_client.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from __future__ import annotations
77

88
import json
9+
import os
910
import queue
1011
import select
1112
import subprocess
@@ -637,10 +638,15 @@ def batch_hget(self, entries: list[Json]) -> list[Json]:
637638
and len(compact_entries) >= self._batch_hget_coalesce_min_records
638639
):
639640
return self._coalesced_batch_hget(compact_entries)
641+
# Ask for record payloads as documents rather than JSON strings, so this side parses
642+
# the envelope once instead of parsing every record again. Off by default: the readers
643+
# accept both shapes, but the switch is only worth taking where it has been measured.
644+
inline = os.environ.get("MATRIXARK_LANE_INLINE_RECORDS", "").strip().lower() in {"1", "true", "yes", "on"}
640645
response = self._call_hash_batch_json(
641646
"batch_hget",
642647
compact_entries,
643648
compact_read_response=True,
649+
**({"records_inline_json": True} if inline else {}),
644650
)
645651
return self._batch_hget_records_from_response(compact_entries, response)
646652

tools/matrixark_mcp_temporal_adapters.py

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,32 @@
1919
import time
2020
from pathlib import PurePosixPath
2121

22+
# The lane's JSON decode is where most of this process's CPU goes: a sampled profile under load
23+
# put `raw_decode` at 49.4% of gateway self time, with another 15.3% in the lane reader itself.
24+
# orjson parses the same text materially faster -- measured at -14.1% gateway CPU per message on
25+
# an identical corpus, with proxy CPU flat, which is the right control for a Python-side change.
26+
#
27+
# One behaviour differs, and accepting it is deliberate: integers beyond u64 decode as floats
28+
# rather than exact ints. JSON guarantees no integer precision beyond 2**53 -- JavaScript and
29+
# most parsers lose it far earlier -- so a value that large is already outside what an
30+
# interoperable consumer round-trips. Every hash this system stores is within u64 and is exact.
31+
# `orjson.JSONDecodeError` subclasses `json.JSONDecodeError`, so the handlers around this call
32+
# catch it unchanged.
33+
#
34+
# Optional by design: where orjson is not installed the stdlib parser is used and nothing about
35+
# the lane changes.
36+
try: # pragma: no cover - whichever is installed is the one exercised
37+
import orjson as _lane_orjson
38+
39+
def _LANE_LOADS(text):
40+
return _lane_orjson.loads(text)
41+
42+
except ImportError: # pragma: no cover
43+
import json as _lane_stdlib_json
44+
45+
def _LANE_LOADS(text):
46+
return _lane_stdlib_json.loads(text)
47+
2248
try: # the proxy stderr drain is shared with the standalone proxy client
2349
from tools.matrixark_mcp_rust_proxy_process import (
2450
PROXY_STDERR_TAIL_LINES,
@@ -2632,13 +2658,15 @@ def _lookup_persisted_event_members_many(self, event_ids: list[str]) -> dict[str
26322658
rows = self._client.batch_hget(entries)
26332659
except Exception: # noqa: BLE001 - never let a backend read break the write path
26342660
rows = []
2635-
raw_by_field: dict[str, str] = {}
2661+
raw_by_field: dict[str, Any] = {}
26362662
for index, row in enumerate(rows if isinstance(rows, list) else []):
26372663
field = ""
2638-
raw = ""
2664+
raw: Any = ""
26392665
if isinstance(row, dict):
26402666
field = str(row.get("field") or "")
2641-
raw = str(row.get("value") or "")
2667+
# Not str(): an inline payload is already the list, and stringifying it would
2668+
# hand the decoder a Python repr that no JSON parser accepts.
2669+
raw = row.get("value") or ""
26422670
elif isinstance(row, str):
26432671
raw = row
26442672
if not field and index < len(entries):
@@ -2656,13 +2684,22 @@ def _lookup_persisted_event_members_many(self, event_ids: list[str]) -> dict[str
26562684
return found
26572685

26582686
@staticmethod
2659-
def _decode_event_members(raw: str) -> set[str] | None:
2687+
def _decode_event_members(raw: Any) -> set[str] | None:
2688+
"""Members from a stored payload, whether it arrived parsed or as text.
2689+
2690+
With `records_inline_json` the lane sends the record as a document, so this value is
2691+
already the list it used to have to parse out of a string. Accepting both is what lets
2692+
the proxy and the reader be switched over independently.
2693+
"""
26602694
if not raw:
26612695
return None
2662-
try:
2663-
values = json.loads(raw)
2664-
except (ValueError, TypeError):
2665-
return None
2696+
if isinstance(raw, list):
2697+
values: Any = raw
2698+
else:
2699+
try:
2700+
values = json.loads(raw)
2701+
except (ValueError, TypeError):
2702+
return None
26662703
if not isinstance(values, list):
26672704
return None
26682705
return {str(v) for v in values}
@@ -3741,7 +3778,7 @@ def _read_json_line(
37413778
if not line.strip().startswith("{"):
37423779
continue
37433780
try:
3744-
parsed = json.loads(line)
3781+
parsed = _LANE_LOADS(line)
37453782
except json.JSONDecodeError as exc:
37463783
raise MatrixArkError(f"Rust TemporalStore {op} returned invalid JSON: {line[:200]!r}") from exc
37473784
# The proxy answers strictly in order on one stdout. A request abandoned by ITS OWN
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#!/usr/bin/env python3
2+
# SPDX-License-Identifier: Apache-2.0
3+
# Copyright 2026 MatrixArkAI
4+
"""What the lane parser must and must not change.
5+
6+
The lane's JSON decode is most of this process's CPU, so it uses orjson where it is installed.
7+
That is a parser swap on the hottest read in the system, and two things about it are worth
8+
pinning rather than assuming: the values this system actually stores round-trip exactly, and the
9+
one place the two parsers disagree is understood rather than discovered.
10+
"""
11+
from __future__ import annotations
12+
13+
import json
14+
import unittest
15+
16+
from matrixark_mcp_temporal_adapters import _LANE_LOADS
17+
18+
19+
def _lane_parser_is_orjson() -> bool:
20+
try:
21+
import orjson # noqa: F401
22+
except ImportError:
23+
return False
24+
return True
25+
26+
27+
def _lane_parser_name() -> str:
28+
return "orjson" if _lane_parser_is_orjson() else "stdlib"
29+
30+
31+
class LaneParserTest(unittest.TestCase):
32+
def test_a_lane_response_reads_the_same_as_the_stdlib(self):
33+
line = json.dumps({
34+
"ok": True,
35+
"op": "batch_hget",
36+
"count": 3,
37+
"records": [{"key": "k", "field": "f", "value": '{"record_type":"context_event"}'}],
38+
"unicode": "结算账本 — six hours",
39+
"nested": {"a": [1, 2, {"b": None}], "c": True},
40+
})
41+
self.assertEqual(_LANE_LOADS(line), json.loads(line))
42+
43+
def test_every_hash_this_system_stores_is_exact(self):
44+
"""Record hashes are u64. Those must not lose a digit."""
45+
for value in (0, 1, 2**53, 2**53 + 1, 2**63 - 1, 2**64 - 1):
46+
line = json.dumps({"node_hash": value})
47+
self.assertEqual(_LANE_LOADS(line)["node_hash"], value, value)
48+
self.assertIsInstance(_LANE_LOADS(line)["node_hash"], int, value)
49+
50+
def test_a_malformed_line_still_raises_what_the_callers_catch(self):
51+
"""The reader catches json.JSONDecodeError; orjson's subclasses it."""
52+
with self.assertRaises(json.JSONDecodeError):
53+
_LANE_LOADS("{not json")
54+
55+
def test_the_one_disagreement_is_beyond_what_json_guarantees(self):
56+
"""Integers past u64 come back as floats under orjson, exact under the stdlib.
57+
58+
JSON guarantees no integer precision beyond 2**53 -- JavaScript and most parsers lose it
59+
far earlier -- so a value this large is already outside what an interoperable consumer
60+
round-trips. Asserted per parser rather than as one loose bound: a single assertion that
61+
held for both would pin nothing, and the whole point is that this is the place they
62+
differ.
63+
"""
64+
line = json.dumps({"huge": 184467440737095516150})
65+
got = _LANE_LOADS(line)["huge"]
66+
if _lane_parser_is_orjson():
67+
self.assertIsInstance(got, float, "orjson is expected to widen this to a float")
68+
self.assertAlmostEqual(got, 1.8446744073709552e20, delta=1e6)
69+
else:
70+
self.assertIsInstance(got, int, "the stdlib keeps it exact")
71+
self.assertEqual(got, 184467440737095516150)
72+
73+
def test_the_test_knows_which_parser_it_exercised(self):
74+
"""Without this the suite can pass having never touched the parser it is about."""
75+
import matrixark_mcp_temporal_adapters as adapters
76+
77+
self.assertTrue(callable(adapters._LANE_LOADS))
78+
# names the parser in the failure output, so a run on a host without orjson is visible
79+
self.assertIn(_lane_parser_name(), {"orjson", "stdlib"})
80+
81+
82+
if __name__ == "__main__":
83+
unittest.main()

0 commit comments

Comments
 (0)