marketbyorder-bot: snapshot orders associated by snapshot_id, which is per-instrument not per-channel

[armcconnell]

marketbyorder-bot associates SnapshotOrder records with an instrument by snapshot_id alone, but Snapshot ID is scoped per instrument, not per channel.

Spec (market-by-order, 0x20): "Monotonically increasing per (channel_id, instrument_id) within the current Reset Count era." The association rule is "discard any SnapshotOrder whose Snapshot ID does not match the currently-open SnapshotBegin" — snapshot_id validates the match, it isn't the key.

Two places assume channel-wide uniqueness:

• shard.go:164-176 — applySnapshotOrder scans s.instruments and inserts into the first instrument whose OpenSnapshot.SnapshotID matches. Go map iteration is randomized, so with two matching instruments the target is arbitrary.
• shard.go:24-27,420 — snapCtx is keyed {channel_id, snapshot_id}, so two instruments sharing an id collide and the later snapshot_begin overwrites the earlier context. wire_snapshots rows can then be written with the wrong instrument_id/symbol.

Publishers must not interleave snapshot groups, which normally leaves one group open at a time. The gap is when a SnapshotEnd is lost: that instrument's OpenSnapshot is never cleared and lingers. Because each instrument's counter advances once per cycle, instruments in the same cycle typically sit at the same snapshot_id — so a stale open snapshot is likely to collide with the next instrument's rather than unlikely. #20 measured ~3.7% of snapshots arriving short on the Tokyo feed, so lingering open snapshots are not hypothetical.

Effects are bounded but real: misattributed orders land in a shadow that fails its count check and is discarded (snapshot_discarded_total), so live books are not corrupted — but recovery for the affected instrument is delayed a cycle, and mis-keyed wire_snapshots rows are persisted.

Fix: key the association on the currently-open snapshot for a known (channel_id, instrument_id) and use snapshot_id only to validate. snapCtx needs the instrument in its key.

Found while writing marketbyprice-parser (#29) — the market-by-price spec scopes Snapshot ID identically. Not verified against a live feed.

[armcconnell]

Third call site, and the most consequential of the three: coordinator.go:38,74-88 keys snapshotRoute by snapKey{channel_id, snapshot_id} to decide which shard a snapshot_order goes to.

Two instruments sharing a snapshot_id generally hash to different shards (instrument_id % n), so the later snapshot_begin overwrites the route and subsequent snapshot_order records are delivered to a shard that does not own the instrument. There, applySnapshotOrder finds no instrument with a matching open snapshot and drops them silently — snapshot_order_dropped_total only counts records with no route at all, not records routed to the wrong shard.

That makes the failure quieter than the shard.go paths: the orders vanish without a counter, and the affected snapshot fails its count check a cycle later.

Same fix — route by the currently-open group's (channel_id, instrument_id), established by snapshot_begin, with snapshot_id used only to validate.

[armcconnell]

Correction to my comment above — I overstated the coordinator case.

coordinator.go deletes snapshotRoute on snapshot_end and counts a missing route via snapshot_order_dropped_total, so entries are not stale-forever and an orphan order is not silently swallowed. I checked this by building the equivalent wrong route in a prototype and testing it: for sequential, complete snapshot groups the two models agree, because the later snapshot_begin overwrites the entry before its own orders arrive.

Mis-routing at the coordinator needs two instruments at the same snapshot_id with an order arriving while the map points at the other one — that is, publisher interleaving (forbidden) or reordering between two equal-id groups. Narrower than "a different shard in general".

The two shard.go findings in the issue body stand as written and do not depend on reordering:

• applySnapshotOrder scanning s.instruments for the first matching OpenSnapshot.SnapshotID picks arbitrarily under Go's randomized map order when two instruments in the same shard have open snapshots at the same id. A lost SnapshotEnd leaves such a snapshot open, and per-instrument counters advance once per cycle, so equal ids are the common case rather than the rare one.
• snapCtx keyed {channel_id, snapshot_id} collides under the same conditions, so wire_snapshots rows can carry the wrong instrument_id/symbol.

The fix is unchanged: key on the currently-open group's (channel_id, instrument_id), with snapshot_id used only to validate.