Merge pull request 'perf(remote): end-to-end Output flow control (daemon byte budget + broker stream credit)' (#21) from perf/output-flow-control into main
Reviewed-on: https://git.bnema.dev/bnema/vev/pulls/21
26 files · +1089 -429
M docs/architecture.md +2 -2
| @@ -1,6 +1,6 @@ | |||
| 1 | 1 | # Architecture | |
| 2 | 2 | ||
| 3 | - | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 65. | |
| 3 | + | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 66. | |
| 4 | 4 | ||
| 5 | 5 | `CommandResult` has a closed explicit outcome contract: | |
| 6 | 6 | ||
| @@ -135,7 +135,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||
| 135 | 135 | `go tool buf generate`, and handle the new variant in `sessionwire`. | |
| 136 | 136 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | |
| 137 | 137 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | |
| 138 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `65`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 138 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `66`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 139 | 139 | ||
| 140 | 140 | ## Broker | |
| 141 | 141 | ||
| @@ -1,6 +1,6 @@ | |||||
| 1 | # Architecture | 1 | # Architecture | ||
| 2 | 2 | ||||
| 3 | - | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 65. | 3 | + | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 66. |
| 4 | 4 | ||||
| 5 | `CommandResult` has a closed explicit outcome contract: | 5 | `CommandResult` has a closed explicit outcome contract: | ||
| 6 | 6 | ||||
| @@ -135,7 +135,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||||
| 135 | `go tool buf generate`, and handle the new variant in `sessionwire`. | 135 | `go tool buf generate`, and handle the new variant in `sessionwire`. | ||
| 136 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | 136 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | ||
| 137 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | 137 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | ||
| 138 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `65`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | 138 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `66`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. |
| 139 | 139 | ||||
| 140 | ## Broker | 140 | ## Broker | ||
| 141 | 141 | ||||
M docs/protocol.md +19 -3
| @@ -109,8 +109,8 @@ Timeout or reply loss after dispatch is outcome unknown and is never replayed. | |||
| 109 | 109 | ## Broker negotiation | |
| 110 | 110 | ||
| 111 | 111 | Broker connections are a separate conversation from the session protocol. | |
| 112 | - | `internal/adapters/brokerwire` owns it: client tags 101-111 and server tags | |
| 113 | - | 201-209 are disjoint from every session tag, and each direction is its own | |
| 112 | + | `internal/adapters/brokerwire` owns it: client tags 101-114 and server tags | |
| 113 | + | 201-211 are disjoint from every session tag, and each direction is its own | |
| 114 | 114 | closed directional `oneof` union. A broker connection opens with exactly one | |
| 115 | 115 | broker preamble (`PreambleRequest` client → server, `PreambleResponse` server | |
| 116 | 116 | → client) using broker roles 3/4. The preamble carries the same magic | |
| @@ -127,7 +127,15 @@ connection dispatcher. | |||
| 127 | 127 | ||
| 128 | 128 | `internal/adapters/brokeripc` carries this conversation over `broker.sock` | |
| 129 | 129 | in the per-user runtime directory, with same-user peer-credential admission | |
| 130 | - | and bounded per-connection queues. The remote helpers `_broker-mux-stdio`, | |
| 130 | + | and bounded per-connection queues. Each logical stream is credit flow | |
| 131 | + | controlled in both directions: a sender spends `StreamChunkCredit` (payload | |
| 132 | + | plus 64 bytes) per data frame out of a 4 MiB window | |
| 133 | + | (`brokerwire.StreamWindowBytes`) and waits when it has too little, and the | |
| 134 | + | receiver returns the credit of frames its consumer has read in | |
| 135 | + | `StreamWindowUpdate` batches. A slow consumer therefore slows its own stream | |
| 136 | + | instead of losing it. Data or cumulative credit beyond the window is a | |
| 137 | + | flow-control violation that settles only that stream; a single grant of zero | |
| 138 | + | or above the window is a malformed frame and fails the connection. The remote helpers `_broker-mux-stdio`, | |
| 131 | 139 | `_broker-mux-quic-bootstrap`, and `_broker-mux-quic-proxy` bridge a remote | |
| 132 | 140 | daemonmux socket to stdio or to one freshly authenticated QUIC stream. | |
| 133 | 141 | ||
| @@ -162,6 +170,12 @@ base. `ValidateOutput` enforces the epoch/base/new chain | |||
| 162 | 170 | claims its carriage's preferred window, or `MaxOutputWindow` when the | |
| 163 | 171 | carriage states none, so output pipelines instead of waiting a round trip | |
| 164 | 172 | per frame. | |
| 173 | + | - Unacknowledged Output data is also bounded by `MaxOutputWindowBytes` | |
| 174 | + | (2 MiB). State frames are charged to their own state, side effects to the | |
| 175 | + | next state, and a cumulative ACK releases what it covers. The next state | |
| 176 | + | frame waits when either window is full, except that a frame may always be | |
| 177 | + | sent while no state frame is in flight. A frame carrying a large image may | |
| 178 | + | exceed the byte window; it then holds the next frame until its ACK. | |
| 165 | 179 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | |
| 166 | 180 | applying + acknowledging daemon frames even while a picker lease owns | |
| 167 | 181 | the terminal (see below) — admission without physical write. | |
| @@ -247,6 +261,8 @@ than `KillResult` and `CommandResult`. | |||
| 247 | 261 | | Queued envelopes per connection | 8 messages, 32 MiB bytes (`streamframe` defaults), then backpressure; sync sends uncharged, async charged once | `adapters/streamframe` | | |
| 248 | 262 | | Encoding buffer | bounded per-connection framing buffers owned by `streamframe` | `adapters/streamframe` | | |
| 249 | 263 | | Output states in flight | 8 (`MaxOutputWindow`) preferred for QUIC/IPC/SSH | daemon, negotiated down | | |
| 264 | + | | Unacknowledged Output bytes | 2 MiB (`MaxOutputWindowBytes`); one frame always allowed when none is in flight | daemon | | |
| 265 | + | | Broker stream window | 4 MiB per stream and direction (`brokerwire.StreamWindowBytes`), returned by `StreamWindowUpdate` | `adapters/brokerwire`, enforced in `adapters/brokeripc` | | |
| 250 | 266 | | QUIC unauthenticated peers | 4 connections, 3 s auth deadline, one bidirectional stream each; extra/unidirectional streams rejected | `adapters/quic` | | |
| 251 | 267 | | QUIC bootstrap | 32-byte token + 16-byte nonce (base64), ≤4 KiB readiness (schema v1, host-independent port, SHA-256 pin, ≤15 s expiry), one bounded (512 B) auth record, atomic one-time consumption, token erasure on close | `adapters/quic` bootstrap, `app/_broker-mux-quic-bootstrap` + `_broker-mux-quic-proxy` | | |
| 252 | 268 | | Handshake, dial → first committed publication | 15 s, single absolute deadline propagated unchanged, never reset | both ends | | |
| @@ -109,8 +109,8 @@ Timeout or reply loss after dispatch is outcome unknown and is never replayed. | |||||
| 109 | ## Broker negotiation | 109 | ## Broker negotiation | ||
| 110 | 110 | ||||
| 111 | Broker connections are a separate conversation from the session protocol. | 111 | Broker connections are a separate conversation from the session protocol. | ||
| 112 | - | `internal/adapters/brokerwire` owns it: client tags 101-111 and server tags | 112 | + | `internal/adapters/brokerwire` owns it: client tags 101-114 and server tags |
| 113 | - | 201-209 are disjoint from every session tag, and each direction is its own | 113 | + | 201-211 are disjoint from every session tag, and each direction is its own |
| 114 | closed directional `oneof` union. A broker connection opens with exactly one | 114 | closed directional `oneof` union. A broker connection opens with exactly one | ||
| 115 | broker preamble (`PreambleRequest` client → server, `PreambleResponse` server | 115 | broker preamble (`PreambleRequest` client → server, `PreambleResponse` server | ||
| 116 | → client) using broker roles 3/4. The preamble carries the same magic | 116 | → client) using broker roles 3/4. The preamble carries the same magic | ||
| @@ -127,7 +127,15 @@ connection dispatcher. | |||||
| 127 | 127 | ||||
| 128 | `internal/adapters/brokeripc` carries this conversation over `broker.sock` | 128 | `internal/adapters/brokeripc` carries this conversation over `broker.sock` | ||
| 129 | in the per-user runtime directory, with same-user peer-credential admission | 129 | in the per-user runtime directory, with same-user peer-credential admission | ||
| 130 | - | and bounded per-connection queues. The remote helpers `_broker-mux-stdio`, | 130 | + | and bounded per-connection queues. Each logical stream is credit flow |
| 131 | + | controlled in both directions: a sender spends `StreamChunkCredit` (payload | |||
| 132 | + | plus 64 bytes) per data frame out of a 4 MiB window | |||
| 133 | + | (`brokerwire.StreamWindowBytes`) and waits when it has too little, and the | |||
| 134 | + | receiver returns the credit of frames its consumer has read in | |||
| 135 | + | `StreamWindowUpdate` batches. A slow consumer therefore slows its own stream | |||
| 136 | + | instead of losing it. Data or cumulative credit beyond the window is a | |||
| 137 | + | flow-control violation that settles only that stream; a single grant of zero | |||
| 138 | + | or above the window is a malformed frame and fails the connection. The remote helpers `_broker-mux-stdio`, | |||
| 131 | `_broker-mux-quic-bootstrap`, and `_broker-mux-quic-proxy` bridge a remote | 139 | `_broker-mux-quic-bootstrap`, and `_broker-mux-quic-proxy` bridge a remote | ||
| 132 | daemonmux socket to stdio or to one freshly authenticated QUIC stream. | 140 | daemonmux socket to stdio or to one freshly authenticated QUIC stream. | ||
| 133 | 141 | ||||
| @@ -162,6 +170,12 @@ base. `ValidateOutput` enforces the epoch/base/new chain | |||||
| 162 | claims its carriage's preferred window, or `MaxOutputWindow` when the | 170 | claims its carriage's preferred window, or `MaxOutputWindow` when the | ||
| 163 | carriage states none, so output pipelines instead of waiting a round trip | 171 | carriage states none, so output pipelines instead of waiting a round trip | ||
| 164 | per frame. | 172 | per frame. | ||
| 173 | + | - Unacknowledged Output data is also bounded by `MaxOutputWindowBytes` | |||
| 174 | + | (2 MiB). State frames are charged to their own state, side effects to the | |||
| 175 | + | next state, and a cumulative ACK releases what it covers. The next state | |||
| 176 | + | frame waits when either window is full, except that a frame may always be | |||
| 177 | + | sent while no state frame is in flight. A frame carrying a large image may | |||
| 178 | + | exceed the byte window; it then holds the next frame until its ACK. | |||
| 165 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | 179 | - The client coalesces ACKs (`client.go:cumulativeAckQueue`) and keeps | ||
| 166 | applying + acknowledging daemon frames even while a picker lease owns | 180 | applying + acknowledging daemon frames even while a picker lease owns | ||
| 167 | the terminal (see below) — admission without physical write. | 181 | the terminal (see below) — admission without physical write. | ||
| @@ -247,6 +261,8 @@ than `KillResult` and `CommandResult`. | |||||
| 247 | | Queued envelopes per connection | 8 messages, 32 MiB bytes (`streamframe` defaults), then backpressure; sync sends uncharged, async charged once | `adapters/streamframe` | | 261 | | Queued envelopes per connection | 8 messages, 32 MiB bytes (`streamframe` defaults), then backpressure; sync sends uncharged, async charged once | `adapters/streamframe` | | ||
| 248 | | Encoding buffer | bounded per-connection framing buffers owned by `streamframe` | `adapters/streamframe` | | 262 | | Encoding buffer | bounded per-connection framing buffers owned by `streamframe` | `adapters/streamframe` | | ||
| 249 | | Output states in flight | 8 (`MaxOutputWindow`) preferred for QUIC/IPC/SSH | daemon, negotiated down | | 263 | | Output states in flight | 8 (`MaxOutputWindow`) preferred for QUIC/IPC/SSH | daemon, negotiated down | | ||
| 264 | + | | Unacknowledged Output bytes | 2 MiB (`MaxOutputWindowBytes`); one frame always allowed when none is in flight | daemon | | |||
| 265 | + | | Broker stream window | 4 MiB per stream and direction (`brokerwire.StreamWindowBytes`), returned by `StreamWindowUpdate` | `adapters/brokerwire`, enforced in `adapters/brokeripc` | | |||
| 250 | | QUIC unauthenticated peers | 4 connections, 3 s auth deadline, one bidirectional stream each; extra/unidirectional streams rejected | `adapters/quic` | | 266 | | QUIC unauthenticated peers | 4 connections, 3 s auth deadline, one bidirectional stream each; extra/unidirectional streams rejected | `adapters/quic` | | ||
| 251 | | QUIC bootstrap | 32-byte token + 16-byte nonce (base64), ≤4 KiB readiness (schema v1, host-independent port, SHA-256 pin, ≤15 s expiry), one bounded (512 B) auth record, atomic one-time consumption, token erasure on close | `adapters/quic` bootstrap, `app/_broker-mux-quic-bootstrap` + `_broker-mux-quic-proxy` | | 267 | | QUIC bootstrap | 32-byte token + 16-byte nonce (base64), ≤4 KiB readiness (schema v1, host-independent port, SHA-256 pin, ≤15 s expiry), one bounded (512 B) auth record, atomic one-time consumption, token erasure on close | `adapters/quic` bootstrap, `app/_broker-mux-quic-bootstrap` + `_broker-mux-quic-proxy` | | ||
| 252 | | Handshake, dial → first committed publication | 15 s, single absolute deadline propagated unchanged, never reset | both ends | | 268 | | Handshake, dial → first committed publication | 15 s, single absolute deadline propagated unchanged, never reset | both ends | | ||
M internal/adapters/brokeripc/client.go +30 -14
| @@ -351,23 +351,24 @@ func (c *client) dispatch(message brokerwire.ServerMessage) error { | |||
| 351 | 351 | if !c.scopeMatches(m.Epoch, m.Connection) { | |
| 352 | 352 | return errors.Join(ErrScopeMismatch, ErrProtocol) | |
| 353 | 353 | } | |
| 354 | - | disposition, err := c.conn.StreamData(m.Stream) | |
| 355 | - | if err != nil { | |
| 356 | - | return errors.Join(ErrProtocol, err) | |
| 354 | + | st, err := c.streamFor(m.Stream) | |
| 355 | + | if st == nil || err != nil { | |
| 356 | + | return err | |
| 357 | 357 | } | |
| 358 | - | if disposition == brokerwire.StreamDiscarded { | |
| 359 | - | return nil | |
| 358 | + | if err := st.deliver(m.Data); errors.Is(err, ErrStreamCredit) { | |
| 359 | + | st.fail(ErrStreamCredit) | |
| 360 | 360 | } | |
| 361 | - | st := c.lookupStream(m.Stream) | |
| 362 | - | if st == nil { | |
| 363 | - | // The stream was retired locally between the tracker check and the | |
| 364 | - | // lookup; its data is discarded, exactly like a retired stream. | |
| 365 | - | return nil | |
| 361 | + | return nil | |
| 362 | + | case brokerwire.StreamWindowUpdate: | |
| 363 | + | if !c.scopeMatches(m.Epoch, m.Connection) { | |
| 364 | + | return errors.Join(ErrScopeMismatch, ErrProtocol) | |
| 366 | 365 | } | |
| 367 | - | if err := st.deliver(m.Data); err != nil { | |
| 368 | - | if errors.Is(err, ErrStreamBackpressure) { | |
| 369 | - | st.fail(ErrStreamBackpressure) | |
| 370 | - | } | |
| 366 | + | st, err := c.streamFor(m.Stream) | |
| 367 | + | if st == nil || err != nil { | |
| 368 | + | return err | |
| 369 | + | } | |
| 370 | + | if err := st.pipe.granted(m.Credit); err != nil { | |
| 371 | + | st.fail(err) | |
| 371 | 372 | } | |
| 372 | 373 | return nil | |
| 373 | 374 | case brokerwire.StreamClosed: | |
| @@ -926,6 +927,21 @@ func (c *client) lookupStream(stream ports.BrokerStreamID) *clientStream { | |||
| 926 | 927 | return c.streams[stream] | |
| 927 | 928 | } | |
| 928 | 929 | ||
| 930 | + | // streamFor classifies one stream-scoped frame (data or a credit grant) and | |
| 931 | + | // returns its live stream. A frame for a retired stream, or one retired locally | |
| 932 | + | // between the tracker check and the lookup, yields no stream and no error; a | |
| 933 | + | // frame for a stream that was never opened is a peer protocol violation. | |
| 934 | + | func (c *client) streamFor(id ports.BrokerStreamID) (*clientStream, error) { | |
| 935 | + | disposition, err := c.conn.StreamData(id) | |
| 936 | + | if err != nil { | |
| 937 | + | return nil, errors.Join(ErrProtocol, err) | |
| 938 | + | } | |
| 939 | + | if disposition == brokerwire.StreamDiscarded { | |
| 940 | + | return nil, nil | |
| 941 | + | } | |
| 942 | + | return c.lookupStream(id), nil | |
| 943 | + | } | |
| 944 | + | ||
| 929 | 945 | // unregisterStream drops one local stream record. | |
| 930 | 946 | func (c *client) unregisterStream(stream ports.BrokerStreamID) { | |
| 931 | 947 | c.mu.Lock() | |
| @@ -351,23 +351,24 @@ func (c *client) dispatch(message brokerwire.ServerMessage) error { | |||||
| 351 | if !c.scopeMatches(m.Epoch, m.Connection) { | 351 | if !c.scopeMatches(m.Epoch, m.Connection) { | ||
| 352 | return errors.Join(ErrScopeMismatch, ErrProtocol) | 352 | return errors.Join(ErrScopeMismatch, ErrProtocol) | ||
| 353 | } | 353 | } | ||
| 354 | - | disposition, err := c.conn.StreamData(m.Stream) | 354 | + | st, err := c.streamFor(m.Stream) |
| 355 | - | if err != nil { | 355 | + | if st == nil || err != nil { |
| 356 | - | return errors.Join(ErrProtocol, err) | 356 | + | return err |
| 357 | } | 357 | } | ||
| 358 | - | if disposition == brokerwire.StreamDiscarded { | 358 | + | if err := st.deliver(m.Data); errors.Is(err, ErrStreamCredit) { |
| 359 | - | return nil | 359 | + | st.fail(ErrStreamCredit) |
| 360 | } | 360 | } | ||
| 361 | - | st := c.lookupStream(m.Stream) | 361 | + | return nil |
| 362 | - | if st == nil { | 362 | + | case brokerwire.StreamWindowUpdate: |
| 363 | - | // The stream was retired locally between the tracker check and the | 363 | + | if !c.scopeMatches(m.Epoch, m.Connection) { |
| 364 | - | // lookup; its data is discarded, exactly like a retired stream. | 364 | + | return errors.Join(ErrScopeMismatch, ErrProtocol) |
| 365 | - | return nil | |||
| 366 | } | 365 | } | ||
| 367 | - | if err := st.deliver(m.Data); err != nil { | 366 | + | st, err := c.streamFor(m.Stream) |
| 368 | - | if errors.Is(err, ErrStreamBackpressure) { | 367 | + | if st == nil || err != nil { |
| 369 | - | st.fail(ErrStreamBackpressure) | 368 | + | return err |
| 370 | - | } | 369 | + | } |
| 370 | + | if err := st.pipe.granted(m.Credit); err != nil { | |||
| 371 | + | st.fail(err) | |||
| 371 | } | 372 | } | ||
| 372 | return nil | 373 | return nil | ||
| 373 | case brokerwire.StreamClosed: | 374 | case brokerwire.StreamClosed: | ||
| @@ -926,6 +927,21 @@ func (c *client) lookupStream(stream ports.BrokerStreamID) *clientStream { | |||||
| 926 | return c.streams[stream] | 927 | return c.streams[stream] | ||
| 927 | } | 928 | } | ||
| 928 | 929 | ||||
| 930 | + | // streamFor classifies one stream-scoped frame (data or a credit grant) and | |||
| 931 | + | // returns its live stream. A frame for a retired stream, or one retired locally | |||
| 932 | + | // between the tracker check and the lookup, yields no stream and no error; a | |||
| 933 | + | // frame for a stream that was never opened is a peer protocol violation. | |||
| 934 | + | func (c *client) streamFor(id ports.BrokerStreamID) (*clientStream, error) { | |||
| 935 | + | disposition, err := c.conn.StreamData(id) | |||
| 936 | + | if err != nil { | |||
| 937 | + | return nil, errors.Join(ErrProtocol, err) | |||
| 938 | + | } | |||
| 939 | + | if disposition == brokerwire.StreamDiscarded { | |||
| 940 | + | return nil, nil | |||
| 941 | + | } | |||
| 942 | + | return c.lookupStream(id), nil | |||
| 943 | + | } | |||
| 944 | + | ||||
| 929 | // unregisterStream drops one local stream record. | 945 | // unregisterStream drops one local stream record. | ||
| 930 | func (c *client) unregisterStream(stream ports.BrokerStreamID) { | 946 | func (c *client) unregisterStream(stream ports.BrokerStreamID) { | ||
| 931 | c.mu.Lock() | 947 | c.mu.Lock() | ||
M internal/adapters/brokeripc/config.go +1 -23
| @@ -15,13 +15,6 @@ const ( | |||
| 15 | 15 | // listener. A listener acquires one slot before accepting, so the bound is | |
| 16 | 16 | // enforced before a socket is admitted rather than after. | |
| 17 | 17 | DefaultMaxClients = 64 | |
| 18 | - | // DefaultStreamInboundChunks bounds one logical stream's inbound chunk | |
| 19 | - | // queue: frames already accepted from the wire and not yet consumed by the | |
| 20 | - | // local session carriage. | |
| 21 | - | DefaultStreamInboundChunks = 64 | |
| 22 | - | // DefaultStreamInboundBytes bounds one logical stream's inbound chunk queue | |
| 23 | - | // in bytes (4 MiB). | |
| 24 | - | DefaultStreamInboundBytes = 4 << 20 | |
| 25 | 18 | ) | |
| 26 | 19 | ||
| 27 | 20 | // Config bounds one endpoint. The zero value is valid and selects every | |
| @@ -29,10 +22,6 @@ const ( | |||
| 29 | 22 | type Config struct { | |
| 30 | 23 | // MaxClients bounds concurrent accepted client connections. | |
| 31 | 24 | MaxClients int | |
| 32 | - | // StreamInboundChunks bounds one logical stream's inbound chunk queue. | |
| 33 | - | StreamInboundChunks int | |
| 34 | - | // StreamInboundBytes bounds one logical stream's inbound chunk bytes. | |
| 35 | - | StreamInboundBytes uint64 | |
| 36 | 25 | // HandshakeTimeout bounds one accepted connection's broker preamble, broker | |
| 37 | 26 | // admission, and the wait for its Register. The client dialer also bounds | |
| 38 | 27 | // its whole setup (Unix dial, preamble, Register send, and wait for | |
| @@ -62,12 +51,6 @@ func (c Config) withDefaults() Config { | |||
| 62 | 51 | if c.MaxClients <= 0 { | |
| 63 | 52 | c.MaxClients = DefaultMaxClients | |
| 64 | 53 | } | |
| 65 | - | if c.StreamInboundChunks <= 0 { | |
| 66 | - | c.StreamInboundChunks = DefaultStreamInboundChunks | |
| 67 | - | } | |
| 68 | - | if c.StreamInboundBytes == 0 { | |
| 69 | - | c.StreamInboundBytes = DefaultStreamInboundBytes | |
| 70 | - | } | |
| 71 | 54 | if c.HandshakeTimeout <= 0 { | |
| 72 | 55 | c.HandshakeTimeout = protocol.HandshakeTimeout | |
| 73 | 56 | } | |
| @@ -80,16 +63,11 @@ func (c Config) withDefaults() Config { | |||
| 80 | 63 | // validate refuses a configuration whose explicit bounds cannot be honored. | |
| 81 | 64 | // The zero value is always valid because withDefaults has already been applied. | |
| 82 | 65 | func (c Config) validate() error { | |
| 83 | - | if c.MaxClients <= 0 || c.StreamInboundChunks <= 0 || c.StreamInboundBytes == 0 || c.HandshakeTimeout <= 0 { | |
| 66 | + | if c.MaxClients <= 0 || c.HandshakeTimeout <= 0 { | |
| 84 | 67 | return ErrConfig | |
| 85 | 68 | } | |
| 86 | 69 | if c.MaxPendingOperations <= 0 || c.MaxPendingOperations > brokerwire.MaxPendingOperations { | |
| 87 | 70 | return ErrConfig | |
| 88 | 71 | } | |
| 89 | - | // A stream inbound byte bound below one maximum chunk can never accept a | |
| 90 | - | // full stream frame, which would starve a legal peer. | |
| 91 | - | if c.StreamInboundBytes < brokerwire.MaxStreamChunkBytes { | |
| 92 | - | return ErrConfig | |
| 93 | - | } | |
| 94 | 72 | return nil | |
| 95 | 73 | } | |
| @@ -15,13 +15,6 @@ const ( | |||||
| 15 | // listener. A listener acquires one slot before accepting, so the bound is | 15 | // listener. A listener acquires one slot before accepting, so the bound is | ||
| 16 | // enforced before a socket is admitted rather than after. | 16 | // enforced before a socket is admitted rather than after. | ||
| 17 | DefaultMaxClients = 64 | 17 | DefaultMaxClients = 64 | ||
| 18 | - | // DefaultStreamInboundChunks bounds one logical stream's inbound chunk | |||
| 19 | - | // queue: frames already accepted from the wire and not yet consumed by the | |||
| 20 | - | // local session carriage. | |||
| 21 | - | DefaultStreamInboundChunks = 64 | |||
| 22 | - | // DefaultStreamInboundBytes bounds one logical stream's inbound chunk queue | |||
| 23 | - | // in bytes (4 MiB). | |||
| 24 | - | DefaultStreamInboundBytes = 4 << 20 | |||
| 25 | ) | 18 | ) | ||
| 26 | 19 | ||||
| 27 | // Config bounds one endpoint. The zero value is valid and selects every | 20 | // Config bounds one endpoint. The zero value is valid and selects every | ||
| @@ -29,10 +22,6 @@ const ( | |||||
| 29 | type Config struct { | 22 | type Config struct { | ||
| 30 | // MaxClients bounds concurrent accepted client connections. | 23 | // MaxClients bounds concurrent accepted client connections. | ||
| 31 | MaxClients int | 24 | MaxClients int | ||
| 32 | - | // StreamInboundChunks bounds one logical stream's inbound chunk queue. | |||
| 33 | - | StreamInboundChunks int | |||
| 34 | - | // StreamInboundBytes bounds one logical stream's inbound chunk bytes. | |||
| 35 | - | StreamInboundBytes uint64 | |||
| 36 | // HandshakeTimeout bounds one accepted connection's broker preamble, broker | 25 | // HandshakeTimeout bounds one accepted connection's broker preamble, broker | ||
| 37 | // admission, and the wait for its Register. The client dialer also bounds | 26 | // admission, and the wait for its Register. The client dialer also bounds | ||
| 38 | // its whole setup (Unix dial, preamble, Register send, and wait for | 27 | // its whole setup (Unix dial, preamble, Register send, and wait for | ||
| @@ -62,12 +51,6 @@ func (c Config) withDefaults() Config { | |||||
| 62 | if c.MaxClients <= 0 { | 51 | if c.MaxClients <= 0 { | ||
| 63 | c.MaxClients = DefaultMaxClients | 52 | c.MaxClients = DefaultMaxClients | ||
| 64 | } | 53 | } | ||
| 65 | - | if c.StreamInboundChunks <= 0 { | |||
| 66 | - | c.StreamInboundChunks = DefaultStreamInboundChunks | |||
| 67 | - | } | |||
| 68 | - | if c.StreamInboundBytes == 0 { | |||
| 69 | - | c.StreamInboundBytes = DefaultStreamInboundBytes | |||
| 70 | - | } | |||
| 71 | if c.HandshakeTimeout <= 0 { | 54 | if c.HandshakeTimeout <= 0 { | ||
| 72 | c.HandshakeTimeout = protocol.HandshakeTimeout | 55 | c.HandshakeTimeout = protocol.HandshakeTimeout | ||
| 73 | } | 56 | } | ||
| @@ -80,16 +63,11 @@ func (c Config) withDefaults() Config { | |||||
| 80 | // validate refuses a configuration whose explicit bounds cannot be honored. | 63 | // validate refuses a configuration whose explicit bounds cannot be honored. | ||
| 81 | // The zero value is always valid because withDefaults has already been applied. | 64 | // The zero value is always valid because withDefaults has already been applied. | ||
| 82 | func (c Config) validate() error { | 65 | func (c Config) validate() error { | ||
| 83 | - | if c.MaxClients <= 0 || c.StreamInboundChunks <= 0 || c.StreamInboundBytes == 0 || c.HandshakeTimeout <= 0 { | 66 | + | if c.MaxClients <= 0 || c.HandshakeTimeout <= 0 { |
| 84 | return ErrConfig | 67 | return ErrConfig | ||
| 85 | } | 68 | } | ||
| 86 | if c.MaxPendingOperations <= 0 || c.MaxPendingOperations > brokerwire.MaxPendingOperations { | 69 | if c.MaxPendingOperations <= 0 || c.MaxPendingOperations > brokerwire.MaxPendingOperations { | ||
| 87 | return ErrConfig | 70 | return ErrConfig | ||
| 88 | } | 71 | } | ||
| 89 | - | // A stream inbound byte bound below one maximum chunk can never accept a | |||
| 90 | - | // full stream frame, which would starve a legal peer. | |||
| 91 | - | if c.StreamInboundBytes < brokerwire.MaxStreamChunkBytes { | |||
| 92 | - | return ErrConfig | |||
| 93 | - | } | |||
| 94 | return nil | 72 | return nil | ||
| 95 | } | 73 | } | ||
M internal/adapters/brokeripc/errors.go +4 -4
| @@ -33,10 +33,10 @@ var ( | |||
| 33 | 33 | // ordering: a duplicate Register, a frame for an unallocated stream, or | |
| 34 | 34 | // stream data before the stream was established. | |
| 35 | 35 | ErrProtocol = errors.New("brokeripc: broker protocol violation") | |
| 36 | - | // ErrStreamBackpressure reports a logical stream whose local consumer is | |
| 37 | - | // not draining its bounded inbound queue. The affected stream is settled | |
| 38 | - | // alone; the connection and every sibling stream stay up. | |
| 39 | - | ErrStreamBackpressure = errors.New("brokeripc: stream consumer is not keeping up") | |
| 36 | + | // ErrStreamCredit reports a peer that sent stream data beyond the credit it | |
| 37 | + | // was granted, or granted credit beyond one stream window. The affected | |
| 38 | + | // stream is settled alone; the connection and every sibling stream stay up. | |
| 39 | + | ErrStreamCredit = errors.New("brokeripc: stream flow-control violation") | |
| 40 | 40 | // ErrStreamGone reports a stream the peer retired while this side was still | |
| 41 | 41 | // starting or using it. | |
| 42 | 42 | ErrStreamGone = errors.New("brokeripc: logical stream is gone") | |
| @@ -33,10 +33,10 @@ var ( | |||||
| 33 | // ordering: a duplicate Register, a frame for an unallocated stream, or | 33 | // ordering: a duplicate Register, a frame for an unallocated stream, or | ||
| 34 | // stream data before the stream was established. | 34 | // stream data before the stream was established. | ||
| 35 | ErrProtocol = errors.New("brokeripc: broker protocol violation") | 35 | ErrProtocol = errors.New("brokeripc: broker protocol violation") | ||
| 36 | - | // ErrStreamBackpressure reports a logical stream whose local consumer is | 36 | + | // ErrStreamCredit reports a peer that sent stream data beyond the credit it |
| 37 | - | // not draining its bounded inbound queue. The affected stream is settled | 37 | + | // was granted, or granted credit beyond one stream window. The affected |
| 38 | - | // alone; the connection and every sibling stream stay up. | 38 | + | // stream is settled alone; the connection and every sibling stream stay up. |
| 39 | - | ErrStreamBackpressure = errors.New("brokeripc: stream consumer is not keeping up") | 39 | + | ErrStreamCredit = errors.New("brokeripc: stream flow-control violation") |
| 40 | // ErrStreamGone reports a stream the peer retired while this side was still | 40 | // ErrStreamGone reports a stream the peer retired while this side was still | ||
| 41 | // starting or using it. | 41 | // starting or using it. | ||
| 42 | ErrStreamGone = errors.New("brokeripc: logical stream is gone") | 42 | ErrStreamGone = errors.New("brokeripc: logical stream is gone") | ||
M internal/adapters/brokeripc/limits_test.go +13 -45
| @@ -147,71 +147,39 @@ func TestConfigDefaultsAndValidation(t *testing.T) { | |||
| 147 | 147 | cfg := Config{}.withDefaults() | |
| 148 | 148 | require.NoError(t, cfg.validate()) | |
| 149 | 149 | require.Equal(t, DefaultMaxClients, cfg.MaxClients) | |
| 150 | - | require.Equal(t, DefaultStreamInboundChunks, cfg.StreamInboundChunks) | |
| 151 | - | require.Equal(t, uint64(DefaultStreamInboundBytes), cfg.StreamInboundBytes) | |
| 152 | 150 | require.Equal(t, brokerwire.MaxPendingOperations, cfg.MaxPendingOperations) | |
| 153 | 151 | ||
| 154 | - | require.ErrorIs(t, (Config{StreamInboundBytes: 1}).withDefaults().validate(), ErrConfig) | |
| 155 | 152 | require.ErrorIs(t, (Config{MaxPendingOperations: brokerwire.MaxPendingOperations + 1}).withDefaults().validate(), ErrConfig) | |
| 156 | 153 | // A negative explicit bound is replaced by its default rather than refused. | |
| 157 | - | require.NoError(t, (Config{MaxClients: -1, StreamInboundChunks: -1}).withDefaults().validate()) | |
| 154 | + | require.NoError(t, (Config{MaxClients: -1}).withDefaults().validate()) | |
| 158 | 155 | } | |
| 159 | 156 | ||
| 160 | - | // TestStreamPipeBoundsAndClose proves the per-stream byte channel is bounded, | |
| 161 | - | // reports backpressure instead of growing, and releases a parked reader on | |
| 162 | - | // close. | |
| 163 | - | func TestStreamPipeBoundsAndClose(t *testing.T) { | |
| 164 | - | pipe := newStreamPipe(2, 4096, func([]byte) error { return nil }) | |
| 165 | - | require.NoError(t, pipe.deliver([]byte("one"))) | |
| 166 | - | require.NoError(t, pipe.deliver([]byte("two"))) | |
| 167 | - | require.ErrorIs(t, pipe.deliver([]byte("three")), ErrStreamBackpressure) | |
| 168 | - | ||
| 169 | - | buf := make([]byte, 3) | |
| 170 | - | n, err := pipe.Read(buf) | |
| 171 | - | require.NoError(t, err) | |
| 172 | - | require.Equal(t, "one", string(buf[:n])) | |
| 173 | - | require.NoError(t, pipe.deliver([]byte("three"))) | |
| 174 | - | ||
| 175 | - | // An orderly Close keeps what is already queued ("two" and "three"): Read | |
| 176 | - | // drains it before observing io.EOF, so a chunk delivered just ahead of an | |
| 177 | - | // orderly close is never discarded underneath its reader (see | |
| 178 | - | // TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose for the focused | |
| 179 | - | // coverage of this behavior). | |
| 180 | - | require.NoError(t, pipe.Close()) | |
| 181 | - | require.ErrorIs(t, pipe.deliver([]byte("four")), ErrStreamGone) | |
| 182 | - | drained := make([]byte, 8) | |
| 183 | - | n, err = pipe.Read(drained) | |
| 184 | - | require.NoError(t, err) | |
| 185 | - | require.Equal(t, "two", string(drained[:n])) | |
| 186 | - | n, err = pipe.Read(drained) | |
| 187 | - | require.NoError(t, err) | |
| 188 | - | require.Equal(t, "three", string(drained[:n])) | |
| 189 | - | _, err = pipe.Read(drained) | |
| 190 | - | require.ErrorIs(t, err, io.EOF) | |
| 191 | - | ||
| 192 | - | // A reader parked on an empty pipe is released by closeWith with its cause. | |
| 193 | - | parked := newStreamPipe(1, 4096, func([]byte) error { return nil }) | |
| 157 | + | // TestStreamPipeCloseReleasesParkedReader proves a reader parked on an empty | |
| 158 | + | // pipe is released by closeWith with its cause. | |
| 159 | + | func TestStreamPipeCloseReleasesParkedReader(t *testing.T) { | |
| 160 | + | parked := newTestPipe() | |
| 194 | 161 | done := make(chan error, 1) | |
| 195 | 162 | go func() { | |
| 196 | - | _, err := parked.Read(buf) | |
| 163 | + | _, err := parked.Read(make([]byte, 3)) | |
| 197 | 164 | done <- err | |
| 198 | 165 | }() | |
| 199 | - | time.Sleep(20 * time.Millisecond) | |
| 200 | - | parked.closeWith(ErrStreamBackpressure) | |
| 166 | + | parked.closeWith(ErrStreamCredit) | |
| 201 | 167 | select { | |
| 202 | 168 | case err := <-done: | |
| 203 | - | require.ErrorIs(t, err, ErrStreamBackpressure) | |
| 169 | + | require.ErrorIs(t, err, ErrStreamCredit) | |
| 204 | 170 | case <-time.After(5 * time.Second): | |
| 205 | 171 | t.Fatal("closeWith must release a parked reader") | |
| 206 | 172 | } | |
| 173 | + | require.ErrorIs(t, parked.deliver([]byte("late")), ErrStreamGone) | |
| 207 | 174 | } | |
| 208 | 175 | ||
| 209 | - | // TestStreamPipeOutboundError proves a failing outbound hook surfaces instead of | |
| 176 | + | // TestStreamPipeOutboundError proves a failing sendFrame hook surfaces instead of | |
| 210 | 177 | // silently dropping bytes. | |
| 211 | 178 | func TestStreamPipeOutboundError(t *testing.T) { | |
| 212 | - | pipe := newStreamPipe(1, 4096, func([]byte) error { return ErrStreamBackpressure }) | |
| 179 | + | failure := errors.New("outbound failed") | |
| 180 | + | pipe := newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return failure }, func(uint64) error { return nil }) | |
| 213 | 181 | _, err := pipe.Write([]byte("payload")) | |
| 214 | - | require.ErrorIs(t, err, ErrStreamBackpressure) | |
| 182 | + | require.ErrorIs(t, err, failure) | |
| 215 | 183 | require.NoError(t, pipe.Close()) | |
| 216 | 184 | _, err = pipe.Write([]byte("payload")) | |
| 217 | 185 | require.ErrorIs(t, err, ErrStreamGone) | |
| @@ -147,71 +147,39 @@ func TestConfigDefaultsAndValidation(t *testing.T) { | |||||
| 147 | cfg := Config{}.withDefaults() | 147 | cfg := Config{}.withDefaults() | ||
| 148 | require.NoError(t, cfg.validate()) | 148 | require.NoError(t, cfg.validate()) | ||
| 149 | require.Equal(t, DefaultMaxClients, cfg.MaxClients) | 149 | require.Equal(t, DefaultMaxClients, cfg.MaxClients) | ||
| 150 | - | require.Equal(t, DefaultStreamInboundChunks, cfg.StreamInboundChunks) | |||
| 151 | - | require.Equal(t, uint64(DefaultStreamInboundBytes), cfg.StreamInboundBytes) | |||
| 152 | require.Equal(t, brokerwire.MaxPendingOperations, cfg.MaxPendingOperations) | 150 | require.Equal(t, brokerwire.MaxPendingOperations, cfg.MaxPendingOperations) | ||
| 153 | 151 | ||||
| 154 | - | require.ErrorIs(t, (Config{StreamInboundBytes: 1}).withDefaults().validate(), ErrConfig) | |||
| 155 | require.ErrorIs(t, (Config{MaxPendingOperations: brokerwire.MaxPendingOperations + 1}).withDefaults().validate(), ErrConfig) | 152 | require.ErrorIs(t, (Config{MaxPendingOperations: brokerwire.MaxPendingOperations + 1}).withDefaults().validate(), ErrConfig) | ||
| 156 | // A negative explicit bound is replaced by its default rather than refused. | 153 | // A negative explicit bound is replaced by its default rather than refused. | ||
| 157 | - | require.NoError(t, (Config{MaxClients: -1, StreamInboundChunks: -1}).withDefaults().validate()) | 154 | + | require.NoError(t, (Config{MaxClients: -1}).withDefaults().validate()) |
| 158 | } | 155 | } | ||
| 159 | 156 | ||||
| 160 | - | // TestStreamPipeBoundsAndClose proves the per-stream byte channel is bounded, | 157 | + | // TestStreamPipeCloseReleasesParkedReader proves a reader parked on an empty |
| 161 | - | // reports backpressure instead of growing, and releases a parked reader on | 158 | + | // pipe is released by closeWith with its cause. |
| 162 | - | // close. | 159 | + | func TestStreamPipeCloseReleasesParkedReader(t *testing.T) { |
| 163 | - | func TestStreamPipeBoundsAndClose(t *testing.T) { | 160 | + | parked := newTestPipe() |
| 164 | - | pipe := newStreamPipe(2, 4096, func([]byte) error { return nil }) | |||
| 165 | - | require.NoError(t, pipe.deliver([]byte("one"))) | |||
| 166 | - | require.NoError(t, pipe.deliver([]byte("two"))) | |||
| 167 | - | require.ErrorIs(t, pipe.deliver([]byte("three")), ErrStreamBackpressure) | |||
| 168 | - | ||||
| 169 | - | buf := make([]byte, 3) | |||
| 170 | - | n, err := pipe.Read(buf) | |||
| 171 | - | require.NoError(t, err) | |||
| 172 | - | require.Equal(t, "one", string(buf[:n])) | |||
| 173 | - | require.NoError(t, pipe.deliver([]byte("three"))) | |||
| 174 | - | ||||
| 175 | - | // An orderly Close keeps what is already queued ("two" and "three"): Read | |||
| 176 | - | // drains it before observing io.EOF, so a chunk delivered just ahead of an | |||
| 177 | - | // orderly close is never discarded underneath its reader (see | |||
| 178 | - | // TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose for the focused | |||
| 179 | - | // coverage of this behavior). | |||
| 180 | - | require.NoError(t, pipe.Close()) | |||
| 181 | - | require.ErrorIs(t, pipe.deliver([]byte("four")), ErrStreamGone) | |||
| 182 | - | drained := make([]byte, 8) | |||
| 183 | - | n, err = pipe.Read(drained) | |||
| 184 | - | require.NoError(t, err) | |||
| 185 | - | require.Equal(t, "two", string(drained[:n])) | |||
| 186 | - | n, err = pipe.Read(drained) | |||
| 187 | - | require.NoError(t, err) | |||
| 188 | - | require.Equal(t, "three", string(drained[:n])) | |||
| 189 | - | _, err = pipe.Read(drained) | |||
| 190 | - | require.ErrorIs(t, err, io.EOF) | |||
| 191 | - | ||||
| 192 | - | // A reader parked on an empty pipe is released by closeWith with its cause. | |||
| 193 | - | parked := newStreamPipe(1, 4096, func([]byte) error { return nil }) | |||
| 194 | done := make(chan error, 1) | 161 | done := make(chan error, 1) | ||
| 195 | go func() { | 162 | go func() { | ||
| 196 | - | _, err := parked.Read(buf) | 163 | + | _, err := parked.Read(make([]byte, 3)) |
| 197 | done <- err | 164 | done <- err | ||
| 198 | }() | 165 | }() | ||
| 199 | - | time.Sleep(20 * time.Millisecond) | 166 | + | parked.closeWith(ErrStreamCredit) |
| 200 | - | parked.closeWith(ErrStreamBackpressure) | |||
| 201 | select { | 167 | select { | ||
| 202 | case err := <-done: | 168 | case err := <-done: | ||
| 203 | - | require.ErrorIs(t, err, ErrStreamBackpressure) | 169 | + | require.ErrorIs(t, err, ErrStreamCredit) |
| 204 | case <-time.After(5 * time.Second): | 170 | case <-time.After(5 * time.Second): | ||
| 205 | t.Fatal("closeWith must release a parked reader") | 171 | t.Fatal("closeWith must release a parked reader") | ||
| 206 | } | 172 | } | ||
| 173 | + | require.ErrorIs(t, parked.deliver([]byte("late")), ErrStreamGone) | |||
| 207 | } | 174 | } | ||
| 208 | 175 | ||||
| 209 | - | // TestStreamPipeOutboundError proves a failing outbound hook surfaces instead of | 176 | + | // TestStreamPipeOutboundError proves a failing sendFrame hook surfaces instead of |
| 210 | // silently dropping bytes. | 177 | // silently dropping bytes. | ||
| 211 | func TestStreamPipeOutboundError(t *testing.T) { | 178 | func TestStreamPipeOutboundError(t *testing.T) { | ||
| 212 | - | pipe := newStreamPipe(1, 4096, func([]byte) error { return ErrStreamBackpressure }) | 179 | + | failure := errors.New("outbound failed") |
| 180 | + | pipe := newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return failure }, func(uint64) error { return nil }) | |||
| 213 | _, err := pipe.Write([]byte("payload")) | 181 | _, err := pipe.Write([]byte("payload")) | ||
| 214 | - | require.ErrorIs(t, err, ErrStreamBackpressure) | 182 | + | require.ErrorIs(t, err, failure) |
| 215 | require.NoError(t, pipe.Close()) | 183 | require.NoError(t, pipe.Close()) | ||
| 216 | _, err = pipe.Write([]byte("payload")) | 184 | _, err = pipe.Write([]byte("payload")) | ||
| 217 | require.ErrorIs(t, err, ErrStreamGone) | 185 | require.ErrorIs(t, err, ErrStreamGone) | ||
M internal/adapters/brokeripc/pipe.go +150 -53
| @@ -3,53 +3,84 @@ package brokeripc | |||
| 3 | 3 | import ( | |
| 4 | 4 | "io" | |
| 5 | 5 | "sync" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/vev/internal/adapters/brokerwire" | |
| 6 | 8 | ) | |
| 7 | 9 | ||
| 8 | - | // streamPipe is the private ordered, bounded byte channel between one broker | |
| 9 | - | // wire session and the sessionwire framing of one logical stream. | |
| 10 | + | // streamPipe is the private ordered, credit-controlled byte channel between one | |
| 11 | + | // broker wire session and the sessionwire framing of one logical stream. | |
| 10 | 12 | // | |
| 11 | - | // The wire reader never blocks on a slow consumer: a chunk the peer sent is | |
| 12 | - | // appended to a bounded queue and refused with ErrStreamBackpressure once the | |
| 13 | - | // bound is reached, so only that stream is settled and every sibling and the | |
| 14 | - | // physical connection stay untouched. The consumer never blocks the writer: | |
| 15 | - | // Read serves already-queued chunks or parks on the wake channel, and Close | |
| 16 | - | // releases both ends promptly. Write hands each byte slice to the injected | |
| 17 | - | // outbound hook, which owns chunking it under the negotiated stream chunk | |
| 18 | - | // ceiling and enqueueing the frames on the connection. | |
| 13 | + | // Each direction of a stream has a brokerwire.StreamWindowBytes credit window. | |
| 14 | + | // The sender spends brokerwire.StreamChunkCredit per data frame before writing | |
| 15 | + | // it and waits while it has too little, so a slow consumer slows its own | |
| 16 | + | // sender instead of failing the stream. The receiver returns the credit of | |
| 17 | + | // every frame its consumer has fully read with one StreamWindowUpdate per | |
| 18 | + | // batch. Because a conforming peer never exceeds the window, the wire reader | |
| 19 | + | // never blocks: it appends each frame to the queue, and a frame beyond the | |
| 20 | + | // granted credit is a peer violation (ErrStreamCredit) that settles only that | |
| 21 | + | // stream. Close releases the reader, a writer waiting for credit, and the | |
| 22 | + | // writer's peer promptly. | |
| 19 | 23 | type streamPipe struct { | |
| 20 | - | mu sync.Mutex | |
| 21 | - | queue [][]byte | |
| 22 | - | queued int | |
| 23 | - | closed bool | |
| 24 | - | err error | |
| 25 | - | maxChunk int | |
| 26 | - | maxBytes int | |
| 24 | + | mu sync.Mutex | |
| 25 | + | queue []pipeChunk | |
| 26 | + | closed bool | |
| 27 | + | err error | |
| 28 | + | ||
| 29 | + | // recvUsed is the credit held by queued frames, recvReturn the credit of | |
| 30 | + | // fully read frames not yet granted back, and returnAt the batch size. | |
| 31 | + | recvUsed uint64 | |
| 32 | + | recvReturn uint64 | |
| 33 | + | returnAt uint64 | |
| 34 | + | // sendCredit is what the peer still allows this side to send. | |
| 35 | + | sendCredit uint64 | |
| 36 | + | creditWake chan struct{} | |
| 27 | 37 | ||
| 28 | 38 | wake chan struct{} | |
| 29 | 39 | closedCh chan struct{} | |
| 30 | 40 | ||
| 31 | - | outbound func([]byte) error | |
| 41 | + | ceiling int | |
| 42 | + | sendFrame func([]byte) error | |
| 43 | + | grant func(uint64) error | |
| 32 | 44 | } | |
| 33 | 45 | ||
| 34 | - | // newStreamPipe builds one bounded stream carriage. outbound must not be nil. | |
| 35 | - | func newStreamPipe(maxChunks int, maxBytes uint64, outbound func([]byte) error) *streamPipe { | |
| 36 | - | if maxChunks <= 0 { | |
| 37 | - | maxChunks = DefaultStreamInboundChunks | |
| 38 | - | } | |
| 39 | - | if maxBytes == 0 { | |
| 40 | - | maxBytes = DefaultStreamInboundBytes | |
| 41 | - | } | |
| 46 | + | // pipeChunk is one queued inbound frame and the credit it holds until it is | |
| 47 | + | // fully read. | |
| 48 | + | type pipeChunk struct { | |
| 49 | + | data []byte | |
| 50 | + | cost uint64 | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | // newStreamPipe builds one stream carriage. Write splits outbound bytes into | |
| 54 | + | // frames of at most ceiling bytes, the negotiated stream chunk limit, and hands | |
| 55 | + | // each to sendFrame once its credit is reserved; grant returns consumed inbound | |
| 56 | + | // credit to the peer. Neither hook may be nil. | |
| 57 | + | func newStreamPipe(ceiling int, sendFrame func([]byte) error, grant func(uint64) error) *streamPipe { | |
| 58 | + | window := brokerwire.StreamWindowBytes | |
| 42 | 59 | return &streamPipe{ | |
| 43 | - | maxChunk: maxChunks, | |
| 44 | - | maxBytes: int(maxBytes), | |
| 45 | - | wake: make(chan struct{}, 1), | |
| 46 | - | closedCh: make(chan struct{}), | |
| 47 | - | outbound: outbound, | |
| 60 | + | returnAt: streamCreditReturnAt(window), | |
| 61 | + | sendCredit: window, | |
| 62 | + | creditWake: make(chan struct{}), | |
| 63 | + | wake: make(chan struct{}, 1), | |
| 64 | + | closedCh: make(chan struct{}), | |
| 65 | + | ceiling: ceiling, | |
| 66 | + | sendFrame: sendFrame, | |
| 67 | + | grant: grant, | |
| 48 | 68 | } | |
| 49 | 69 | } | |
| 50 | 70 | ||
| 51 | - | // Read serves the next queued chunk, blocking until one arrives or the pipe | |
| 52 | - | // closes. A partial copy retains the unread remainder for the next call. | |
| 71 | + | // streamCreditReturnAt is the consumed credit a receiver batches before it | |
| 72 | + | // grants it back: half the window, lowered so a sender blocked on a maximum | |
| 73 | + | // frame is always released. A sender waits only once more than | |
| 74 | + | // window-StreamChunkCredit(max) is unreturned, so granting at or below that | |
| 75 | + | // point can never deadlock. | |
| 76 | + | func streamCreditReturnAt(window uint64) uint64 { | |
| 77 | + | cost := brokerwire.StreamChunkCredit(int(brokerwire.MaxStreamChunkBytes)) | |
| 78 | + | return max(min(window/2, window-cost+1), 1) | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | // Read serves the next queued frame, blocking until one arrives or the pipe | |
| 82 | + | // closes. A partial copy retains the unread remainder for the next call. The | |
| 83 | + | // credit of each fully read frame is returned to the peer in batches. | |
| 53 | 84 | func (p *streamPipe) Read(b []byte) (int, error) { | |
| 54 | 85 | if len(b) == 0 { | |
| 55 | 86 | return 0, nil | |
| @@ -57,16 +88,26 @@ func (p *streamPipe) Read(b []byte) (int, error) { | |||
| 57 | 88 | for { | |
| 58 | 89 | p.mu.Lock() | |
| 59 | 90 | if len(p.queue) > 0 { | |
| 60 | - | chunk := p.queue[0] | |
| 61 | - | n := copy(b, chunk) | |
| 62 | - | if n == len(chunk) { | |
| 63 | - | p.queue[0] = nil | |
| 91 | + | chunk := &p.queue[0] | |
| 92 | + | n := copy(b, chunk.data) | |
| 93 | + | var grant uint64 | |
| 94 | + | if n == len(chunk.data) { | |
| 95 | + | p.recvUsed -= chunk.cost | |
| 96 | + | p.recvReturn += chunk.cost | |
| 97 | + | p.queue[0] = pipeChunk{} | |
| 64 | 98 | p.queue = p.queue[1:] | |
| 99 | + | if p.recvReturn >= p.returnAt && !p.closed { | |
| 100 | + | grant, p.recvReturn = p.recvReturn, 0 | |
| 101 | + | } | |
| 65 | 102 | } else { | |
| 66 | - | p.queue[0] = chunk[n:] | |
| 103 | + | chunk.data = chunk.data[n:] | |
| 67 | 104 | } | |
| 68 | - | p.queued -= n | |
| 69 | 105 | p.mu.Unlock() | |
| 106 | + | if grant > 0 { | |
| 107 | + | // A failed grant means the connection itself is failing; its | |
| 108 | + | // terminal path settles this stream, so there is nothing to retry. | |
| 109 | + | _ = p.grant(grant) | |
| 110 | + | } | |
| 70 | 111 | return n, nil | |
| 71 | 112 | } | |
| 72 | 113 | if p.closed { | |
| @@ -85,40 +126,96 @@ func (p *streamPipe) Read(b []byte) (int, error) { | |||
| 85 | 126 | } | |
| 86 | 127 | } | |
| 87 | 128 | ||
| 88 | - | // Write hands one byte slice to the outbound hook. It returns ErrClosed once the | |
| 89 | - | // pipe reached its terminal outcome. | |
| 129 | + | // Write sends b in order as frames under the chunk ceiling, waiting for the | |
| 130 | + | // peer's credit before each one: a peer that is not reading slows this writer | |
| 131 | + | // instead of losing the stream. It returns ErrStreamGone once the pipe reached | |
| 132 | + | // its terminal outcome. | |
| 90 | 133 | func (p *streamPipe) Write(b []byte) (int, error) { | |
| 134 | + | if p.ceiling <= 0 { | |
| 135 | + | return 0, ErrConfig | |
| 136 | + | } | |
| 91 | 137 | p.mu.Lock() | |
| 92 | 138 | closed := p.closed | |
| 93 | 139 | p.mu.Unlock() | |
| 94 | 140 | if closed { | |
| 95 | 141 | return 0, ErrStreamGone | |
| 96 | 142 | } | |
| 97 | - | if err := p.outbound(b); err != nil { | |
| 98 | - | return 0, err | |
| 143 | + | for data := b; len(data) > 0; { | |
| 144 | + | n := min(len(data), p.ceiling) | |
| 145 | + | if err := p.reserve(n); err != nil { | |
| 146 | + | return 0, err | |
| 147 | + | } | |
| 148 | + | if err := p.sendFrame(data[:n]); err != nil { | |
| 149 | + | return 0, err | |
| 150 | + | } | |
| 151 | + | data = data[n:] | |
| 99 | 152 | } | |
| 100 | 153 | return len(b), nil | |
| 101 | 154 | } | |
| 102 | 155 | ||
| 103 | - | // deliver accepts one inbound chunk from the connection reader. It returns | |
| 104 | - | // ErrStreamBackpressure when the bound is reached and ErrStreamGone once the | |
| 105 | - | // pipe is closed; the caller settles the affected stream in both cases. | |
| 156 | + | // reserve spends the send credit of one outbound frame of n bytes, waiting for | |
| 157 | + | // the peer to grant more when it is short. It returns ErrStreamGone once the | |
| 158 | + | // pipe closes, so a writer waiting for credit is always released. | |
| 159 | + | func (p *streamPipe) reserve(n int) error { | |
| 160 | + | cost := brokerwire.StreamChunkCredit(n) | |
| 161 | + | for { | |
| 162 | + | p.mu.Lock() | |
| 163 | + | if p.closed { | |
| 164 | + | p.mu.Unlock() | |
| 165 | + | return ErrStreamGone | |
| 166 | + | } | |
| 167 | + | if p.sendCredit >= cost { | |
| 168 | + | p.sendCredit -= cost | |
| 169 | + | p.mu.Unlock() | |
| 170 | + | return nil | |
| 171 | + | } | |
| 172 | + | wake := p.creditWake | |
| 173 | + | p.mu.Unlock() | |
| 174 | + | select { | |
| 175 | + | case <-wake: | |
| 176 | + | case <-p.closedCh: | |
| 177 | + | } | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | // granted adds credit the peer returned. Credit beyond the window is a peer | |
| 182 | + | // flow-control violation. | |
| 183 | + | func (p *streamPipe) granted(credit uint64) error { | |
| 184 | + | p.mu.Lock() | |
| 185 | + | defer p.mu.Unlock() | |
| 186 | + | if p.closed { | |
| 187 | + | return nil | |
| 188 | + | } | |
| 189 | + | if credit > brokerwire.StreamWindowBytes-p.sendCredit { | |
| 190 | + | return ErrStreamCredit | |
| 191 | + | } | |
| 192 | + | p.sendCredit += credit | |
| 193 | + | close(p.creditWake) | |
| 194 | + | p.creditWake = make(chan struct{}) | |
| 195 | + | return nil | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | // deliver accepts one inbound frame from the connection reader. It returns | |
| 199 | + | // ErrStreamCredit when the peer sent beyond the credit it was granted and | |
| 200 | + | // ErrStreamGone once the pipe is closed; the caller settles the affected stream | |
| 201 | + | // in both cases. The pipe takes ownership of chunk: callers pass freshly | |
| 202 | + | // decoded bytes and never touch them again. | |
| 106 | 203 | func (p *streamPipe) deliver(chunk []byte) error { | |
| 107 | 204 | if len(chunk) == 0 { | |
| 108 | 205 | return nil | |
| 109 | 206 | } | |
| 207 | + | cost := brokerwire.StreamChunkCredit(len(chunk)) | |
| 110 | 208 | p.mu.Lock() | |
| 111 | 209 | if p.closed { | |
| 112 | 210 | p.mu.Unlock() | |
| 113 | 211 | return ErrStreamGone | |
| 114 | 212 | } | |
| 115 | - | if len(p.queue) >= p.maxChunk || p.queued+len(chunk) > p.maxBytes { | |
| 213 | + | if p.recvUsed+p.recvReturn+cost > brokerwire.StreamWindowBytes { | |
| 116 | 214 | p.mu.Unlock() | |
| 117 | - | return ErrStreamBackpressure | |
| 215 | + | return ErrStreamCredit | |
| 118 | 216 | } | |
| 119 | - | owned := append([]byte(nil), chunk...) | |
| 120 | - | p.queue = append(p.queue, owned) | |
| 121 | - | p.queued += len(owned) | |
| 217 | + | p.queue = append(p.queue, pipeChunk{data: chunk, cost: cost}) | |
| 218 | + | p.recvUsed += cost | |
| 122 | 219 | p.mu.Unlock() | |
| 123 | 220 | select { | |
| 124 | 221 | case p.wake <- struct{}{}: | |
| @@ -128,7 +225,7 @@ func (p *streamPipe) deliver(chunk []byte) error { | |||
| 128 | 225 | } | |
| 129 | 226 | ||
| 130 | 227 | // closeWith settles the pipe exactly once with cause. A nil cause is an orderly | |
| 131 | - | // close; every blocked Read observes it promptly. On a nil cause, any chunks | |
| 228 | + | // close; every blocked Read observes it promptly. On a nil cause, any frames | |
| 132 | 229 | // already queued are kept: Read serves them first and only returns io.EOF once | |
| 133 | 230 | // the queue is drained, so a reply that arrived just before an orderly peer | |
| 134 | 231 | // close is never discarded underneath its reader. An error cause is a failure: | |
| @@ -144,7 +241,7 @@ func (p *streamPipe) closeWith(cause error) { | |||
| 144 | 241 | p.err = cause | |
| 145 | 242 | if cause != nil { | |
| 146 | 243 | p.queue = nil | |
| 147 | - | p.queued = 0 | |
| 244 | + | p.recvUsed = 0 | |
| 148 | 245 | } | |
| 149 | 246 | p.mu.Unlock() | |
| 150 | 247 | close(p.closedCh) | |
| @@ -3,53 +3,84 @@ package brokeripc | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "io" | 4 | "io" | ||
| 5 | "sync" | 5 | "sync" | ||
| 6 | + | ||||
| 7 | + | "github.com/bnema/vev/internal/adapters/brokerwire" | |||
| 6 | ) | 8 | ) | ||
| 7 | 9 | ||||
| 8 | - | // streamPipe is the private ordered, bounded byte channel between one broker | 10 | + | // streamPipe is the private ordered, credit-controlled byte channel between one |
| 9 | - | // wire session and the sessionwire framing of one logical stream. | 11 | + | // broker wire session and the sessionwire framing of one logical stream. |
| 10 | // | 12 | // | ||
| 11 | - | // The wire reader never blocks on a slow consumer: a chunk the peer sent is | 13 | + | // Each direction of a stream has a brokerwire.StreamWindowBytes credit window. |
| 12 | - | // appended to a bounded queue and refused with ErrStreamBackpressure once the | 14 | + | // The sender spends brokerwire.StreamChunkCredit per data frame before writing |
| 13 | - | // bound is reached, so only that stream is settled and every sibling and the | 15 | + | // it and waits while it has too little, so a slow consumer slows its own |
| 14 | - | // physical connection stay untouched. The consumer never blocks the writer: | 16 | + | // sender instead of failing the stream. The receiver returns the credit of |
| 15 | - | // Read serves already-queued chunks or parks on the wake channel, and Close | 17 | + | // every frame its consumer has fully read with one StreamWindowUpdate per |
| 16 | - | // releases both ends promptly. Write hands each byte slice to the injected | 18 | + | // batch. Because a conforming peer never exceeds the window, the wire reader |
| 17 | - | // outbound hook, which owns chunking it under the negotiated stream chunk | 19 | + | // never blocks: it appends each frame to the queue, and a frame beyond the |
| 18 | - | // ceiling and enqueueing the frames on the connection. | 20 | + | // granted credit is a peer violation (ErrStreamCredit) that settles only that |
| 21 | + | // stream. Close releases the reader, a writer waiting for credit, and the | |||
| 22 | + | // writer's peer promptly. | |||
| 19 | type streamPipe struct { | 23 | type streamPipe struct { | ||
| 20 | - | mu sync.Mutex | 24 | + | mu sync.Mutex |
| 21 | - | queue [][]byte | 25 | + | queue []pipeChunk |
| 22 | - | queued int | 26 | + | closed bool |
| 23 | - | closed bool | 27 | + | err error |
| 24 | - | err error | 28 | + | |
| 25 | - | maxChunk int | 29 | + | // recvUsed is the credit held by queued frames, recvReturn the credit of |
| 26 | - | maxBytes int | 30 | + | // fully read frames not yet granted back, and returnAt the batch size. |
| 31 | + | recvUsed uint64 | |||
| 32 | + | recvReturn uint64 | |||
| 33 | + | returnAt uint64 | |||
| 34 | + | // sendCredit is what the peer still allows this side to send. | |||
| 35 | + | sendCredit uint64 | |||
| 36 | + | creditWake chan struct{} | |||
| 27 | 37 | ||||
| 28 | wake chan struct{} | 38 | wake chan struct{} | ||
| 29 | closedCh chan struct{} | 39 | closedCh chan struct{} | ||
| 30 | 40 | ||||
| 31 | - | outbound func([]byte) error | 41 | + | ceiling int |
| 42 | + | sendFrame func([]byte) error | |||
| 43 | + | grant func(uint64) error | |||
| 32 | } | 44 | } | ||
| 33 | 45 | ||||
| 34 | - | // newStreamPipe builds one bounded stream carriage. outbound must not be nil. | 46 | + | // pipeChunk is one queued inbound frame and the credit it holds until it is |
| 35 | - | func newStreamPipe(maxChunks int, maxBytes uint64, outbound func([]byte) error) *streamPipe { | 47 | + | // fully read. |
| 36 | - | if maxChunks <= 0 { | 48 | + | type pipeChunk struct { |
| 37 | - | maxChunks = DefaultStreamInboundChunks | 49 | + | data []byte |
| 38 | - | } | 50 | + | cost uint64 |
| 39 | - | if maxBytes == 0 { | 51 | + | } |
| 40 | - | maxBytes = DefaultStreamInboundBytes | 52 | + | |
| 41 | - | } | 53 | + | // newStreamPipe builds one stream carriage. Write splits outbound bytes into |
| 54 | + | // frames of at most ceiling bytes, the negotiated stream chunk limit, and hands | |||
| 55 | + | // each to sendFrame once its credit is reserved; grant returns consumed inbound | |||
| 56 | + | // credit to the peer. Neither hook may be nil. | |||
| 57 | + | func newStreamPipe(ceiling int, sendFrame func([]byte) error, grant func(uint64) error) *streamPipe { | |||
| 58 | + | window := brokerwire.StreamWindowBytes | |||
| 42 | return &streamPipe{ | 59 | return &streamPipe{ | ||
| 43 | - | maxChunk: maxChunks, | 60 | + | returnAt: streamCreditReturnAt(window), |
| 44 | - | maxBytes: int(maxBytes), | 61 | + | sendCredit: window, |
| 45 | - | wake: make(chan struct{}, 1), | 62 | + | creditWake: make(chan struct{}), |
| 46 | - | closedCh: make(chan struct{}), | 63 | + | wake: make(chan struct{}, 1), |
| 47 | - | outbound: outbound, | 64 | + | closedCh: make(chan struct{}), |
| 65 | + | ceiling: ceiling, | |||
| 66 | + | sendFrame: sendFrame, | |||
| 67 | + | grant: grant, | |||
| 48 | } | 68 | } | ||
| 49 | } | 69 | } | ||
| 50 | 70 | ||||
| 51 | - | // Read serves the next queued chunk, blocking until one arrives or the pipe | 71 | + | // streamCreditReturnAt is the consumed credit a receiver batches before it |
| 52 | - | // closes. A partial copy retains the unread remainder for the next call. | 72 | + | // grants it back: half the window, lowered so a sender blocked on a maximum |
| 73 | + | // frame is always released. A sender waits only once more than | |||
| 74 | + | // window-StreamChunkCredit(max) is unreturned, so granting at or below that | |||
| 75 | + | // point can never deadlock. | |||
| 76 | + | func streamCreditReturnAt(window uint64) uint64 { | |||
| 77 | + | cost := brokerwire.StreamChunkCredit(int(brokerwire.MaxStreamChunkBytes)) | |||
| 78 | + | return max(min(window/2, window-cost+1), 1) | |||
| 79 | + | } | |||
| 80 | + | ||||
| 81 | + | // Read serves the next queued frame, blocking until one arrives or the pipe | |||
| 82 | + | // closes. A partial copy retains the unread remainder for the next call. The | |||
| 83 | + | // credit of each fully read frame is returned to the peer in batches. | |||
| 53 | func (p *streamPipe) Read(b []byte) (int, error) { | 84 | func (p *streamPipe) Read(b []byte) (int, error) { | ||
| 54 | if len(b) == 0 { | 85 | if len(b) == 0 { | ||
| 55 | return 0, nil | 86 | return 0, nil | ||
| @@ -57,16 +88,26 @@ func (p *streamPipe) Read(b []byte) (int, error) { | |||||
| 57 | for { | 88 | for { | ||
| 58 | p.mu.Lock() | 89 | p.mu.Lock() | ||
| 59 | if len(p.queue) > 0 { | 90 | if len(p.queue) > 0 { | ||
| 60 | - | chunk := p.queue[0] | 91 | + | chunk := &p.queue[0] |
| 61 | - | n := copy(b, chunk) | 92 | + | n := copy(b, chunk.data) |
| 62 | - | if n == len(chunk) { | 93 | + | var grant uint64 |
| 63 | - | p.queue[0] = nil | 94 | + | if n == len(chunk.data) { |
| 95 | + | p.recvUsed -= chunk.cost | |||
| 96 | + | p.recvReturn += chunk.cost | |||
| 97 | + | p.queue[0] = pipeChunk{} | |||
| 64 | p.queue = p.queue[1:] | 98 | p.queue = p.queue[1:] | ||
| 99 | + | if p.recvReturn >= p.returnAt && !p.closed { | |||
| 100 | + | grant, p.recvReturn = p.recvReturn, 0 | |||
| 101 | + | } | |||
| 65 | } else { | 102 | } else { | ||
| 66 | - | p.queue[0] = chunk[n:] | 103 | + | chunk.data = chunk.data[n:] |
| 67 | } | 104 | } | ||
| 68 | - | p.queued -= n | |||
| 69 | p.mu.Unlock() | 105 | p.mu.Unlock() | ||
| 106 | + | if grant > 0 { | |||
| 107 | + | // A failed grant means the connection itself is failing; its | |||
| 108 | + | // terminal path settles this stream, so there is nothing to retry. | |||
| 109 | + | _ = p.grant(grant) | |||
| 110 | + | } | |||
| 70 | return n, nil | 111 | return n, nil | ||
| 71 | } | 112 | } | ||
| 72 | if p.closed { | 113 | if p.closed { | ||
| @@ -85,40 +126,96 @@ func (p *streamPipe) Read(b []byte) (int, error) { | |||||
| 85 | } | 126 | } | ||
| 86 | } | 127 | } | ||
| 87 | 128 | ||||
| 88 | - | // Write hands one byte slice to the outbound hook. It returns ErrClosed once the | 129 | + | // Write sends b in order as frames under the chunk ceiling, waiting for the |
| 89 | - | // pipe reached its terminal outcome. | 130 | + | // peer's credit before each one: a peer that is not reading slows this writer |
| 131 | + | // instead of losing the stream. It returns ErrStreamGone once the pipe reached | |||
| 132 | + | // its terminal outcome. | |||
| 90 | func (p *streamPipe) Write(b []byte) (int, error) { | 133 | func (p *streamPipe) Write(b []byte) (int, error) { | ||
| 134 | + | if p.ceiling <= 0 { | |||
| 135 | + | return 0, ErrConfig | |||
| 136 | + | } | |||
| 91 | p.mu.Lock() | 137 | p.mu.Lock() | ||
| 92 | closed := p.closed | 138 | closed := p.closed | ||
| 93 | p.mu.Unlock() | 139 | p.mu.Unlock() | ||
| 94 | if closed { | 140 | if closed { | ||
| 95 | return 0, ErrStreamGone | 141 | return 0, ErrStreamGone | ||
| 96 | } | 142 | } | ||
| 97 | - | if err := p.outbound(b); err != nil { | 143 | + | for data := b; len(data) > 0; { |
| 98 | - | return 0, err | 144 | + | n := min(len(data), p.ceiling) |
| 145 | + | if err := p.reserve(n); err != nil { | |||
| 146 | + | return 0, err | |||
| 147 | + | } | |||
| 148 | + | if err := p.sendFrame(data[:n]); err != nil { | |||
| 149 | + | return 0, err | |||
| 150 | + | } | |||
| 151 | + | data = data[n:] | |||
| 99 | } | 152 | } | ||
| 100 | return len(b), nil | 153 | return len(b), nil | ||
| 101 | } | 154 | } | ||
| 102 | 155 | ||||
| 103 | - | // deliver accepts one inbound chunk from the connection reader. It returns | 156 | + | // reserve spends the send credit of one outbound frame of n bytes, waiting for |
| 104 | - | // ErrStreamBackpressure when the bound is reached and ErrStreamGone once the | 157 | + | // the peer to grant more when it is short. It returns ErrStreamGone once the |
| 105 | - | // pipe is closed; the caller settles the affected stream in both cases. | 158 | + | // pipe closes, so a writer waiting for credit is always released. |
| 159 | + | func (p *streamPipe) reserve(n int) error { | |||
| 160 | + | cost := brokerwire.StreamChunkCredit(n) | |||
| 161 | + | for { | |||
| 162 | + | p.mu.Lock() | |||
| 163 | + | if p.closed { | |||
| 164 | + | p.mu.Unlock() | |||
| 165 | + | return ErrStreamGone | |||
| 166 | + | } | |||
| 167 | + | if p.sendCredit >= cost { | |||
| 168 | + | p.sendCredit -= cost | |||
| 169 | + | p.mu.Unlock() | |||
| 170 | + | return nil | |||
| 171 | + | } | |||
| 172 | + | wake := p.creditWake | |||
| 173 | + | p.mu.Unlock() | |||
| 174 | + | select { | |||
| 175 | + | case <-wake: | |||
| 176 | + | case <-p.closedCh: | |||
| 177 | + | } | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | // granted adds credit the peer returned. Credit beyond the window is a peer | |||
| 182 | + | // flow-control violation. | |||
| 183 | + | func (p *streamPipe) granted(credit uint64) error { | |||
| 184 | + | p.mu.Lock() | |||
| 185 | + | defer p.mu.Unlock() | |||
| 186 | + | if p.closed { | |||
| 187 | + | return nil | |||
| 188 | + | } | |||
| 189 | + | if credit > brokerwire.StreamWindowBytes-p.sendCredit { | |||
| 190 | + | return ErrStreamCredit | |||
| 191 | + | } | |||
| 192 | + | p.sendCredit += credit | |||
| 193 | + | close(p.creditWake) | |||
| 194 | + | p.creditWake = make(chan struct{}) | |||
| 195 | + | return nil | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | // deliver accepts one inbound frame from the connection reader. It returns | |||
| 199 | + | // ErrStreamCredit when the peer sent beyond the credit it was granted and | |||
| 200 | + | // ErrStreamGone once the pipe is closed; the caller settles the affected stream | |||
| 201 | + | // in both cases. The pipe takes ownership of chunk: callers pass freshly | |||
| 202 | + | // decoded bytes and never touch them again. | |||
| 106 | func (p *streamPipe) deliver(chunk []byte) error { | 203 | func (p *streamPipe) deliver(chunk []byte) error { | ||
| 107 | if len(chunk) == 0 { | 204 | if len(chunk) == 0 { | ||
| 108 | return nil | 205 | return nil | ||
| 109 | } | 206 | } | ||
| 207 | + | cost := brokerwire.StreamChunkCredit(len(chunk)) | |||
| 110 | p.mu.Lock() | 208 | p.mu.Lock() | ||
| 111 | if p.closed { | 209 | if p.closed { | ||
| 112 | p.mu.Unlock() | 210 | p.mu.Unlock() | ||
| 113 | return ErrStreamGone | 211 | return ErrStreamGone | ||
| 114 | } | 212 | } | ||
| 115 | - | if len(p.queue) >= p.maxChunk || p.queued+len(chunk) > p.maxBytes { | 213 | + | if p.recvUsed+p.recvReturn+cost > brokerwire.StreamWindowBytes { |
| 116 | p.mu.Unlock() | 214 | p.mu.Unlock() | ||
| 117 | - | return ErrStreamBackpressure | 215 | + | return ErrStreamCredit |
| 118 | } | 216 | } | ||
| 119 | - | owned := append([]byte(nil), chunk...) | 217 | + | p.queue = append(p.queue, pipeChunk{data: chunk, cost: cost}) |
| 120 | - | p.queue = append(p.queue, owned) | 218 | + | p.recvUsed += cost |
| 121 | - | p.queued += len(owned) | |||
| 122 | p.mu.Unlock() | 219 | p.mu.Unlock() | ||
| 123 | select { | 220 | select { | ||
| 124 | case p.wake <- struct{}{}: | 221 | case p.wake <- struct{}{}: | ||
| @@ -128,7 +225,7 @@ func (p *streamPipe) deliver(chunk []byte) error { | |||||
| 128 | } | 225 | } | ||
| 129 | 226 | ||||
| 130 | // closeWith settles the pipe exactly once with cause. A nil cause is an orderly | 227 | // closeWith settles the pipe exactly once with cause. A nil cause is an orderly | ||
| 131 | - | // close; every blocked Read observes it promptly. On a nil cause, any chunks | 228 | + | // close; every blocked Read observes it promptly. On a nil cause, any frames |
| 132 | // already queued are kept: Read serves them first and only returns io.EOF once | 229 | // already queued are kept: Read serves them first and only returns io.EOF once | ||
| 133 | // the queue is drained, so a reply that arrived just before an orderly peer | 230 | // the queue is drained, so a reply that arrived just before an orderly peer | ||
| 134 | // close is never discarded underneath its reader. An error cause is a failure: | 231 | // close is never discarded underneath its reader. An error cause is a failure: | ||
| @@ -144,7 +241,7 @@ func (p *streamPipe) closeWith(cause error) { | |||||
| 144 | p.err = cause | 241 | p.err = cause | ||
| 145 | if cause != nil { | 242 | if cause != nil { | ||
| 146 | p.queue = nil | 243 | p.queue = nil | ||
| 147 | - | p.queued = 0 | 244 | + | p.recvUsed = 0 |
| 148 | } | 245 | } | ||
| 149 | p.mu.Unlock() | 246 | p.mu.Unlock() | ||
| 150 | close(p.closedCh) | 247 | close(p.closedCh) | ||
M internal/adapters/brokeripc/pipe_test.go +100 -19
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "errors" | |
| 6 | 6 | "io" | |
| 7 | 7 | "testing" | |
| 8 | + | "time" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/vev/internal/adapters/brokerwire" | |
| 10 | 11 | "github.com/bnema/vev/internal/protocol" | |
| @@ -71,7 +72,7 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||
| 71 | 72 | ||
| 72 | 73 | for _, tt := range tests { | |
| 73 | 74 | t.Run(tt.name, func(t *testing.T) { | |
| 74 | - | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | |
| 75 | + | p := newTestPipe() | |
| 75 | 76 | for _, chunk := range tt.chunks { | |
| 76 | 77 | require.NoError(t, p.deliver(chunk)) | |
| 77 | 78 | } | |
| @@ -109,26 +110,106 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||
| 109 | 110 | } | |
| 110 | 111 | } | |
| 111 | 112 | ||
| 112 | - | // TestStreamPipeHoldsAFullOutputWindow pins the client pipe headroom for the | |
| 113 | - | // session output window. A client claims protocol.MaxOutputWindow unacked | |
| 114 | - | // Output frames; with a terminal that is not draining, all of them may sit in | |
| 115 | - | // this pipe at once, and overflow fails the whole stream | |
| 116 | - | // (ErrStreamBackpressure). The frame size is a typical text repaint: a | |
| 117 | - | // near-full page of a 500x140 terminal measured about 70 KB. Uncompressed diff | |
| 118 | - | // frames with per-cell truecolor SGR can be far larger (estimated near 1 MB at | |
| 119 | - | // 300x80), and eight of those can still overflow the default 4 MiB bound. | |
| 120 | - | func TestStreamPipeHoldsAFullOutputWindow(t *testing.T) { | |
| 121 | - | const frameBytes = 72 << 10 | |
| 113 | + | // newTestPipe is a pipe whose outbound writes and credit grants go nowhere. | |
| 114 | + | func newTestPipe() *streamPipe { | |
| 115 | + | return newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return nil }, func(uint64) error { return nil }) | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | // TestStreamWindowCoversTheSessionOutputWindow pins the sizing relation | |
| 119 | + | // between the two flow-control layers: one broker stream window holds the | |
| 120 | + | // daemon's unacknowledged byte budget, so the daemon is mostly paced by its own | |
| 121 | + | // ACK window and coalesces frames. Credit may still pace it at the margin: the | |
| 122 | + | // receiver batches up to half a window before returning it, chunks cost 64 | |
| 123 | + | // bytes of overhead, and one frame may overrun the byte budget (a big image). | |
| 124 | + | // Credit stays the hard bound either way. | |
| 125 | + | func TestStreamWindowCoversTheSessionOutputWindow(t *testing.T) { | |
| 126 | + | require.GreaterOrEqual(t, brokerwire.StreamWindowBytes, uint64(protocol.MaxOutputWindowBytes), | |
| 127 | + | "one stream window must hold the daemon's whole unacknowledged byte budget") | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | // TestStreamPipeCredit covers the receive and send sides of stream credit: | |
| 131 | + | // a peer may send up to one window, the consumer's reads return credit in | |
| 132 | + | // batches, a sender waits instead of failing while it has none, and grants or | |
| 133 | + | // data beyond the window are flow-control violations. | |
| 134 | + | func TestStreamPipeCredit(t *testing.T) { | |
| 122 | 135 | chunk := int(brokerwire.MaxStreamChunkBytes) | |
| 123 | - | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | |
| 124 | - | payload := bytes.Repeat([]byte{'x'}, frameBytes) | |
| 125 | - | for frame := range protocol.MaxOutputWindow { | |
| 126 | - | for data := payload; len(data) > 0; { | |
| 127 | - | n := min(len(data), chunk) | |
| 128 | - | require.NoError(t, p.deliver(data[:n]), "frame %d overflowed the client pipe", frame) | |
| 129 | - | data = data[n:] | |
| 136 | + | cost := brokerwire.StreamChunkCredit(chunk) | |
| 137 | + | perWindow := int(brokerwire.StreamWindowBytes / cost) | |
| 138 | + | ||
| 139 | + | t.Run("a full window is accepted and one more frame is a violation", func(t *testing.T) { | |
| 140 | + | p := newTestPipe() | |
| 141 | + | for range perWindow { | |
| 142 | + | require.NoError(t, p.deliver(make([]byte, chunk))) | |
| 130 | 143 | } | |
| 131 | - | } | |
| 144 | + | require.ErrorIs(t, p.deliver(make([]byte, chunk)), ErrStreamCredit) | |
| 145 | + | }) | |
| 146 | + | ||
| 147 | + | t.Run("reads return credit in batches", func(t *testing.T) { | |
| 148 | + | var grants []uint64 | |
| 149 | + | p := newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return nil }, func(c uint64) error { grants = append(grants, c); return nil }) | |
| 150 | + | for range perWindow { | |
| 151 | + | require.NoError(t, p.deliver(make([]byte, chunk))) | |
| 152 | + | } | |
| 153 | + | buf := make([]byte, chunk) | |
| 154 | + | for range perWindow { | |
| 155 | + | _, err := io.ReadFull(p, buf) | |
| 156 | + | require.NoError(t, err) | |
| 157 | + | } | |
| 158 | + | require.NotEmpty(t, grants) | |
| 159 | + | total := uint64(0) | |
| 160 | + | for _, g := range grants { | |
| 161 | + | require.GreaterOrEqual(t, g, streamCreditReturnAt(brokerwire.StreamWindowBytes)) | |
| 162 | + | total += g | |
| 163 | + | } | |
| 164 | + | require.LessOrEqual(t, total, uint64(perWindow)*cost) | |
| 165 | + | // Once the returned credit reaches the peer, the window is open again. | |
| 166 | + | require.NoError(t, p.deliver(make([]byte, chunk))) | |
| 167 | + | }) | |
| 168 | + | ||
| 169 | + | t.Run("a sender waits for credit and resumes on a grant", func(t *testing.T) { | |
| 170 | + | p := newTestPipe() | |
| 171 | + | for range perWindow { | |
| 172 | + | require.NoError(t, p.reserve(chunk)) | |
| 173 | + | } | |
| 174 | + | reserved := make(chan error, 1) | |
| 175 | + | go func() { reserved <- p.reserve(chunk) }() | |
| 176 | + | select { | |
| 177 | + | case err := <-reserved: | |
| 178 | + | t.Fatalf("reserve returned %v without credit", err) | |
| 179 | + | case <-time.After(20 * time.Millisecond): | |
| 180 | + | } | |
| 181 | + | require.NoError(t, p.granted(cost)) | |
| 182 | + | select { | |
| 183 | + | case err := <-reserved: | |
| 184 | + | require.NoError(t, err) | |
| 185 | + | case <-time.After(5 * time.Second): | |
| 186 | + | t.Fatal("a grant must release a sender waiting for credit") | |
| 187 | + | } | |
| 188 | + | }) | |
| 189 | + | ||
| 190 | + | t.Run("close releases a sender waiting for credit", func(t *testing.T) { | |
| 191 | + | p := newTestPipe() | |
| 192 | + | for range perWindow { | |
| 193 | + | require.NoError(t, p.reserve(chunk)) | |
| 194 | + | } | |
| 195 | + | reserved := make(chan error, 1) | |
| 196 | + | go func() { reserved <- p.reserve(chunk) }() | |
| 197 | + | p.closeWith(ErrStreamGone) | |
| 198 | + | select { | |
| 199 | + | case err := <-reserved: | |
| 200 | + | require.ErrorIs(t, err, ErrStreamGone) | |
| 201 | + | case <-time.After(5 * time.Second): | |
| 202 | + | t.Fatal("close must release a sender waiting for credit") | |
| 203 | + | } | |
| 204 | + | }) | |
| 205 | + | ||
| 206 | + | t.Run("a grant beyond the window is a violation", func(t *testing.T) { | |
| 207 | + | p := newTestPipe() | |
| 208 | + | require.ErrorIs(t, p.granted(1), ErrStreamCredit) | |
| 209 | + | require.NoError(t, p.reserve(chunk)) | |
| 210 | + | require.NoError(t, p.granted(cost)) | |
| 211 | + | require.ErrorIs(t, p.granted(1), ErrStreamCredit) | |
| 212 | + | }) | |
| 132 | 213 | } | |
| 133 | 214 | ||
| 134 | 215 | // TestClientStreamPrefersTheFullOutputWindow pins what a real broker-routed | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "io" | 6 | "io" | ||
| 7 | "testing" | 7 | "testing" | ||
| 8 | + | "time" | |||
| 8 | 9 | ||||
| 9 | "github.com/bnema/vev/internal/adapters/brokerwire" | 10 | "github.com/bnema/vev/internal/adapters/brokerwire" | ||
| 10 | "github.com/bnema/vev/internal/protocol" | 11 | "github.com/bnema/vev/internal/protocol" | ||
| @@ -71,7 +72,7 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||||
| 71 | 72 | ||||
| 72 | for _, tt := range tests { | 73 | for _, tt := range tests { | ||
| 73 | t.Run(tt.name, func(t *testing.T) { | 74 | t.Run(tt.name, func(t *testing.T) { | ||
| 74 | - | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | 75 | + | p := newTestPipe() |
| 75 | for _, chunk := range tt.chunks { | 76 | for _, chunk := range tt.chunks { | ||
| 76 | require.NoError(t, p.deliver(chunk)) | 77 | require.NoError(t, p.deliver(chunk)) | ||
| 77 | } | 78 | } | ||
| @@ -109,26 +110,106 @@ func TestStreamPipeCloseWithPreservesQueuedDataOnOrderlyClose(t *testing.T) { | |||||
| 109 | } | 110 | } | ||
| 110 | } | 111 | } | ||
| 111 | 112 | ||||
| 112 | - | // TestStreamPipeHoldsAFullOutputWindow pins the client pipe headroom for the | 113 | + | // newTestPipe is a pipe whose outbound writes and credit grants go nowhere. |
| 113 | - | // session output window. A client claims protocol.MaxOutputWindow unacked | 114 | + | func newTestPipe() *streamPipe { |
| 114 | - | // Output frames; with a terminal that is not draining, all of them may sit in | 115 | + | return newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return nil }, func(uint64) error { return nil }) |
| 115 | - | // this pipe at once, and overflow fails the whole stream | 116 | + | } |
| 116 | - | // (ErrStreamBackpressure). The frame size is a typical text repaint: a | 117 | + | |
| 117 | - | // near-full page of a 500x140 terminal measured about 70 KB. Uncompressed diff | 118 | + | // TestStreamWindowCoversTheSessionOutputWindow pins the sizing relation |
| 118 | - | // frames with per-cell truecolor SGR can be far larger (estimated near 1 MB at | 119 | + | // between the two flow-control layers: one broker stream window holds the |
| 119 | - | // 300x80), and eight of those can still overflow the default 4 MiB bound. | 120 | + | // daemon's unacknowledged byte budget, so the daemon is mostly paced by its own |
| 120 | - | func TestStreamPipeHoldsAFullOutputWindow(t *testing.T) { | 121 | + | // ACK window and coalesces frames. Credit may still pace it at the margin: the |
| 121 | - | const frameBytes = 72 << 10 | 122 | + | // receiver batches up to half a window before returning it, chunks cost 64 |
| 123 | + | // bytes of overhead, and one frame may overrun the byte budget (a big image). | |||
| 124 | + | // Credit stays the hard bound either way. | |||
| 125 | + | func TestStreamWindowCoversTheSessionOutputWindow(t *testing.T) { | |||
| 126 | + | require.GreaterOrEqual(t, brokerwire.StreamWindowBytes, uint64(protocol.MaxOutputWindowBytes), | |||
| 127 | + | "one stream window must hold the daemon's whole unacknowledged byte budget") | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | // TestStreamPipeCredit covers the receive and send sides of stream credit: | |||
| 131 | + | // a peer may send up to one window, the consumer's reads return credit in | |||
| 132 | + | // batches, a sender waits instead of failing while it has none, and grants or | |||
| 133 | + | // data beyond the window are flow-control violations. | |||
| 134 | + | func TestStreamPipeCredit(t *testing.T) { | |||
| 122 | chunk := int(brokerwire.MaxStreamChunkBytes) | 135 | chunk := int(brokerwire.MaxStreamChunkBytes) | ||
| 123 | - | p := newStreamPipe(0, 0, func([]byte) error { return nil }) | 136 | + | cost := brokerwire.StreamChunkCredit(chunk) |
| 124 | - | payload := bytes.Repeat([]byte{'x'}, frameBytes) | 137 | + | perWindow := int(brokerwire.StreamWindowBytes / cost) |
| 125 | - | for frame := range protocol.MaxOutputWindow { | 138 | + | |
| 126 | - | for data := payload; len(data) > 0; { | 139 | + | t.Run("a full window is accepted and one more frame is a violation", func(t *testing.T) { |
| 127 | - | n := min(len(data), chunk) | 140 | + | p := newTestPipe() |
| 128 | - | require.NoError(t, p.deliver(data[:n]), "frame %d overflowed the client pipe", frame) | 141 | + | for range perWindow { |
| 129 | - | data = data[n:] | 142 | + | require.NoError(t, p.deliver(make([]byte, chunk))) |
| 130 | } | 143 | } | ||
| 131 | - | } | 144 | + | require.ErrorIs(t, p.deliver(make([]byte, chunk)), ErrStreamCredit) |
| 145 | + | }) | |||
| 146 | + | ||||
| 147 | + | t.Run("reads return credit in batches", func(t *testing.T) { | |||
| 148 | + | var grants []uint64 | |||
| 149 | + | p := newStreamPipe(int(brokerwire.MaxStreamChunkBytes), func([]byte) error { return nil }, func(c uint64) error { grants = append(grants, c); return nil }) | |||
| 150 | + | for range perWindow { | |||
| 151 | + | require.NoError(t, p.deliver(make([]byte, chunk))) | |||
| 152 | + | } | |||
| 153 | + | buf := make([]byte, chunk) | |||
| 154 | + | for range perWindow { | |||
| 155 | + | _, err := io.ReadFull(p, buf) | |||
| 156 | + | require.NoError(t, err) | |||
| 157 | + | } | |||
| 158 | + | require.NotEmpty(t, grants) | |||
| 159 | + | total := uint64(0) | |||
| 160 | + | for _, g := range grants { | |||
| 161 | + | require.GreaterOrEqual(t, g, streamCreditReturnAt(brokerwire.StreamWindowBytes)) | |||
| 162 | + | total += g | |||
| 163 | + | } | |||
| 164 | + | require.LessOrEqual(t, total, uint64(perWindow)*cost) | |||
| 165 | + | // Once the returned credit reaches the peer, the window is open again. | |||
| 166 | + | require.NoError(t, p.deliver(make([]byte, chunk))) | |||
| 167 | + | }) | |||
| 168 | + | ||||
| 169 | + | t.Run("a sender waits for credit and resumes on a grant", func(t *testing.T) { | |||
| 170 | + | p := newTestPipe() | |||
| 171 | + | for range perWindow { | |||
| 172 | + | require.NoError(t, p.reserve(chunk)) | |||
| 173 | + | } | |||
| 174 | + | reserved := make(chan error, 1) | |||
| 175 | + | go func() { reserved <- p.reserve(chunk) }() | |||
| 176 | + | select { | |||
| 177 | + | case err := <-reserved: | |||
| 178 | + | t.Fatalf("reserve returned %v without credit", err) | |||
| 179 | + | case <-time.After(20 * time.Millisecond): | |||
| 180 | + | } | |||
| 181 | + | require.NoError(t, p.granted(cost)) | |||
| 182 | + | select { | |||
| 183 | + | case err := <-reserved: | |||
| 184 | + | require.NoError(t, err) | |||
| 185 | + | case <-time.After(5 * time.Second): | |||
| 186 | + | t.Fatal("a grant must release a sender waiting for credit") | |||
| 187 | + | } | |||
| 188 | + | }) | |||
| 189 | + | ||||
| 190 | + | t.Run("close releases a sender waiting for credit", func(t *testing.T) { | |||
| 191 | + | p := newTestPipe() | |||
| 192 | + | for range perWindow { | |||
| 193 | + | require.NoError(t, p.reserve(chunk)) | |||
| 194 | + | } | |||
| 195 | + | reserved := make(chan error, 1) | |||
| 196 | + | go func() { reserved <- p.reserve(chunk) }() | |||
| 197 | + | p.closeWith(ErrStreamGone) | |||
| 198 | + | select { | |||
| 199 | + | case err := <-reserved: | |||
| 200 | + | require.ErrorIs(t, err, ErrStreamGone) | |||
| 201 | + | case <-time.After(5 * time.Second): | |||
| 202 | + | t.Fatal("close must release a sender waiting for credit") | |||
| 203 | + | } | |||
| 204 | + | }) | |||
| 205 | + | ||||
| 206 | + | t.Run("a grant beyond the window is a violation", func(t *testing.T) { | |||
| 207 | + | p := newTestPipe() | |||
| 208 | + | require.ErrorIs(t, p.granted(1), ErrStreamCredit) | |||
| 209 | + | require.NoError(t, p.reserve(chunk)) | |||
| 210 | + | require.NoError(t, p.granted(cost)) | |||
| 211 | + | require.ErrorIs(t, p.granted(1), ErrStreamCredit) | |||
| 212 | + | }) | |||
| 132 | } | 213 | } | ||
| 133 | 214 | ||||
| 134 | // TestClientStreamPrefersTheFullOutputWindow pins what a real broker-routed | 215 | // TestClientStreamPrefersTheFullOutputWindow pins what a real broker-routed | ||
M internal/adapters/brokeripc/server.go +5 -0
| @@ -763,6 +763,11 @@ func (s *serverSession) dispatch(message brokerwire.ClientMessage) error { | |||
| 763 | 763 | } | |
| 764 | 764 | s.closeStreamByPeer(m.Stream) | |
| 765 | 765 | return nil | |
| 766 | + | case brokerwire.StreamWindowUpdate: | |
| 767 | + | if !s.scopeMatches(m.Epoch, m.Connection) { | |
| 768 | + | return s.refuseScope() | |
| 769 | + | } | |
| 770 | + | return s.grantStreamCredit(m) | |
| 766 | 771 | case brokerwire.StartPreview: | |
| 767 | 772 | if !s.scopeMatches(m.Epoch, m.Connection) { | |
| 768 | 773 | return s.refuseScope() | |
| @@ -763,6 +763,11 @@ func (s *serverSession) dispatch(message brokerwire.ClientMessage) error { | |||||
| 763 | } | 763 | } | ||
| 764 | s.closeStreamByPeer(m.Stream) | 764 | s.closeStreamByPeer(m.Stream) | ||
| 765 | return nil | 765 | return nil | ||
| 766 | + | case brokerwire.StreamWindowUpdate: | |||
| 767 | + | if !s.scopeMatches(m.Epoch, m.Connection) { | |||
| 768 | + | return s.refuseScope() | |||
| 769 | + | } | |||
| 770 | + | return s.grantStreamCredit(m) | |||
| 766 | case brokerwire.StartPreview: | 771 | case brokerwire.StartPreview: | ||
| 767 | if !s.scopeMatches(m.Epoch, m.Connection) { | 772 | if !s.scopeMatches(m.Epoch, m.Connection) { | ||
| 768 | return s.refuseScope() | 773 | return s.refuseScope() | ||
M internal/adapters/brokeripc/session_test.go +56 -22
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package brokeripc | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "context" | |
| 5 | 6 | "errors" | |
| 6 | 7 | "io" | |
| @@ -856,43 +857,76 @@ func TestStalledSubscriberDoesNotBlockOtherClients(t *testing.T) { | |||
| 856 | 857 | requireConnectionDelegates(t, active) | |
| 857 | 858 | } | |
| 858 | 859 | ||
| 859 | - | // TestClientStreamBackpressureSettlesOnlyThatStream proves a local consumer that | |
| 860 | - | // stops draining a stream's bounded inbound queue settles exactly that stream: | |
| 861 | - | // the peer is told, the stream reports backpressure, and the connection and its | |
| 862 | - | // operations stay up. | |
| 863 | - | func TestClientStreamBackpressureSettlesOnlyThatStream(t *testing.T) { | |
| 860 | + | // TestStalledClientStreamWaitsForCredit proves a local consumer that stops | |
| 861 | + | // reading slows the broker relay instead of losing its stream: the daemon's | |
| 862 | + | // messages beyond one stream window wait at the broker, every one is delivered | |
| 863 | + | // in order once the consumer reads again, and the connection stays usable | |
| 864 | + | // meanwhile. | |
| 865 | + | func TestStalledClientStreamWaitsForCredit(t *testing.T) { | |
| 864 | 866 | e := startEndpoint(t, Config{MaxClients: 2}) | |
| 865 | - | client := e.dialWith(Config{StreamInboundChunks: 1}) | |
| 867 | + | client := e.dial() | |
| 866 | 868 | _ = e.accept() | |
| 867 | 869 | core := e.authority.last() | |
| 868 | 870 | ||
| 869 | - | ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| 871 | + | ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | |
| 870 | 872 | defer cancel() | |
| 871 | 873 | stream, err := client.OpenStream(ctx, openRequest(nextStreamID(t, client))) | |
| 872 | 874 | require.NoError(t, err) | |
| 875 | + | t.Cleanup(func() { _ = stream.Close() }) | |
| 873 | 876 | conn := core.logicalConn(1) | |
| 874 | 877 | require.NotNil(t, conn) | |
| 878 | + | // Complete the session preamble before the daemon speaks. | |
| 879 | + | require.NoError(t, stream.SendClient(protocol.Ping{})) | |
| 880 | + | select { | |
| 881 | + | case <-conn.fromClient: | |
| 882 | + | case <-ctx.Done(): | |
| 883 | + | t.Fatal("client message never reached the broker core") | |
| 884 | + | } | |
| 875 | 885 | ||
| 876 | - | // Two daemon messages arrive while the local consumer never reads: the | |
| 877 | - | // bounded capacity-one queue refuses the second one. | |
| 878 | - | conn.toClient <- protocol.Pong{} | |
| 879 | - | conn.toClient <- protocol.Pong{} | |
| 886 | + | // Well over one stream window of daemon output while nothing reads. Side | |
| 887 | + | // effect Outputs need no view context and are never compressed. | |
| 888 | + | const messages = 24 | |
| 889 | + | data := bytes.Repeat([]byte{'x'}, 512<<10) | |
| 890 | + | go func() { | |
| 891 | + | for i := range messages { | |
| 892 | + | payload := append([]byte{byte(i)}, data...) | |
| 893 | + | select { | |
| 894 | + | case conn.toClient <- protocol.Output{Epoch: 1, Size: domain.Size{Cols: 80, Rows: 24}, Data: payload}: | |
| 895 | + | case <-ctx.Done(): | |
| 896 | + | return | |
| 897 | + | } | |
| 898 | + | } | |
| 899 | + | }() | |
| 880 | 900 | ||
| 901 | + | // The relay fills exactly one window of the client pipe and then waits on | |
| 902 | + | // credit instead of failing: the stream stays open and the connection keeps | |
| 903 | + | // serving other work. | |
| 904 | + | local, ok := stream.(*clientStream) | |
| 905 | + | require.True(t, ok) | |
| 906 | + | full := brokerwire.StreamWindowBytes - brokerwire.StreamChunkCredit(int(brokerwire.MaxStreamChunkBytes)) | |
| 907 | + | held := func() uint64 { | |
| 908 | + | local.pipe.mu.Lock() | |
| 909 | + | defer local.pipe.mu.Unlock() | |
| 910 | + | return local.pipe.recvUsed + local.pipe.recvReturn | |
| 911 | + | } | |
| 912 | + | require.Eventually(t, func() bool { return held() > full }, 5*time.Second, time.Millisecond, | |
| 913 | + | "the relay must fill one stream window") | |
| 914 | + | // Data beyond the credit would settle the stream with ErrStreamCredit. | |
| 881 | 915 | select { | |
| 882 | 916 | case <-stream.Done(): | |
| 883 | - | case <-time.After(5 * time.Second): | |
| 884 | - | t.Fatal("a stalled stream consumer must settle the stream") | |
| 917 | + | t.Fatalf("a stalled consumer must not settle its stream: %v", stream.Err()) | |
| 918 | + | default: | |
| 885 | 919 | } | |
| 886 | - | require.ErrorIs(t, stream.(*clientStream).Err(), ErrStreamBackpressure) | |
| 887 | - | ||
| 888 | - | require.Eventually(t, func() bool { | |
| 889 | - | core.mu.Lock() | |
| 890 | - | defer core.mu.Unlock() | |
| 891 | - | return len(core.closedIDs) > 0 | |
| 892 | - | }, 5*time.Second, 10*time.Millisecond, "the broker must be told to retire the stalled stream") | |
| 893 | - | ||
| 894 | - | // The connection survives a stream-local stall. | |
| 895 | 920 | requireConnectionDelegates(t, client) | |
| 921 | + | ||
| 922 | + | for i := range messages { | |
| 923 | + | message, err := stream.ReceiveServer() | |
| 924 | + | require.NoError(t, err) | |
| 925 | + | got, ok := message.(protocol.Output) | |
| 926 | + | require.True(t, ok, "message %d is %T", i, message) | |
| 927 | + | require.Len(t, got.Data, len(data)+1) | |
| 928 | + | require.Equal(t, byte(i), got.Data[0], "messages arrive in order") | |
| 929 | + | } | |
| 896 | 930 | } | |
| 897 | 931 | ||
| 898 | 932 | // TestOrderlyStreamCloseReportsNoError proves a local close is an orderly end: | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package brokeripc | 1 | package brokeripc | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "context" | 5 | "context" | ||
| 5 | "errors" | 6 | "errors" | ||
| 6 | "io" | 7 | "io" | ||
| @@ -856,43 +857,76 @@ func TestStalledSubscriberDoesNotBlockOtherClients(t *testing.T) { | |||||
| 856 | requireConnectionDelegates(t, active) | 857 | requireConnectionDelegates(t, active) | ||
| 857 | } | 858 | } | ||
| 858 | 859 | ||||
| 859 | - | // TestClientStreamBackpressureSettlesOnlyThatStream proves a local consumer that | 860 | + | // TestStalledClientStreamWaitsForCredit proves a local consumer that stops |
| 860 | - | // stops draining a stream's bounded inbound queue settles exactly that stream: | 861 | + | // reading slows the broker relay instead of losing its stream: the daemon's |
| 861 | - | // the peer is told, the stream reports backpressure, and the connection and its | 862 | + | // messages beyond one stream window wait at the broker, every one is delivered |
| 862 | - | // operations stay up. | 863 | + | // in order once the consumer reads again, and the connection stays usable |
| 863 | - | func TestClientStreamBackpressureSettlesOnlyThatStream(t *testing.T) { | 864 | + | // meanwhile. |
| 865 | + | func TestStalledClientStreamWaitsForCredit(t *testing.T) { | |||
| 864 | e := startEndpoint(t, Config{MaxClients: 2}) | 866 | e := startEndpoint(t, Config{MaxClients: 2}) | ||
| 865 | - | client := e.dialWith(Config{StreamInboundChunks: 1}) | 867 | + | client := e.dial() |
| 866 | _ = e.accept() | 868 | _ = e.accept() | ||
| 867 | core := e.authority.last() | 869 | core := e.authority.last() | ||
| 868 | 870 | ||||
| 869 | - | ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | 871 | + | ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) |
| 870 | defer cancel() | 872 | defer cancel() | ||
| 871 | stream, err := client.OpenStream(ctx, openRequest(nextStreamID(t, client))) | 873 | stream, err := client.OpenStream(ctx, openRequest(nextStreamID(t, client))) | ||
| 872 | require.NoError(t, err) | 874 | require.NoError(t, err) | ||
| 875 | + | t.Cleanup(func() { _ = stream.Close() }) | |||
| 873 | conn := core.logicalConn(1) | 876 | conn := core.logicalConn(1) | ||
| 874 | require.NotNil(t, conn) | 877 | require.NotNil(t, conn) | ||
| 878 | + | // Complete the session preamble before the daemon speaks. | |||
| 879 | + | require.NoError(t, stream.SendClient(protocol.Ping{})) | |||
| 880 | + | select { | |||
| 881 | + | case <-conn.fromClient: | |||
| 882 | + | case <-ctx.Done(): | |||
| 883 | + | t.Fatal("client message never reached the broker core") | |||
| 884 | + | } | |||
| 875 | 885 | ||||
| 876 | - | // Two daemon messages arrive while the local consumer never reads: the | 886 | + | // Well over one stream window of daemon output while nothing reads. Side |
| 877 | - | // bounded capacity-one queue refuses the second one. | 887 | + | // effect Outputs need no view context and are never compressed. |
| 878 | - | conn.toClient <- protocol.Pong{} | 888 | + | const messages = 24 |
| 879 | - | conn.toClient <- protocol.Pong{} | 889 | + | data := bytes.Repeat([]byte{'x'}, 512<<10) |
| 890 | + | go func() { | |||
| 891 | + | for i := range messages { | |||
| 892 | + | payload := append([]byte{byte(i)}, data...) | |||
| 893 | + | select { | |||
| 894 | + | case conn.toClient <- protocol.Output{Epoch: 1, Size: domain.Size{Cols: 80, Rows: 24}, Data: payload}: | |||
| 895 | + | case <-ctx.Done(): | |||
| 896 | + | return | |||
| 897 | + | } | |||
| 898 | + | } | |||
| 899 | + | }() | |||
| 880 | 900 | ||||
| 901 | + | // The relay fills exactly one window of the client pipe and then waits on | |||
| 902 | + | // credit instead of failing: the stream stays open and the connection keeps | |||
| 903 | + | // serving other work. | |||
| 904 | + | local, ok := stream.(*clientStream) | |||
| 905 | + | require.True(t, ok) | |||
| 906 | + | full := brokerwire.StreamWindowBytes - brokerwire.StreamChunkCredit(int(brokerwire.MaxStreamChunkBytes)) | |||
| 907 | + | held := func() uint64 { | |||
| 908 | + | local.pipe.mu.Lock() | |||
| 909 | + | defer local.pipe.mu.Unlock() | |||
| 910 | + | return local.pipe.recvUsed + local.pipe.recvReturn | |||
| 911 | + | } | |||
| 912 | + | require.Eventually(t, func() bool { return held() > full }, 5*time.Second, time.Millisecond, | |||
| 913 | + | "the relay must fill one stream window") | |||
| 914 | + | // Data beyond the credit would settle the stream with ErrStreamCredit. | |||
| 881 | select { | 915 | select { | ||
| 882 | case <-stream.Done(): | 916 | case <-stream.Done(): | ||
| 883 | - | case <-time.After(5 * time.Second): | 917 | + | t.Fatalf("a stalled consumer must not settle its stream: %v", stream.Err()) |
| 884 | - | t.Fatal("a stalled stream consumer must settle the stream") | 918 | + | default: |
| 885 | } | 919 | } | ||
| 886 | - | require.ErrorIs(t, stream.(*clientStream).Err(), ErrStreamBackpressure) | |||
| 887 | - | ||||
| 888 | - | require.Eventually(t, func() bool { | |||
| 889 | - | core.mu.Lock() | |||
| 890 | - | defer core.mu.Unlock() | |||
| 891 | - | return len(core.closedIDs) > 0 | |||
| 892 | - | }, 5*time.Second, 10*time.Millisecond, "the broker must be told to retire the stalled stream") | |||
| 893 | - | ||||
| 894 | - | // The connection survives a stream-local stall. | |||
| 895 | requireConnectionDelegates(t, client) | 920 | requireConnectionDelegates(t, client) | ||
| 921 | + | ||||
| 922 | + | for i := range messages { | |||
| 923 | + | message, err := stream.ReceiveServer() | |||
| 924 | + | require.NoError(t, err) | |||
| 925 | + | got, ok := message.(protocol.Output) | |||
| 926 | + | require.True(t, ok, "message %d is %T", i, message) | |||
| 927 | + | require.Len(t, got.Data, len(data)+1) | |||
| 928 | + | require.Equal(t, byte(i), got.Data[0], "messages arrive in order") | |||
| 929 | + | } | |||
| 896 | } | 930 | } | ||
| 897 | 931 | ||||
| 898 | // TestOrderlyStreamCloseReportsNoError proves a local close is an orderly end: | 932 | // TestOrderlyStreamCloseReportsNoError proves a local close is an orderly end: | ||
M internal/adapters/brokeripc/stream.go +67 -62
| @@ -42,28 +42,25 @@ func newServerStream(s *serverSession, id ports.BrokerStreamID, core ports.Broke | |||
| 42 | 42 | return nil, ErrConfig | |
| 43 | 43 | } | |
| 44 | 44 | st := &serverStream{session: s, id: id, core: core, done: make(chan struct{})} | |
| 45 | - | st.pipe = newStreamPipe(s.cfg.StreamInboundChunks, s.cfg.StreamInboundBytes, st.sendChunk) | |
| 45 | + | st.pipe = newStreamPipe(int(s.ceilings.StreamChunkLimit), st.sendFrame, st.grantCredit) | |
| 46 | 46 | st.carriage = newCarriage(st.pipe) | |
| 47 | 47 | return st, nil | |
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | - | // sendChunk splits one outbound byte slice under the negotiated stream chunk | |
| 51 | - | // ceiling and writes each frame in order. | |
| 52 | - | func (st *serverStream) sendChunk(data []byte) error { | |
| 53 | - | ceiling := int(st.session.ceilings.StreamChunkLimit) | |
| 54 | - | if ceiling <= 0 { | |
| 55 | - | return ErrConfig | |
| 56 | - | } | |
| 57 | - | for len(data) > 0 { | |
| 58 | - | n := min(len(data), ceiling) | |
| 59 | - | if err := st.session.send(brokerwire.ServerStreamData{ | |
| 60 | - | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Data: data[:n], | |
| 61 | - | }); err != nil { | |
| 62 | - | return err | |
| 63 | - | } | |
| 64 | - | data = data[n:] | |
| 65 | - | } | |
| 66 | - | return nil | |
| 50 | + | // sendFrame writes one credited outbound frame to the client. A client that | |
| 51 | + | // is not reading slows this relay, and through the daemon link the daemon, | |
| 52 | + | // instead of losing its stream. | |
| 53 | + | func (st *serverStream) sendFrame(data []byte) error { | |
| 54 | + | return st.session.send(brokerwire.ServerStreamData{ | |
| 55 | + | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Data: data, | |
| 56 | + | }) | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | // grantCredit returns consumed inbound credit to the client. | |
| 60 | + | func (st *serverStream) grantCredit(credit uint64) error { | |
| 61 | + | return st.session.send(brokerwire.StreamWindowUpdate{ | |
| 62 | + | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Credit: credit, | |
| 63 | + | }) | |
| 67 | 64 | } | |
| 68 | 65 | ||
| 69 | 66 | // deliver accepts one inbound chunk from the connection reader. | |
| @@ -347,40 +344,51 @@ func (s *serverSession) closeStreamByPeer(id ports.BrokerStreamID) { | |||
| 347 | 344 | _ = s.core.CloseStream(s.scope.Connection, id) | |
| 348 | 345 | } | |
| 349 | 346 | ||
| 350 | - | // deliverStreamData routes one inbound client chunk to its bridge. Backpressure | |
| 351 | - | // settles only that stream; a chunk for an identity the peer never opened is a | |
| 352 | - | // protocol violation. | |
| 347 | + | // deliverStreamData routes one inbound client chunk to its bridge. Data beyond | |
| 348 | + | // the granted credit settles only that stream; a chunk for an identity the peer | |
| 349 | + | // never opened is a protocol violation. | |
| 353 | 350 | func (s *serverSession) deliverStreamData(m brokerwire.ClientStreamData) error { | |
| 354 | - | disposition, err := s.conn.StreamData(m.Stream) | |
| 355 | - | if err != nil { | |
| 356 | - | // Stream data for a stream that was never allocated, or before it was | |
| 357 | - | // confirmed open, is a peer protocol violation. | |
| 358 | - | return errors.Join(ErrProtocol, err) | |
| 351 | + | st, err := s.streamFor(m.Stream) | |
| 352 | + | if st == nil || err != nil { | |
| 353 | + | return err | |
| 359 | 354 | } | |
| 360 | - | if disposition == brokerwire.StreamDiscarded { | |
| 361 | - | return nil | |
| 355 | + | if err := st.deliver(m.Data); errors.Is(err, ErrStreamCredit) { | |
| 356 | + | s.settleStream(st, ErrStreamCredit) | |
| 362 | 357 | } | |
| 363 | - | st := s.lookupStream(m.Stream) | |
| 364 | - | if st == nil { | |
| 365 | - | // The bridge was settled between the tracker check and the lookup (a | |
| 366 | - | // stream-local failure won the race); the chunk is discarded with it. | |
| 367 | - | return nil | |
| 358 | + | // Any other error means the stream already reached its terminal outcome; | |
| 359 | + | // its data is discarded. | |
| 360 | + | return nil | |
| 361 | + | } | |
| 362 | + | ||
| 363 | + | // grantStreamCredit applies one client credit grant to its bridge. | |
| 364 | + | func (s *serverSession) grantStreamCredit(m brokerwire.StreamWindowUpdate) error { | |
| 365 | + | st, err := s.streamFor(m.Stream) | |
| 366 | + | if st == nil || err != nil { | |
| 367 | + | return err | |
| 368 | 368 | } | |
| 369 | - | switch err := st.deliver(m.Data); { | |
| 370 | - | case err == nil: | |
| 371 | - | return nil | |
| 372 | - | case errors.Is(err, ErrStreamBackpressure): | |
| 373 | - | s.settleStream(st, ErrStreamBackpressure) | |
| 374 | - | return nil | |
| 375 | - | default: | |
| 376 | - | // The stream already reached its terminal outcome; its data is | |
| 377 | - | // discarded. | |
| 378 | - | return nil | |
| 369 | + | if err := st.pipe.granted(m.Credit); err != nil { | |
| 370 | + | s.settleStream(st, err) | |
| 379 | 371 | } | |
| 372 | + | return nil | |
| 373 | + | } | |
| 374 | + | ||
| 375 | + | // streamFor classifies one stream-scoped frame and returns its live bridge. A | |
| 376 | + | // frame for a retired stream, or one settled between the tracker check and the | |
| 377 | + | // lookup, yields no bridge and no error; a frame for a stream that was never | |
| 378 | + | // allocated, or before it was confirmed open, is a peer protocol violation. | |
| 379 | + | func (s *serverSession) streamFor(id ports.BrokerStreamID) (*serverStream, error) { | |
| 380 | + | disposition, err := s.conn.StreamData(id) | |
| 381 | + | if err != nil { | |
| 382 | + | return nil, errors.Join(ErrProtocol, err) | |
| 383 | + | } | |
| 384 | + | if disposition == brokerwire.StreamDiscarded { | |
| 385 | + | return nil, nil | |
| 386 | + | } | |
| 387 | + | return s.lookupStream(id), nil | |
| 380 | 388 | } | |
| 381 | 389 | ||
| 382 | 390 | // clientStream is the client-side half of one logical stream: the sessionwire | |
| 383 | - | // client connection over the stream's bounded pipe, plus the terminal outcome a | |
| 391 | + | // client connection over the stream's credit-controlled pipe, plus the terminal outcome a | |
| 384 | 392 | // ports.BrokerLogicalConnection exposes independently of reads. | |
| 385 | 393 | type clientStream struct { | |
| 386 | 394 | ports.ClientConnection | |
| @@ -407,7 +415,7 @@ func newClientStream(c *client, id ports.BrokerStreamID) (*clientStream, error) | |||
| 407 | 415 | opened: make(chan error, 1), | |
| 408 | 416 | done: make(chan struct{}), | |
| 409 | 417 | } | |
| 410 | - | st.pipe = newStreamPipe(c.cfg.StreamInboundChunks, c.cfg.StreamInboundBytes, st.sendChunk) | |
| 418 | + | st.pipe = newStreamPipe(int(c.ceilings.StreamChunkLimit), st.sendFrame, st.grantCredit) | |
| 411 | 419 | st.carriage = newCarriage(st.pipe) | |
| 412 | 420 | st.ClientConnection = sessionwire.NewClientConnection(st.carriage) | |
| 413 | 421 | if st.ClientConnection == nil { | |
| @@ -416,23 +424,20 @@ func newClientStream(c *client, id ports.BrokerStreamID) (*clientStream, error) | |||
| 416 | 424 | return st, nil | |
| 417 | 425 | } | |
| 418 | 426 | ||
| 419 | - | // sendChunk splits one outbound client byte slice under the negotiated stream | |
| 420 | - | // chunk ceiling. | |
| 421 | - | func (st *clientStream) sendChunk(data []byte) error { | |
| 422 | - | ceiling := int(st.client.ceilings.StreamChunkLimit) | |
| 423 | - | if ceiling <= 0 { | |
| 424 | - | return ErrConfig | |
| 425 | - | } | |
| 426 | - | for len(data) > 0 { | |
| 427 | - | n := min(len(data), ceiling) | |
| 428 | - | if err := st.client.send(brokerwire.ClientStreamData{ | |
| 429 | - | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Data: data[:n], | |
| 430 | - | }); err != nil { | |
| 431 | - | return err | |
| 432 | - | } | |
| 433 | - | data = data[n:] | |
| 434 | - | } | |
| 435 | - | return nil | |
| 427 | + | // sendFrame writes one credited outbound frame to the broker. | |
| 428 | + | func (st *clientStream) sendFrame(data []byte) error { | |
| 429 | + | return st.client.send(brokerwire.ClientStreamData{ | |
| 430 | + | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Data: data, | |
| 431 | + | }) | |
| 432 | + | } | |
| 433 | + | ||
| 434 | + | // grantCredit returns consumed inbound credit to the broker. It is written | |
| 435 | + | // synchronously because a dropped grant would stall the stream forever; the | |
| 436 | + | // write never waits long, since the broker's reader never blocks on a stream. | |
| 437 | + | func (st *clientStream) grantCredit(credit uint64) error { | |
| 438 | + | return st.client.send(brokerwire.StreamWindowUpdate{ | |
| 439 | + | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Credit: credit, | |
| 440 | + | }) | |
| 436 | 441 | } | |
| 437 | 442 | ||
| 438 | 443 | // deliver accepts one inbound server chunk. | |
| @@ -42,28 +42,25 @@ func newServerStream(s *serverSession, id ports.BrokerStreamID, core ports.Broke | |||||
| 42 | return nil, ErrConfig | 42 | return nil, ErrConfig | ||
| 43 | } | 43 | } | ||
| 44 | st := &serverStream{session: s, id: id, core: core, done: make(chan struct{})} | 44 | st := &serverStream{session: s, id: id, core: core, done: make(chan struct{})} | ||
| 45 | - | st.pipe = newStreamPipe(s.cfg.StreamInboundChunks, s.cfg.StreamInboundBytes, st.sendChunk) | 45 | + | st.pipe = newStreamPipe(int(s.ceilings.StreamChunkLimit), st.sendFrame, st.grantCredit) |
| 46 | st.carriage = newCarriage(st.pipe) | 46 | st.carriage = newCarriage(st.pipe) | ||
| 47 | return st, nil | 47 | return st, nil | ||
| 48 | } | 48 | } | ||
| 49 | 49 | ||||
| 50 | - | // sendChunk splits one outbound byte slice under the negotiated stream chunk | 50 | + | // sendFrame writes one credited outbound frame to the client. A client that |
| 51 | - | // ceiling and writes each frame in order. | 51 | + | // is not reading slows this relay, and through the daemon link the daemon, |
| 52 | - | func (st *serverStream) sendChunk(data []byte) error { | 52 | + | // instead of losing its stream. |
| 53 | - | ceiling := int(st.session.ceilings.StreamChunkLimit) | 53 | + | func (st *serverStream) sendFrame(data []byte) error { |
| 54 | - | if ceiling <= 0 { | 54 | + | return st.session.send(brokerwire.ServerStreamData{ |
| 55 | - | return ErrConfig | 55 | + | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Data: data, |
| 56 | - | } | 56 | + | }) |
| 57 | - | for len(data) > 0 { | 57 | + | } |
| 58 | - | n := min(len(data), ceiling) | 58 | + | |
| 59 | - | if err := st.session.send(brokerwire.ServerStreamData{ | 59 | + | // grantCredit returns consumed inbound credit to the client. |
| 60 | - | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Data: data[:n], | 60 | + | func (st *serverStream) grantCredit(credit uint64) error { |
| 61 | - | }); err != nil { | 61 | + | return st.session.send(brokerwire.StreamWindowUpdate{ |
| 62 | - | return err | 62 | + | Epoch: st.session.epoch, Connection: st.session.scope.Connection, Stream: st.id, Credit: credit, |
| 63 | - | } | 63 | + | }) |
| 64 | - | data = data[n:] | |||
| 65 | - | } | |||
| 66 | - | return nil | |||
| 67 | } | 64 | } | ||
| 68 | 65 | ||||
| 69 | // deliver accepts one inbound chunk from the connection reader. | 66 | // deliver accepts one inbound chunk from the connection reader. | ||
| @@ -347,40 +344,51 @@ func (s *serverSession) closeStreamByPeer(id ports.BrokerStreamID) { | |||||
| 347 | _ = s.core.CloseStream(s.scope.Connection, id) | 344 | _ = s.core.CloseStream(s.scope.Connection, id) | ||
| 348 | } | 345 | } | ||
| 349 | 346 | ||||
| 350 | - | // deliverStreamData routes one inbound client chunk to its bridge. Backpressure | 347 | + | // deliverStreamData routes one inbound client chunk to its bridge. Data beyond |
| 351 | - | // settles only that stream; a chunk for an identity the peer never opened is a | 348 | + | // the granted credit settles only that stream; a chunk for an identity the peer |
| 352 | - | // protocol violation. | 349 | + | // never opened is a protocol violation. |
| 353 | func (s *serverSession) deliverStreamData(m brokerwire.ClientStreamData) error { | 350 | func (s *serverSession) deliverStreamData(m brokerwire.ClientStreamData) error { | ||
| 354 | - | disposition, err := s.conn.StreamData(m.Stream) | 351 | + | st, err := s.streamFor(m.Stream) |
| 355 | - | if err != nil { | 352 | + | if st == nil || err != nil { |
| 356 | - | // Stream data for a stream that was never allocated, or before it was | 353 | + | return err |
| 357 | - | // confirmed open, is a peer protocol violation. | |||
| 358 | - | return errors.Join(ErrProtocol, err) | |||
| 359 | } | 354 | } | ||
| 360 | - | if disposition == brokerwire.StreamDiscarded { | 355 | + | if err := st.deliver(m.Data); errors.Is(err, ErrStreamCredit) { |
| 361 | - | return nil | 356 | + | s.settleStream(st, ErrStreamCredit) |
| 362 | } | 357 | } | ||
| 363 | - | st := s.lookupStream(m.Stream) | 358 | + | // Any other error means the stream already reached its terminal outcome; |
| 364 | - | if st == nil { | 359 | + | // its data is discarded. |
| 365 | - | // The bridge was settled between the tracker check and the lookup (a | 360 | + | return nil |
| 366 | - | // stream-local failure won the race); the chunk is discarded with it. | 361 | + | } |
| 367 | - | return nil | 362 | + | |
| 363 | + | // grantStreamCredit applies one client credit grant to its bridge. | |||
| 364 | + | func (s *serverSession) grantStreamCredit(m brokerwire.StreamWindowUpdate) error { | |||
| 365 | + | st, err := s.streamFor(m.Stream) | |||
| 366 | + | if st == nil || err != nil { | |||
| 367 | + | return err | |||
| 368 | } | 368 | } | ||
| 369 | - | switch err := st.deliver(m.Data); { | 369 | + | if err := st.pipe.granted(m.Credit); err != nil { |
| 370 | - | case err == nil: | 370 | + | s.settleStream(st, err) |
| 371 | - | return nil | |||
| 372 | - | case errors.Is(err, ErrStreamBackpressure): | |||
| 373 | - | s.settleStream(st, ErrStreamBackpressure) | |||
| 374 | - | return nil | |||
| 375 | - | default: | |||
| 376 | - | // The stream already reached its terminal outcome; its data is | |||
| 377 | - | // discarded. | |||
| 378 | - | return nil | |||
| 379 | } | 371 | } | ||
| 372 | + | return nil | |||
| 373 | + | } | |||
| 374 | + | ||||
| 375 | + | // streamFor classifies one stream-scoped frame and returns its live bridge. A | |||
| 376 | + | // frame for a retired stream, or one settled between the tracker check and the | |||
| 377 | + | // lookup, yields no bridge and no error; a frame for a stream that was never | |||
| 378 | + | // allocated, or before it was confirmed open, is a peer protocol violation. | |||
| 379 | + | func (s *serverSession) streamFor(id ports.BrokerStreamID) (*serverStream, error) { | |||
| 380 | + | disposition, err := s.conn.StreamData(id) | |||
| 381 | + | if err != nil { | |||
| 382 | + | return nil, errors.Join(ErrProtocol, err) | |||
| 383 | + | } | |||
| 384 | + | if disposition == brokerwire.StreamDiscarded { | |||
| 385 | + | return nil, nil | |||
| 386 | + | } | |||
| 387 | + | return s.lookupStream(id), nil | |||
| 380 | } | 388 | } | ||
| 381 | 389 | ||||
| 382 | // clientStream is the client-side half of one logical stream: the sessionwire | 390 | // clientStream is the client-side half of one logical stream: the sessionwire | ||
| 383 | - | // client connection over the stream's bounded pipe, plus the terminal outcome a | 391 | + | // client connection over the stream's credit-controlled pipe, plus the terminal outcome a |
| 384 | // ports.BrokerLogicalConnection exposes independently of reads. | 392 | // ports.BrokerLogicalConnection exposes independently of reads. | ||
| 385 | type clientStream struct { | 393 | type clientStream struct { | ||
| 386 | ports.ClientConnection | 394 | ports.ClientConnection | ||
| @@ -407,7 +415,7 @@ func newClientStream(c *client, id ports.BrokerStreamID) (*clientStream, error) | |||||
| 407 | opened: make(chan error, 1), | 415 | opened: make(chan error, 1), | ||
| 408 | done: make(chan struct{}), | 416 | done: make(chan struct{}), | ||
| 409 | } | 417 | } | ||
| 410 | - | st.pipe = newStreamPipe(c.cfg.StreamInboundChunks, c.cfg.StreamInboundBytes, st.sendChunk) | 418 | + | st.pipe = newStreamPipe(int(c.ceilings.StreamChunkLimit), st.sendFrame, st.grantCredit) |
| 411 | st.carriage = newCarriage(st.pipe) | 419 | st.carriage = newCarriage(st.pipe) | ||
| 412 | st.ClientConnection = sessionwire.NewClientConnection(st.carriage) | 420 | st.ClientConnection = sessionwire.NewClientConnection(st.carriage) | ||
| 413 | if st.ClientConnection == nil { | 421 | if st.ClientConnection == nil { | ||
| @@ -416,23 +424,20 @@ func newClientStream(c *client, id ports.BrokerStreamID) (*clientStream, error) | |||||
| 416 | return st, nil | 424 | return st, nil | ||
| 417 | } | 425 | } | ||
| 418 | 426 | ||||
| 419 | - | // sendChunk splits one outbound client byte slice under the negotiated stream | 427 | + | // sendFrame writes one credited outbound frame to the broker. |
| 420 | - | // chunk ceiling. | 428 | + | func (st *clientStream) sendFrame(data []byte) error { |
| 421 | - | func (st *clientStream) sendChunk(data []byte) error { | 429 | + | return st.client.send(brokerwire.ClientStreamData{ |
| 422 | - | ceiling := int(st.client.ceilings.StreamChunkLimit) | 430 | + | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Data: data, |
| 423 | - | if ceiling <= 0 { | 431 | + | }) |
| 424 | - | return ErrConfig | 432 | + | } |
| 425 | - | } | 433 | + | |
| 426 | - | for len(data) > 0 { | 434 | + | // grantCredit returns consumed inbound credit to the broker. It is written |
| 427 | - | n := min(len(data), ceiling) | 435 | + | // synchronously because a dropped grant would stall the stream forever; the |
| 428 | - | if err := st.client.send(brokerwire.ClientStreamData{ | 436 | + | // write never waits long, since the broker's reader never blocks on a stream. |
| 429 | - | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Data: data[:n], | 437 | + | func (st *clientStream) grantCredit(credit uint64) error { |
| 430 | - | }); err != nil { | 438 | + | return st.client.send(brokerwire.StreamWindowUpdate{ |
| 431 | - | return err | 439 | + | Epoch: st.client.scope.Epoch, Connection: st.client.scope.Connection, Stream: st.id, Credit: credit, |
| 432 | - | } | 440 | + | }) |
| 433 | - | data = data[n:] | |||
| 434 | - | } | |||
| 435 | - | return nil | |||
| 436 | } | 441 | } | ||
| 437 | 442 | ||||
| 438 | // deliver accepts one inbound server chunk. | 443 | // deliver accepts one inbound server chunk. | ||
M internal/adapters/brokerwire/codec.go +59 -2
| @@ -241,6 +241,17 @@ func encodeClientEnvelope(message ClientMessage, maxChunkBytes uint64) (*wire.Br | |||
| 241 | 241 | return nil, ErrInvalidMessage | |
| 242 | 242 | } | |
| 243 | 243 | return encodeClientEnvelope(*m, maxChunkBytes) | |
| 244 | + | case StreamWindowUpdate: | |
| 245 | + | converted, err := streamWindowUpdateToWire(m) | |
| 246 | + | if err != nil { | |
| 247 | + | return nil, err | |
| 248 | + | } | |
| 249 | + | return &wire.BrokerClientEnvelope{Payload: &wire.BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: converted}}, nil | |
| 250 | + | case *StreamWindowUpdate: | |
| 251 | + | if m == nil { | |
| 252 | + | return nil, ErrInvalidMessage | |
| 253 | + | } | |
| 254 | + | return encodeClientEnvelope(*m, maxChunkBytes) | |
| 244 | 255 | case StartPreview: | |
| 245 | 256 | converted, err := startPreviewToWire(m) | |
| 246 | 257 | if err != nil { | |
| @@ -308,6 +319,8 @@ func decodeClientEnvelope(envelope *wire.BrokerClientEnvelope, maxChunkBytes uin | |||
| 308 | 319 | return nil, ErrInvalidMessage | |
| 309 | 320 | } | |
| 310 | 321 | return CloseStream{Epoch: epoch, Connection: connection, Stream: stream}, nil | |
| 322 | + | case *wire.BrokerClientEnvelope_StreamWindowUpdate: | |
| 323 | + | return streamWindowUpdateFromWire(payload.StreamWindowUpdate) | |
| 311 | 324 | case *wire.BrokerClientEnvelope_StartPreview: | |
| 312 | 325 | return startPreviewFromWire(payload.StartPreview) | |
| 313 | 326 | case *wire.BrokerClientEnvelope_CancelPreview: | |
| @@ -707,6 +720,33 @@ func openStreamFromWire(message *wire.OpenStream) (OpenStream, error) { | |||
| 707 | 720 | return candidate, nil | |
| 708 | 721 | } | |
| 709 | 722 | ||
| 723 | + | // streamWindowUpdateToWire refuses a zero or above-window grant: credit is | |
| 724 | + | // returned only for consumed data, so a valid grant never exceeds one window. | |
| 725 | + | func streamWindowUpdateToWire(m StreamWindowUpdate) (*wire.StreamWindowUpdate, error) { | |
| 726 | + | if m.Epoch == 0 || m.Connection.Validate() != nil || m.Stream.Validate() != nil { | |
| 727 | + | return nil, ErrInvalidMessage | |
| 728 | + | } | |
| 729 | + | if m.Credit == 0 || m.Credit > StreamWindowBytes { | |
| 730 | + | return nil, ErrInvalidMessage | |
| 731 | + | } | |
| 732 | + | return &wire.StreamWindowUpdate{Ref: refToWire(m.Epoch, m.Connection, m.Stream), Credit: m.Credit}, nil | |
| 733 | + | } | |
| 734 | + | ||
| 735 | + | func streamWindowUpdateFromWire(message *wire.StreamWindowUpdate) (StreamWindowUpdate, error) { | |
| 736 | + | if message == nil { | |
| 737 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |
| 738 | + | } | |
| 739 | + | epoch, connection, stream, err := refFromWire(message.GetRef()) | |
| 740 | + | if err != nil { | |
| 741 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |
| 742 | + | } | |
| 743 | + | credit := message.GetCredit() | |
| 744 | + | if credit == 0 || credit > StreamWindowBytes { | |
| 745 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |
| 746 | + | } | |
| 747 | + | return StreamWindowUpdate{Epoch: epoch, Connection: connection, Stream: stream, Credit: credit}, nil | |
| 748 | + | } | |
| 749 | + | ||
| 710 | 750 | func clientStreamDataToWire(m ClientStreamData, maxChunkBytes uint64) (*wire.ClientStreamData, error) { | |
| 711 | 751 | if m.Epoch == 0 { | |
| 712 | 752 | return nil, ErrInvalidMessage | |
| @@ -751,7 +791,9 @@ func clientStreamDataFromWire(message *wire.ClientStreamData, maxChunkBytes uint | |||
| 751 | 791 | if len(data) == 0 { | |
| 752 | 792 | return ClientStreamData{}, ErrInvalidMessage | |
| 753 | 793 | } | |
| 754 | - | return ClientStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: append([]byte(nil), data...)}, nil | |
| 794 | + | // proto.Unmarshal copies bytes fields out of the payload, so the caller | |
| 795 | + | // owns data without another copy. | |
| 796 | + | return ClientStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: data}, nil | |
| 755 | 797 | } | |
| 756 | 798 | ||
| 757 | 799 | func encodeServerEnvelope(message ServerMessage, maxChunkBytes uint64) (*wire.BrokerServerEnvelope, error) { | |
| @@ -828,6 +870,17 @@ func encodeServerEnvelope(message ServerMessage, maxChunkBytes uint64) (*wire.Br | |||
| 828 | 870 | return nil, ErrInvalidMessage | |
| 829 | 871 | } | |
| 830 | 872 | return encodeServerEnvelope(*m, maxChunkBytes) | |
| 873 | + | case StreamWindowUpdate: | |
| 874 | + | converted, err := streamWindowUpdateToWire(m) | |
| 875 | + | if err != nil { | |
| 876 | + | return nil, err | |
| 877 | + | } | |
| 878 | + | return &wire.BrokerServerEnvelope{Payload: &wire.BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: converted}}, nil | |
| 879 | + | case *StreamWindowUpdate: | |
| 880 | + | if m == nil { | |
| 881 | + | return nil, ErrInvalidMessage | |
| 882 | + | } | |
| 883 | + | return encodeServerEnvelope(*m, maxChunkBytes) | |
| 831 | 884 | case StreamClosed: | |
| 832 | 885 | converted, err := streamClosedToWire(m) | |
| 833 | 886 | if err != nil { | |
| @@ -929,6 +982,8 @@ func decodeServerEnvelope(envelope *wire.BrokerServerEnvelope, maxChunkBytes uin | |||
| 929 | 982 | return serverStreamDataFromWire(payload.ServerStreamData, maxChunkBytes) | |
| 930 | 983 | case *wire.BrokerServerEnvelope_StreamClosed: | |
| 931 | 984 | return streamClosedFromWire(payload.StreamClosed) | |
| 985 | + | case *wire.BrokerServerEnvelope_StreamWindowUpdate: | |
| 986 | + | return streamWindowUpdateFromWire(payload.StreamWindowUpdate) | |
| 932 | 987 | case *wire.BrokerServerEnvelope_Progress: | |
| 933 | 988 | return progressFromWire(payload.Progress) | |
| 934 | 989 | case *wire.BrokerServerEnvelope_BrokerErrorMessage: | |
| @@ -1492,7 +1547,9 @@ func serverStreamDataFromWire(message *wire.ServerStreamData, maxChunkBytes uint | |||
| 1492 | 1547 | if len(data) == 0 { | |
| 1493 | 1548 | return ServerStreamData{}, ErrInvalidMessage | |
| 1494 | 1549 | } | |
| 1495 | - | return ServerStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: append([]byte(nil), data...)}, nil | |
| 1550 | + | // proto.Unmarshal copies bytes fields out of the payload, so the caller | |
| 1551 | + | // owns data without another copy. | |
| 1552 | + | return ServerStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: data}, nil | |
| 1496 | 1553 | } | |
| 1497 | 1554 | ||
| 1498 | 1555 | func streamClosedToWire(m StreamClosed) (*wire.StreamClosed, error) { | |
| @@ -241,6 +241,17 @@ func encodeClientEnvelope(message ClientMessage, maxChunkBytes uint64) (*wire.Br | |||||
| 241 | return nil, ErrInvalidMessage | 241 | return nil, ErrInvalidMessage | ||
| 242 | } | 242 | } | ||
| 243 | return encodeClientEnvelope(*m, maxChunkBytes) | 243 | return encodeClientEnvelope(*m, maxChunkBytes) | ||
| 244 | + | case StreamWindowUpdate: | |||
| 245 | + | converted, err := streamWindowUpdateToWire(m) | |||
| 246 | + | if err != nil { | |||
| 247 | + | return nil, err | |||
| 248 | + | } | |||
| 249 | + | return &wire.BrokerClientEnvelope{Payload: &wire.BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: converted}}, nil | |||
| 250 | + | case *StreamWindowUpdate: | |||
| 251 | + | if m == nil { | |||
| 252 | + | return nil, ErrInvalidMessage | |||
| 253 | + | } | |||
| 254 | + | return encodeClientEnvelope(*m, maxChunkBytes) | |||
| 244 | case StartPreview: | 255 | case StartPreview: | ||
| 245 | converted, err := startPreviewToWire(m) | 256 | converted, err := startPreviewToWire(m) | ||
| 246 | if err != nil { | 257 | if err != nil { | ||
| @@ -308,6 +319,8 @@ func decodeClientEnvelope(envelope *wire.BrokerClientEnvelope, maxChunkBytes uin | |||||
| 308 | return nil, ErrInvalidMessage | 319 | return nil, ErrInvalidMessage | ||
| 309 | } | 320 | } | ||
| 310 | return CloseStream{Epoch: epoch, Connection: connection, Stream: stream}, nil | 321 | return CloseStream{Epoch: epoch, Connection: connection, Stream: stream}, nil | ||
| 322 | + | case *wire.BrokerClientEnvelope_StreamWindowUpdate: | |||
| 323 | + | return streamWindowUpdateFromWire(payload.StreamWindowUpdate) | |||
| 311 | case *wire.BrokerClientEnvelope_StartPreview: | 324 | case *wire.BrokerClientEnvelope_StartPreview: | ||
| 312 | return startPreviewFromWire(payload.StartPreview) | 325 | return startPreviewFromWire(payload.StartPreview) | ||
| 313 | case *wire.BrokerClientEnvelope_CancelPreview: | 326 | case *wire.BrokerClientEnvelope_CancelPreview: | ||
| @@ -707,6 +720,33 @@ func openStreamFromWire(message *wire.OpenStream) (OpenStream, error) { | |||||
| 707 | return candidate, nil | 720 | return candidate, nil | ||
| 708 | } | 721 | } | ||
| 709 | 722 | ||||
| 723 | + | // streamWindowUpdateToWire refuses a zero or above-window grant: credit is | |||
| 724 | + | // returned only for consumed data, so a valid grant never exceeds one window. | |||
| 725 | + | func streamWindowUpdateToWire(m StreamWindowUpdate) (*wire.StreamWindowUpdate, error) { | |||
| 726 | + | if m.Epoch == 0 || m.Connection.Validate() != nil || m.Stream.Validate() != nil { | |||
| 727 | + | return nil, ErrInvalidMessage | |||
| 728 | + | } | |||
| 729 | + | if m.Credit == 0 || m.Credit > StreamWindowBytes { | |||
| 730 | + | return nil, ErrInvalidMessage | |||
| 731 | + | } | |||
| 732 | + | return &wire.StreamWindowUpdate{Ref: refToWire(m.Epoch, m.Connection, m.Stream), Credit: m.Credit}, nil | |||
| 733 | + | } | |||
| 734 | + | ||||
| 735 | + | func streamWindowUpdateFromWire(message *wire.StreamWindowUpdate) (StreamWindowUpdate, error) { | |||
| 736 | + | if message == nil { | |||
| 737 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |||
| 738 | + | } | |||
| 739 | + | epoch, connection, stream, err := refFromWire(message.GetRef()) | |||
| 740 | + | if err != nil { | |||
| 741 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |||
| 742 | + | } | |||
| 743 | + | credit := message.GetCredit() | |||
| 744 | + | if credit == 0 || credit > StreamWindowBytes { | |||
| 745 | + | return StreamWindowUpdate{}, ErrInvalidMessage | |||
| 746 | + | } | |||
| 747 | + | return StreamWindowUpdate{Epoch: epoch, Connection: connection, Stream: stream, Credit: credit}, nil | |||
| 748 | + | } | |||
| 749 | + | ||||
| 710 | func clientStreamDataToWire(m ClientStreamData, maxChunkBytes uint64) (*wire.ClientStreamData, error) { | 750 | func clientStreamDataToWire(m ClientStreamData, maxChunkBytes uint64) (*wire.ClientStreamData, error) { | ||
| 711 | if m.Epoch == 0 { | 751 | if m.Epoch == 0 { | ||
| 712 | return nil, ErrInvalidMessage | 752 | return nil, ErrInvalidMessage | ||
| @@ -751,7 +791,9 @@ func clientStreamDataFromWire(message *wire.ClientStreamData, maxChunkBytes uint | |||||
| 751 | if len(data) == 0 { | 791 | if len(data) == 0 { | ||
| 752 | return ClientStreamData{}, ErrInvalidMessage | 792 | return ClientStreamData{}, ErrInvalidMessage | ||
| 753 | } | 793 | } | ||
| 754 | - | return ClientStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: append([]byte(nil), data...)}, nil | 794 | + | // proto.Unmarshal copies bytes fields out of the payload, so the caller |
| 795 | + | // owns data without another copy. | |||
| 796 | + | return ClientStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: data}, nil | |||
| 755 | } | 797 | } | ||
| 756 | 798 | ||||
| 757 | func encodeServerEnvelope(message ServerMessage, maxChunkBytes uint64) (*wire.BrokerServerEnvelope, error) { | 799 | func encodeServerEnvelope(message ServerMessage, maxChunkBytes uint64) (*wire.BrokerServerEnvelope, error) { | ||
| @@ -828,6 +870,17 @@ func encodeServerEnvelope(message ServerMessage, maxChunkBytes uint64) (*wire.Br | |||||
| 828 | return nil, ErrInvalidMessage | 870 | return nil, ErrInvalidMessage | ||
| 829 | } | 871 | } | ||
| 830 | return encodeServerEnvelope(*m, maxChunkBytes) | 872 | return encodeServerEnvelope(*m, maxChunkBytes) | ||
| 873 | + | case StreamWindowUpdate: | |||
| 874 | + | converted, err := streamWindowUpdateToWire(m) | |||
| 875 | + | if err != nil { | |||
| 876 | + | return nil, err | |||
| 877 | + | } | |||
| 878 | + | return &wire.BrokerServerEnvelope{Payload: &wire.BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: converted}}, nil | |||
| 879 | + | case *StreamWindowUpdate: | |||
| 880 | + | if m == nil { | |||
| 881 | + | return nil, ErrInvalidMessage | |||
| 882 | + | } | |||
| 883 | + | return encodeServerEnvelope(*m, maxChunkBytes) | |||
| 831 | case StreamClosed: | 884 | case StreamClosed: | ||
| 832 | converted, err := streamClosedToWire(m) | 885 | converted, err := streamClosedToWire(m) | ||
| 833 | if err != nil { | 886 | if err != nil { | ||
| @@ -929,6 +982,8 @@ func decodeServerEnvelope(envelope *wire.BrokerServerEnvelope, maxChunkBytes uin | |||||
| 929 | return serverStreamDataFromWire(payload.ServerStreamData, maxChunkBytes) | 982 | return serverStreamDataFromWire(payload.ServerStreamData, maxChunkBytes) | ||
| 930 | case *wire.BrokerServerEnvelope_StreamClosed: | 983 | case *wire.BrokerServerEnvelope_StreamClosed: | ||
| 931 | return streamClosedFromWire(payload.StreamClosed) | 984 | return streamClosedFromWire(payload.StreamClosed) | ||
| 985 | + | case *wire.BrokerServerEnvelope_StreamWindowUpdate: | |||
| 986 | + | return streamWindowUpdateFromWire(payload.StreamWindowUpdate) | |||
| 932 | case *wire.BrokerServerEnvelope_Progress: | 987 | case *wire.BrokerServerEnvelope_Progress: | ||
| 933 | return progressFromWire(payload.Progress) | 988 | return progressFromWire(payload.Progress) | ||
| 934 | case *wire.BrokerServerEnvelope_BrokerErrorMessage: | 989 | case *wire.BrokerServerEnvelope_BrokerErrorMessage: | ||
| @@ -1492,7 +1547,9 @@ func serverStreamDataFromWire(message *wire.ServerStreamData, maxChunkBytes uint | |||||
| 1492 | if len(data) == 0 { | 1547 | if len(data) == 0 { | ||
| 1493 | return ServerStreamData{}, ErrInvalidMessage | 1548 | return ServerStreamData{}, ErrInvalidMessage | ||
| 1494 | } | 1549 | } | ||
| 1495 | - | return ServerStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: append([]byte(nil), data...)}, nil | 1550 | + | // proto.Unmarshal copies bytes fields out of the payload, so the caller |
| 1551 | + | // owns data without another copy. | |||
| 1552 | + | return ServerStreamData{Epoch: epoch, Connection: connection, Stream: stream, Data: data}, nil | |||
| 1496 | } | 1553 | } | ||
| 1497 | 1554 | ||||
| 1498 | func streamClosedToWire(m StreamClosed) (*wire.StreamClosed, error) { | 1555 | func streamClosedToWire(m StreamClosed) (*wire.StreamClosed, error) { | ||
M internal/adapters/brokerwire/codec_test.go +55 -1
| @@ -360,8 +360,9 @@ func TestBrokerClientVariants(t *testing.T) { | |||
| 360 | 360 | {"update_host_policy", UpdateHostPolicy{Epoch: 7, Connection: connection, Operation: operation, Registration: testRegistration(), Policy: testPolicy()}, 111}, | |
| 361 | 361 | {"start_preview", testStartPreview(connection), 112}, | |
| 362 | 362 | {"cancel_preview", CancelPreview{Epoch: 7, Connection: connection, Generation: 3}, 113}, | |
| 363 | + | {"stream_window_update", StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: 4096}, 114}, | |
| 363 | 364 | } | |
| 364 | - | require.Len(t, messages, 13) | |
| 365 | + | require.Len(t, messages, 14) | |
| 365 | 366 | for _, tc := range messages { | |
| 366 | 367 | t.Run(tc.name, func(t *testing.T) { | |
| 367 | 368 | raw := mustEncodeClient(t, tc.message) | |
| @@ -376,6 +377,22 @@ func TestBrokerClientVariants(t *testing.T) { | |||
| 376 | 377 | } | |
| 377 | 378 | } | |
| 378 | 379 | ||
| 380 | + | // TestBrokerStreamDataOwnsItsBytes proves decoded stream data never aliases the | |
| 381 | + | // payload: receivers reuse their buffers, and brokeripc queues Data as is. | |
| 382 | + | func TestBrokerStreamDataOwnsItsBytes(t *testing.T) { | |
| 383 | + | connection := testConnectionID(0x11) | |
| 384 | + | client := mustEncodeClient(t, ClientStreamData{Epoch: 7, Connection: connection, Stream: 3, Data: []byte("frame")}) | |
| 385 | + | server := mustEncodeServer(t, ServerStreamData{Epoch: 7, Connection: connection, Stream: 3, Data: []byte("frame")}) | |
| 386 | + | decodedClient, err := DecodeClient(client, testEnvelopeCeiling, testChunkCeiling) | |
| 387 | + | require.NoError(t, err) | |
| 388 | + | decodedServer, err := DecodeServer(server, testEnvelopeCeiling, testChunkCeiling) | |
| 389 | + | require.NoError(t, err) | |
| 390 | + | clear(client) | |
| 391 | + | clear(server) | |
| 392 | + | require.Equal(t, []byte("frame"), decodedClient.(ClientStreamData).Data) | |
| 393 | + | require.Equal(t, []byte("frame"), decodedServer.(ServerStreamData).Data) | |
| 394 | + | } | |
| 395 | + | ||
| 379 | 396 | // TestBrokerOpenStreamAdmissionVariants pins the attachment-admission | |
| 380 | 397 | // contract: exact, create-named, and create-ephemeral round-trip, control | |
| 381 | 398 | // carries none, and wrong admission/purpose/name/target combinations are | |
| @@ -653,6 +670,7 @@ func TestBrokerServerVariants(t *testing.T) { | |||
| 653 | 670 | {"shutdown", Shutdown{Epoch: 7, Connection: connection, Reason: ShutdownTerminating, Text: "stopping"}, 209}, | |
| 654 | 671 | {"preview_publication", testPreviewPublication(connection), 210}, | |
| 655 | 672 | {"preview_publication_error", PreviewPublication{Epoch: 7, Connection: connection, Generation: 5, Error: testErrorDetail(), HasError: true}, 210}, | |
| 673 | + | {"stream_window_update", StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: StreamWindowBytes}, 211}, | |
| 656 | 674 | } | |
| 657 | 675 | for _, tc := range messages { | |
| 658 | 676 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -924,6 +942,40 @@ func TestBrokerValidationNegatives(t *testing.T) { | |||
| 924 | 942 | _, err := EncodeClient(CloseStream{Epoch: 7, Connection: connection}, testEnvelopeCeiling, testChunkCeiling) | |
| 925 | 943 | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 926 | 944 | }) | |
| 945 | + | t.Run("stream window update bounds", func(t *testing.T) { | |
| 946 | + | valid := StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: 1} | |
| 947 | + | cases := map[string]func(*StreamWindowUpdate){ | |
| 948 | + | "zero credit": func(m *StreamWindowUpdate) { m.Credit = 0 }, | |
| 949 | + | "credit above window": func(m *StreamWindowUpdate) { m.Credit = StreamWindowBytes + 1 }, | |
| 950 | + | "zero epoch": func(m *StreamWindowUpdate) { m.Epoch = 0 }, | |
| 951 | + | "zero stream": func(m *StreamWindowUpdate) { m.Stream = 0 }, | |
| 952 | + | } | |
| 953 | + | for name, mutate := range cases { | |
| 954 | + | t.Run(name, func(t *testing.T) { | |
| 955 | + | bad := valid | |
| 956 | + | mutate(&bad) | |
| 957 | + | _, err := EncodeClient(bad, testEnvelopeCeiling, testChunkCeiling) | |
| 958 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 959 | + | _, err = EncodeServer(bad, testEnvelopeCeiling, testChunkCeiling) | |
| 960 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 961 | + | }) | |
| 962 | + | } | |
| 963 | + | // The decoder refuses the same out-of-range credit a peer could put | |
| 964 | + | // on the wire directly. | |
| 965 | + | for name, credit := range map[string]uint64{"zero credit": 0, "credit above window": StreamWindowBytes + 1} { | |
| 966 | + | t.Run("decode "+name, func(t *testing.T) { | |
| 967 | + | message := &wire.StreamWindowUpdate{Ref: refToWire(7, connection, 3), Credit: credit} | |
| 968 | + | client, err := proto.Marshal(&wire.BrokerClientEnvelope{Payload: &wire.BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: message}}) | |
| 969 | + | require.NoError(t, err) | |
| 970 | + | _, err = DecodeClient(client, testEnvelopeCeiling, testChunkCeiling) | |
| 971 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 972 | + | server, err := proto.Marshal(&wire.BrokerServerEnvelope{Payload: &wire.BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: message}}) | |
| 973 | + | require.NoError(t, err) | |
| 974 | + | _, err = DecodeServer(server, testEnvelopeCeiling, testChunkCeiling) | |
| 975 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 976 | + | }) | |
| 977 | + | } | |
| 978 | + | }) | |
| 927 | 979 | t.Run("error detail never travels without HasError", func(t *testing.T) { | |
| 928 | 980 | detail := testErrorDetail() | |
| 929 | 981 | // HasError=false is exactly the absent detail, whatever the outcome | |
| @@ -1254,6 +1306,7 @@ func fuzzClientSeeds(t testing.TB) [][]byte { | |||
| 1254 | 1306 | OpenStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Purpose: ports.BrokerStreamAttachment, Admission: ports.BrokerAdmissionExact, Endpoint: "dev@host:22", Registration: testRegistration(), Target: testTarget(), Policy: testPolicy(), StartMode: ports.BrokerDaemonStartIfNeeded}, | |
| 1255 | 1307 | ClientStreamData{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Data: []byte("frame")}, | |
| 1256 | 1308 | CloseStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3}, | |
| 1309 | + | StreamWindowUpdate{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Credit: 4096}, | |
| 1257 | 1310 | } { | |
| 1258 | 1311 | raw, err := EncodeClient(message, testEnvelopeCeiling, testChunkCeiling) | |
| 1259 | 1312 | require.NoError(t, err) | |
| @@ -1272,6 +1325,7 @@ func fuzzServerSeeds(t testing.TB) [][]byte { | |||
| 1272 | 1325 | OperationResult{Epoch: 7, Connection: testConnectionID(0x11), Operation: testOperationID(0x14), Outcome: ports.BrokerOutcomeOK, Registration: testRegistration()}, | |
| 1273 | 1326 | BrokerErrorMessage{Epoch: 7, Connection: testConnectionID(0x11), Error: testErrorDetail()}, | |
| 1274 | 1327 | Shutdown{Epoch: 7, Connection: testConnectionID(0x11), Reason: ShutdownTerminating}, | |
| 1328 | + | StreamWindowUpdate{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Credit: 4096}, | |
| 1275 | 1329 | } { | |
| 1276 | 1330 | raw, err := EncodeServer(message, testEnvelopeCeiling, testChunkCeiling) | |
| 1277 | 1331 | require.NoError(t, err) | |
| @@ -360,8 +360,9 @@ func TestBrokerClientVariants(t *testing.T) { | |||||
| 360 | {"update_host_policy", UpdateHostPolicy{Epoch: 7, Connection: connection, Operation: operation, Registration: testRegistration(), Policy: testPolicy()}, 111}, | 360 | {"update_host_policy", UpdateHostPolicy{Epoch: 7, Connection: connection, Operation: operation, Registration: testRegistration(), Policy: testPolicy()}, 111}, | ||
| 361 | {"start_preview", testStartPreview(connection), 112}, | 361 | {"start_preview", testStartPreview(connection), 112}, | ||
| 362 | {"cancel_preview", CancelPreview{Epoch: 7, Connection: connection, Generation: 3}, 113}, | 362 | {"cancel_preview", CancelPreview{Epoch: 7, Connection: connection, Generation: 3}, 113}, | ||
| 363 | + | {"stream_window_update", StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: 4096}, 114}, | |||
| 363 | } | 364 | } | ||
| 364 | - | require.Len(t, messages, 13) | 365 | + | require.Len(t, messages, 14) |
| 365 | for _, tc := range messages { | 366 | for _, tc := range messages { | ||
| 366 | t.Run(tc.name, func(t *testing.T) { | 367 | t.Run(tc.name, func(t *testing.T) { | ||
| 367 | raw := mustEncodeClient(t, tc.message) | 368 | raw := mustEncodeClient(t, tc.message) | ||
| @@ -376,6 +377,22 @@ func TestBrokerClientVariants(t *testing.T) { | |||||
| 376 | } | 377 | } | ||
| 377 | } | 378 | } | ||
| 378 | 379 | ||||
| 380 | + | // TestBrokerStreamDataOwnsItsBytes proves decoded stream data never aliases the | |||
| 381 | + | // payload: receivers reuse their buffers, and brokeripc queues Data as is. | |||
| 382 | + | func TestBrokerStreamDataOwnsItsBytes(t *testing.T) { | |||
| 383 | + | connection := testConnectionID(0x11) | |||
| 384 | + | client := mustEncodeClient(t, ClientStreamData{Epoch: 7, Connection: connection, Stream: 3, Data: []byte("frame")}) | |||
| 385 | + | server := mustEncodeServer(t, ServerStreamData{Epoch: 7, Connection: connection, Stream: 3, Data: []byte("frame")}) | |||
| 386 | + | decodedClient, err := DecodeClient(client, testEnvelopeCeiling, testChunkCeiling) | |||
| 387 | + | require.NoError(t, err) | |||
| 388 | + | decodedServer, err := DecodeServer(server, testEnvelopeCeiling, testChunkCeiling) | |||
| 389 | + | require.NoError(t, err) | |||
| 390 | + | clear(client) | |||
| 391 | + | clear(server) | |||
| 392 | + | require.Equal(t, []byte("frame"), decodedClient.(ClientStreamData).Data) | |||
| 393 | + | require.Equal(t, []byte("frame"), decodedServer.(ServerStreamData).Data) | |||
| 394 | + | } | |||
| 395 | + | ||||
| 379 | // TestBrokerOpenStreamAdmissionVariants pins the attachment-admission | 396 | // TestBrokerOpenStreamAdmissionVariants pins the attachment-admission | ||
| 380 | // contract: exact, create-named, and create-ephemeral round-trip, control | 397 | // contract: exact, create-named, and create-ephemeral round-trip, control | ||
| 381 | // carries none, and wrong admission/purpose/name/target combinations are | 398 | // carries none, and wrong admission/purpose/name/target combinations are | ||
| @@ -653,6 +670,7 @@ func TestBrokerServerVariants(t *testing.T) { | |||||
| 653 | {"shutdown", Shutdown{Epoch: 7, Connection: connection, Reason: ShutdownTerminating, Text: "stopping"}, 209}, | 670 | {"shutdown", Shutdown{Epoch: 7, Connection: connection, Reason: ShutdownTerminating, Text: "stopping"}, 209}, | ||
| 654 | {"preview_publication", testPreviewPublication(connection), 210}, | 671 | {"preview_publication", testPreviewPublication(connection), 210}, | ||
| 655 | {"preview_publication_error", PreviewPublication{Epoch: 7, Connection: connection, Generation: 5, Error: testErrorDetail(), HasError: true}, 210}, | 672 | {"preview_publication_error", PreviewPublication{Epoch: 7, Connection: connection, Generation: 5, Error: testErrorDetail(), HasError: true}, 210}, | ||
| 673 | + | {"stream_window_update", StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: StreamWindowBytes}, 211}, | |||
| 656 | } | 674 | } | ||
| 657 | for _, tc := range messages { | 675 | for _, tc := range messages { | ||
| 658 | t.Run(tc.name, func(t *testing.T) { | 676 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -924,6 +942,40 @@ func TestBrokerValidationNegatives(t *testing.T) { | |||||
| 924 | _, err := EncodeClient(CloseStream{Epoch: 7, Connection: connection}, testEnvelopeCeiling, testChunkCeiling) | 942 | _, err := EncodeClient(CloseStream{Epoch: 7, Connection: connection}, testEnvelopeCeiling, testChunkCeiling) | ||
| 925 | require.ErrorIs(t, err, ErrInvalidMessage) | 943 | require.ErrorIs(t, err, ErrInvalidMessage) | ||
| 926 | }) | 944 | }) | ||
| 945 | + | t.Run("stream window update bounds", func(t *testing.T) { | |||
| 946 | + | valid := StreamWindowUpdate{Epoch: 7, Connection: connection, Stream: 3, Credit: 1} | |||
| 947 | + | cases := map[string]func(*StreamWindowUpdate){ | |||
| 948 | + | "zero credit": func(m *StreamWindowUpdate) { m.Credit = 0 }, | |||
| 949 | + | "credit above window": func(m *StreamWindowUpdate) { m.Credit = StreamWindowBytes + 1 }, | |||
| 950 | + | "zero epoch": func(m *StreamWindowUpdate) { m.Epoch = 0 }, | |||
| 951 | + | "zero stream": func(m *StreamWindowUpdate) { m.Stream = 0 }, | |||
| 952 | + | } | |||
| 953 | + | for name, mutate := range cases { | |||
| 954 | + | t.Run(name, func(t *testing.T) { | |||
| 955 | + | bad := valid | |||
| 956 | + | mutate(&bad) | |||
| 957 | + | _, err := EncodeClient(bad, testEnvelopeCeiling, testChunkCeiling) | |||
| 958 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |||
| 959 | + | _, err = EncodeServer(bad, testEnvelopeCeiling, testChunkCeiling) | |||
| 960 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |||
| 961 | + | }) | |||
| 962 | + | } | |||
| 963 | + | // The decoder refuses the same out-of-range credit a peer could put | |||
| 964 | + | // on the wire directly. | |||
| 965 | + | for name, credit := range map[string]uint64{"zero credit": 0, "credit above window": StreamWindowBytes + 1} { | |||
| 966 | + | t.Run("decode "+name, func(t *testing.T) { | |||
| 967 | + | message := &wire.StreamWindowUpdate{Ref: refToWire(7, connection, 3), Credit: credit} | |||
| 968 | + | client, err := proto.Marshal(&wire.BrokerClientEnvelope{Payload: &wire.BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: message}}) | |||
| 969 | + | require.NoError(t, err) | |||
| 970 | + | _, err = DecodeClient(client, testEnvelopeCeiling, testChunkCeiling) | |||
| 971 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |||
| 972 | + | server, err := proto.Marshal(&wire.BrokerServerEnvelope{Payload: &wire.BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: message}}) | |||
| 973 | + | require.NoError(t, err) | |||
| 974 | + | _, err = DecodeServer(server, testEnvelopeCeiling, testChunkCeiling) | |||
| 975 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |||
| 976 | + | }) | |||
| 977 | + | } | |||
| 978 | + | }) | |||
| 927 | t.Run("error detail never travels without HasError", func(t *testing.T) { | 979 | t.Run("error detail never travels without HasError", func(t *testing.T) { | ||
| 928 | detail := testErrorDetail() | 980 | detail := testErrorDetail() | ||
| 929 | // HasError=false is exactly the absent detail, whatever the outcome | 981 | // HasError=false is exactly the absent detail, whatever the outcome | ||
| @@ -1254,6 +1306,7 @@ func fuzzClientSeeds(t testing.TB) [][]byte { | |||||
| 1254 | OpenStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Purpose: ports.BrokerStreamAttachment, Admission: ports.BrokerAdmissionExact, Endpoint: "dev@host:22", Registration: testRegistration(), Target: testTarget(), Policy: testPolicy(), StartMode: ports.BrokerDaemonStartIfNeeded}, | 1306 | OpenStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Purpose: ports.BrokerStreamAttachment, Admission: ports.BrokerAdmissionExact, Endpoint: "dev@host:22", Registration: testRegistration(), Target: testTarget(), Policy: testPolicy(), StartMode: ports.BrokerDaemonStartIfNeeded}, | ||
| 1255 | ClientStreamData{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Data: []byte("frame")}, | 1307 | ClientStreamData{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Data: []byte("frame")}, | ||
| 1256 | CloseStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3}, | 1308 | CloseStream{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3}, | ||
| 1309 | + | StreamWindowUpdate{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Credit: 4096}, | |||
| 1257 | } { | 1310 | } { | ||
| 1258 | raw, err := EncodeClient(message, testEnvelopeCeiling, testChunkCeiling) | 1311 | raw, err := EncodeClient(message, testEnvelopeCeiling, testChunkCeiling) | ||
| 1259 | require.NoError(t, err) | 1312 | require.NoError(t, err) | ||
| @@ -1272,6 +1325,7 @@ func fuzzServerSeeds(t testing.TB) [][]byte { | |||||
| 1272 | OperationResult{Epoch: 7, Connection: testConnectionID(0x11), Operation: testOperationID(0x14), Outcome: ports.BrokerOutcomeOK, Registration: testRegistration()}, | 1325 | OperationResult{Epoch: 7, Connection: testConnectionID(0x11), Operation: testOperationID(0x14), Outcome: ports.BrokerOutcomeOK, Registration: testRegistration()}, | ||
| 1273 | BrokerErrorMessage{Epoch: 7, Connection: testConnectionID(0x11), Error: testErrorDetail()}, | 1326 | BrokerErrorMessage{Epoch: 7, Connection: testConnectionID(0x11), Error: testErrorDetail()}, | ||
| 1274 | Shutdown{Epoch: 7, Connection: testConnectionID(0x11), Reason: ShutdownTerminating}, | 1327 | Shutdown{Epoch: 7, Connection: testConnectionID(0x11), Reason: ShutdownTerminating}, | ||
| 1328 | + | StreamWindowUpdate{Epoch: 7, Connection: testConnectionID(0x11), Stream: 3, Credit: 4096}, | |||
| 1275 | } { | 1329 | } { | ||
| 1276 | raw, err := EncodeServer(message, testEnvelopeCeiling, testChunkCeiling) | 1330 | raw, err := EncodeServer(message, testEnvelopeCeiling, testChunkCeiling) | ||
| 1277 | require.NoError(t, err) | 1331 | require.NoError(t, err) | ||
M internal/adapters/brokerwire/limits.go +14 -0
| @@ -39,8 +39,22 @@ const ( | |||
| 39 | 39 | // MaxStreamChunkBytes is the largest legal stream chunk ceiling: | |
| 40 | 40 | // 64 KiB per opaque stream frame. | |
| 41 | 41 | MaxStreamChunkBytes uint64 = 64 << 10 | |
| 42 | + | ||
| 43 | + | // StreamWindowBytes is the stream-data credit each side of one logical | |
| 44 | + | // stream grants its peer per direction. A sender spends StreamChunkCredit | |
| 45 | + | // per data frame and waits when it has too little, so the receiver never | |
| 46 | + | // queues more than this per stream; StreamWindowUpdate returns the credit | |
| 47 | + | // of consumed frames. | |
| 48 | + | StreamWindowBytes uint64 = 4 << 20 | |
| 49 | + | // StreamChunkCreditOverhead is the fixed credit one data frame costs on top | |
| 50 | + | // of its payload, so a burst of tiny frames is bounded in count as well. | |
| 51 | + | StreamChunkCreditOverhead uint64 = 64 | |
| 42 | 52 | ) | |
| 43 | 53 | ||
| 54 | + | // StreamChunkCredit is the flow-control cost of one stream data frame of n | |
| 55 | + | // payload bytes. | |
| 56 | + | func StreamChunkCredit(n int) uint64 { return uint64(n) + StreamChunkCreditOverhead } | |
| 57 | + | ||
| 44 | 58 | // Ceilings are the immutable per-connection ceilings negotiated once by the | |
| 45 | 59 | // broker preamble. MaxReceiveEnvelopeBytes bounds every envelope this side | |
| 46 | 60 | // accepts; StreamChunkLimit bounds one opaque stream data frame in either | |
| @@ -39,8 +39,22 @@ const ( | |||||
| 39 | // MaxStreamChunkBytes is the largest legal stream chunk ceiling: | 39 | // MaxStreamChunkBytes is the largest legal stream chunk ceiling: | ||
| 40 | // 64 KiB per opaque stream frame. | 40 | // 64 KiB per opaque stream frame. | ||
| 41 | MaxStreamChunkBytes uint64 = 64 << 10 | 41 | MaxStreamChunkBytes uint64 = 64 << 10 | ||
| 42 | + | ||||
| 43 | + | // StreamWindowBytes is the stream-data credit each side of one logical | |||
| 44 | + | // stream grants its peer per direction. A sender spends StreamChunkCredit | |||
| 45 | + | // per data frame and waits when it has too little, so the receiver never | |||
| 46 | + | // queues more than this per stream; StreamWindowUpdate returns the credit | |||
| 47 | + | // of consumed frames. | |||
| 48 | + | StreamWindowBytes uint64 = 4 << 20 | |||
| 49 | + | // StreamChunkCreditOverhead is the fixed credit one data frame costs on top | |||
| 50 | + | // of its payload, so a burst of tiny frames is bounded in count as well. | |||
| 51 | + | StreamChunkCreditOverhead uint64 = 64 | |||
| 42 | ) | 52 | ) | ||
| 43 | 53 | ||||
| 54 | + | // StreamChunkCredit is the flow-control cost of one stream data frame of n | |||
| 55 | + | // payload bytes. | |||
| 56 | + | func StreamChunkCredit(n int) uint64 { return uint64(n) + StreamChunkCreditOverhead } | |||
| 57 | + | ||||
| 44 | // Ceilings are the immutable per-connection ceilings negotiated once by the | 58 | // Ceilings are the immutable per-connection ceilings negotiated once by the | ||
| 45 | // broker preamble. MaxReceiveEnvelopeBytes bounds every envelope this side | 59 | // broker preamble. MaxReceiveEnvelopeBytes bounds every envelope this side | ||
| 46 | // accepts; StreamChunkLimit bounds one opaque stream data frame in either | 60 | // accepts; StreamChunkLimit bounds one opaque stream data frame in either | ||
M internal/adapters/brokerwire/messages.go +13 -0
| @@ -165,6 +165,19 @@ type CloseStream struct { | |||
| 165 | 165 | ||
| 166 | 166 | func (CloseStream) brokerClientMessage() {} | |
| 167 | 167 | ||
| 168 | + | // StreamWindowUpdate grants the peer Credit more stream-data bytes on one | |
| 169 | + | // logical stream after the receiver consumed them. It travels in both | |
| 170 | + | // directions: each side grants credit for the data it receives. | |
| 171 | + | type StreamWindowUpdate struct { | |
| 172 | + | Epoch ports.BrokerEpoch | |
| 173 | + | Connection ports.BrokerConnectionID | |
| 174 | + | Stream ports.BrokerStreamID | |
| 175 | + | Credit uint64 | |
| 176 | + | } | |
| 177 | + | ||
| 178 | + | func (StreamWindowUpdate) brokerClientMessage() {} | |
| 179 | + | func (StreamWindowUpdate) brokerServerMessage() {} | |
| 180 | + | ||
| 168 | 181 | // StartPreview starts or replaces one connection-scoped live preview. | |
| 169 | 182 | // Generation is the connection-scoped preview authority; Route names the | |
| 170 | 183 | // observed daemon (the broker allocates the stream) and Preview is the bounded | |
| @@ -165,6 +165,19 @@ type CloseStream struct { | |||||
| 165 | 165 | ||||
| 166 | func (CloseStream) brokerClientMessage() {} | 166 | func (CloseStream) brokerClientMessage() {} | ||
| 167 | 167 | ||||
| 168 | + | // StreamWindowUpdate grants the peer Credit more stream-data bytes on one | |||
| 169 | + | // logical stream after the receiver consumed them. It travels in both | |||
| 170 | + | // directions: each side grants credit for the data it receives. | |||
| 171 | + | type StreamWindowUpdate struct { | |||
| 172 | + | Epoch ports.BrokerEpoch | |||
| 173 | + | Connection ports.BrokerConnectionID | |||
| 174 | + | Stream ports.BrokerStreamID | |||
| 175 | + | Credit uint64 | |||
| 176 | + | } | |||
| 177 | + | ||||
| 178 | + | func (StreamWindowUpdate) brokerClientMessage() {} | |||
| 179 | + | func (StreamWindowUpdate) brokerServerMessage() {} | |||
| 180 | + | ||||
| 168 | // StartPreview starts or replaces one connection-scoped live preview. | 181 | // StartPreview starts or replaces one connection-scoped live preview. | ||
| 169 | // Generation is the connection-scoped preview authority; Route names the | 182 | // Generation is the connection-scoped preview authority; Route names the | ||
| 170 | // observed daemon (the broker allocates the stream) and Preview is the bounded | 183 | // observed daemon (the broker allocates the stream) and Preview is the bounded | ||
M internal/protocol/session.go +8 -1
| @@ -17,7 +17,7 @@ import ( | |||
| 17 | 17 | // daemonmux physical preamble conversation (multiplex.proto), whose | |
| 18 | 18 | // negotiated ceilings and accepted daemon binding ride the same wire | |
| 19 | 19 | // version as the session and broker conversations. | |
| 20 | - | const Version uint16 = 65 | |
| 20 | + | const Version uint16 = 66 | |
| 21 | 21 | ||
| 22 | 22 | // HandshakeTimeout bounds every transport handshake from connect through the | |
| 23 | 23 | // first committed publication. It excludes the preceding client-local | |
| @@ -33,6 +33,13 @@ const MaxOutputDataLen = (16 << 20) - 1 - 55 - (8 + 16 + 2 + 64 + 1 + 2 + 128 + | |||
| 33 | 33 | // MaxOutputWindow caps output states sent before client acknowledgement. | |
| 34 | 34 | const MaxOutputWindow = 8 | |
| 35 | 35 | ||
| 36 | + | // MaxOutputWindowBytes caps the Output data a daemon keeps unacknowledged per | |
| 37 | + | // attachment, state frames and side effects together. A full window of large | |
| 38 | + | // frames would otherwise overrun every bounded buffer between daemon and | |
| 39 | + | // terminal. The daemon may always send one frame when none is unacknowledged, | |
| 40 | + | // so a single frame up to MaxOutputDataLen still makes progress. | |
| 41 | + | const MaxOutputWindowBytes = 2 << 20 | |
| 42 | + | ||
| 36 | 43 | // ConnectionCapabilities describes transport behavior relevant to session | |
| 37 | 44 | // flow without naming a concrete carriage. | |
| 38 | 45 | type ConnectionCapabilities struct { | |
| @@ -17,7 +17,7 @@ import ( | |||||
| 17 | // daemonmux physical preamble conversation (multiplex.proto), whose | 17 | // daemonmux physical preamble conversation (multiplex.proto), whose | ||
| 18 | // negotiated ceilings and accepted daemon binding ride the same wire | 18 | // negotiated ceilings and accepted daemon binding ride the same wire | ||
| 19 | // version as the session and broker conversations. | 19 | // version as the session and broker conversations. | ||
| 20 | - | const Version uint16 = 65 | 20 | + | const Version uint16 = 66 |
| 21 | 21 | ||||
| 22 | // HandshakeTimeout bounds every transport handshake from connect through the | 22 | // HandshakeTimeout bounds every transport handshake from connect through the | ||
| 23 | // first committed publication. It excludes the preceding client-local | 23 | // first committed publication. It excludes the preceding client-local | ||
| @@ -33,6 +33,13 @@ const MaxOutputDataLen = (16 << 20) - 1 - 55 - (8 + 16 + 2 + 64 + 1 + 2 + 128 + | |||||
| 33 | // MaxOutputWindow caps output states sent before client acknowledgement. | 33 | // MaxOutputWindow caps output states sent before client acknowledgement. | ||
| 34 | const MaxOutputWindow = 8 | 34 | const MaxOutputWindow = 8 | ||
| 35 | 35 | ||||
| 36 | + | // MaxOutputWindowBytes caps the Output data a daemon keeps unacknowledged per | |||
| 37 | + | // attachment, state frames and side effects together. A full window of large | |||
| 38 | + | // frames would otherwise overrun every bounded buffer between daemon and | |||
| 39 | + | // terminal. The daemon may always send one frame when none is unacknowledged, | |||
| 40 | + | // so a single frame up to MaxOutputDataLen still makes progress. | |||
| 41 | + | const MaxOutputWindowBytes = 2 << 20 | |||
| 42 | + | ||||
| 36 | // ConnectionCapabilities describes transport behavior relevant to session | 43 | // ConnectionCapabilities describes transport behavior relevant to session | ||
| 37 | // flow without naming a concrete carriage. | 44 | // flow without naming a concrete carriage. | ||
| 38 | type ConnectionCapabilities struct { | 45 | type ConnectionCapabilities struct { | ||
M internal/protocol/session_test.go +1 -1
| @@ -10,7 +10,7 @@ import ( | |||
| 10 | 10 | ) | |
| 11 | 11 | ||
| 12 | 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | |
| 13 | - | require.Equal(t, uint16(65), Version) | |
| 13 | + | require.Equal(t, uint16(66), Version) | |
| 14 | 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | |
| 15 | 15 | } | |
| 16 | 16 | ||
| @@ -10,7 +10,7 @@ import ( | |||||
| 10 | ) | 10 | ) | ||
| 11 | 11 | ||||
| 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | ||
| 13 | - | require.Equal(t, uint16(65), Version) | 13 | + | require.Equal(t, uint16(66), Version) |
| 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | ||
| 15 | } | 15 | } | ||
| 16 | 16 | ||||
M internal/protocol/wire/broker.pb.go +262 -163 generated file: collapsed
M internal/protocol/wire/broker_test.go +8 -2
| @@ -20,7 +20,7 @@ var brokerClientTags = map[string]protoreflect.FieldNumber{ | |||
| 20 | 20 | "register": 101, "subscribe": 102, "resync": 103, "unsubscribe": 104, | |
| 21 | 21 | "add_host": 105, "remove_host": 106, "reconcile": 107, "open_stream": 108, | |
| 22 | 22 | "client_stream_data": 109, "close_stream": 110, "update_host_policy": 111, | |
| 23 | - | "start_preview": 112, "cancel_preview": 113, | |
| 23 | + | "start_preview": 112, "cancel_preview": 113, "stream_window_update": 114, | |
| 24 | 24 | } | |
| 25 | 25 | ||
| 26 | 26 | // brokerServerTags is the frozen server tag inventory. | |
| @@ -28,7 +28,7 @@ var brokerServerTags = map[string]protoreflect.FieldNumber{ | |||
| 28 | 28 | "registered": 201, "snapshot_part": 202, "operation_result": 203, | |
| 29 | 29 | "stream_opened": 204, "server_stream_data": 205, "stream_closed": 206, | |
| 30 | 30 | "progress": 207, "broker_error_message": 208, "shutdown": 209, | |
| 31 | - | "preview_publication": 210, | |
| 31 | + | "preview_publication": 210, "stream_window_update": 211, | |
| 32 | 32 | } | |
| 33 | 33 | ||
| 34 | 34 | func oneofTags(t *testing.T, message proto.Message) map[string]protoreflect.FieldNumber { | |
| @@ -194,6 +194,9 @@ func brokerClientSamples() map[string]*BrokerClientEnvelope { | |||
| 194 | 194 | "cancel_preview": {Payload: &BrokerClientEnvelope_CancelPreview{CancelPreview: &CancelPreview{ | |
| 195 | 195 | Scope: brokerScope(), Generation: 3, | |
| 196 | 196 | }}}, | |
| 197 | + | "stream_window_update": {Payload: &BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: &StreamWindowUpdate{ | |
| 198 | + | Ref: brokerRef(), Credit: 4096, | |
| 199 | + | }}}, | |
| 197 | 200 | } | |
| 198 | 201 | } | |
| 199 | 202 | ||
| @@ -241,6 +244,9 @@ func brokerServerSamples() map[string]*BrokerServerEnvelope { | |||
| 241 | 244 | Scope: brokerScope(), Generation: 3, | |
| 242 | 245 | Result: &PreviewPublication_Error{Error: &BrokerErrorDetail{Code: 1, Text: "unavailable"}}, | |
| 243 | 246 | }}}, | |
| 247 | + | "stream_window_update": {Payload: &BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: &StreamWindowUpdate{ | |
| 248 | + | Ref: brokerRef(), Credit: 4096, | |
| 249 | + | }}}, | |
| 244 | 250 | } | |
| 245 | 251 | } | |
| 246 | 252 | ||
| @@ -20,7 +20,7 @@ var brokerClientTags = map[string]protoreflect.FieldNumber{ | |||||
| 20 | "register": 101, "subscribe": 102, "resync": 103, "unsubscribe": 104, | 20 | "register": 101, "subscribe": 102, "resync": 103, "unsubscribe": 104, | ||
| 21 | "add_host": 105, "remove_host": 106, "reconcile": 107, "open_stream": 108, | 21 | "add_host": 105, "remove_host": 106, "reconcile": 107, "open_stream": 108, | ||
| 22 | "client_stream_data": 109, "close_stream": 110, "update_host_policy": 111, | 22 | "client_stream_data": 109, "close_stream": 110, "update_host_policy": 111, | ||
| 23 | - | "start_preview": 112, "cancel_preview": 113, | 23 | + | "start_preview": 112, "cancel_preview": 113, "stream_window_update": 114, |
| 24 | } | 24 | } | ||
| 25 | 25 | ||||
| 26 | // brokerServerTags is the frozen server tag inventory. | 26 | // brokerServerTags is the frozen server tag inventory. | ||
| @@ -28,7 +28,7 @@ var brokerServerTags = map[string]protoreflect.FieldNumber{ | |||||
| 28 | "registered": 201, "snapshot_part": 202, "operation_result": 203, | 28 | "registered": 201, "snapshot_part": 202, "operation_result": 203, | ||
| 29 | "stream_opened": 204, "server_stream_data": 205, "stream_closed": 206, | 29 | "stream_opened": 204, "server_stream_data": 205, "stream_closed": 206, | ||
| 30 | "progress": 207, "broker_error_message": 208, "shutdown": 209, | 30 | "progress": 207, "broker_error_message": 208, "shutdown": 209, | ||
| 31 | - | "preview_publication": 210, | 31 | + | "preview_publication": 210, "stream_window_update": 211, |
| 32 | } | 32 | } | ||
| 33 | 33 | ||||
| 34 | func oneofTags(t *testing.T, message proto.Message) map[string]protoreflect.FieldNumber { | 34 | func oneofTags(t *testing.T, message proto.Message) map[string]protoreflect.FieldNumber { | ||
| @@ -194,6 +194,9 @@ func brokerClientSamples() map[string]*BrokerClientEnvelope { | |||||
| 194 | "cancel_preview": {Payload: &BrokerClientEnvelope_CancelPreview{CancelPreview: &CancelPreview{ | 194 | "cancel_preview": {Payload: &BrokerClientEnvelope_CancelPreview{CancelPreview: &CancelPreview{ | ||
| 195 | Scope: brokerScope(), Generation: 3, | 195 | Scope: brokerScope(), Generation: 3, | ||
| 196 | }}}, | 196 | }}}, | ||
| 197 | + | "stream_window_update": {Payload: &BrokerClientEnvelope_StreamWindowUpdate{StreamWindowUpdate: &StreamWindowUpdate{ | |||
| 198 | + | Ref: brokerRef(), Credit: 4096, | |||
| 199 | + | }}}, | |||
| 197 | } | 200 | } | ||
| 198 | } | 201 | } | ||
| 199 | 202 | ||||
| @@ -241,6 +244,9 @@ func brokerServerSamples() map[string]*BrokerServerEnvelope { | |||||
| 241 | Scope: brokerScope(), Generation: 3, | 244 | Scope: brokerScope(), Generation: 3, | ||
| 242 | Result: &PreviewPublication_Error{Error: &BrokerErrorDetail{Code: 1, Text: "unavailable"}}, | 245 | Result: &PreviewPublication_Error{Error: &BrokerErrorDetail{Code: 1, Text: "unavailable"}}, | ||
| 243 | }}}, | 246 | }}}, | ||
| 247 | + | "stream_window_update": {Payload: &BrokerServerEnvelope_StreamWindowUpdate{StreamWindowUpdate: &StreamWindowUpdate{ | |||
| 248 | + | Ref: brokerRef(), Credit: 4096, | |||
| 249 | + | }}}, | |||
| 244 | } | 250 | } | ||
| 245 | } | 251 | } | ||
| 246 | 252 | ||||
M internal/protocol/wire/envelope_inventory_test.go +2 -2
| @@ -15,8 +15,8 @@ import ( | |||
| 15 | 15 | // shares a payload type except route navigation failure, and the preamble | |
| 16 | 16 | // constants agree with the schema authority. | |
| 17 | 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | |
| 18 | - | if protocol.Version != 65 { | |
| 19 | - | t.Fatalf("protocol.Version = %d, want 65", protocol.Version) | |
| 18 | + | if protocol.Version != 66 { | |
| 19 | + | t.Fatalf("protocol.Version = %d, want 66", protocol.Version) | |
| 20 | 20 | } | |
| 21 | 21 | if ProtocolEpoch != 1 { | |
| 22 | 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | |
| @@ -15,8 +15,8 @@ import ( | |||||
| 15 | // shares a payload type except route navigation failure, and the preamble | 15 | // shares a payload type except route navigation failure, and the preamble | ||
| 16 | // constants agree with the schema authority. | 16 | // constants agree with the schema authority. | ||
| 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | ||
| 18 | - | if protocol.Version != 65 { | 18 | + | if protocol.Version != 66 { |
| 19 | - | t.Fatalf("protocol.Version = %d, want 65", protocol.Version) | 19 | + | t.Fatalf("protocol.Version = %d, want 66", protocol.Version) |
| 20 | } | 20 | } | ||
| 21 | if ProtocolEpoch != 1 { | 21 | if ProtocolEpoch != 1 { | ||
| 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | ||
M internal/protocol/wire/schema/broker.proto +11 -0
| @@ -89,6 +89,7 @@ message BrokerClientEnvelope { | |||
| 89 | 89 | UpdateHostPolicy update_host_policy = 111; | |
| 90 | 90 | StartPreview start_preview = 112; | |
| 91 | 91 | CancelPreview cancel_preview = 113; | |
| 92 | + | StreamWindowUpdate stream_window_update = 114; | |
| 92 | 93 | } | |
| 93 | 94 | } | |
| 94 | 95 | ||
| @@ -106,6 +107,7 @@ message BrokerServerEnvelope { | |||
| 106 | 107 | BrokerErrorMessage broker_error_message = 208; | |
| 107 | 108 | Shutdown shutdown = 209; | |
| 108 | 109 | PreviewPublication preview_publication = 210; | |
| 110 | + | StreamWindowUpdate stream_window_update = 211; | |
| 109 | 111 | } | |
| 110 | 112 | } | |
| 111 | 113 | ||
| @@ -221,6 +223,15 @@ message CloseStream { | |||
| 221 | 223 | BrokerStreamRef ref = 1; | |
| 222 | 224 | } | |
| 223 | 225 | ||
| 226 | + | // StreamWindowUpdate grants the peer credit to send more stream data on one | |
| 227 | + | // logical stream: the receiver consumed that much of its stream window. Each | |
| 228 | + | // direction of a stream has its own window, so a client grants credit for | |
| 229 | + | // ServerStreamData and the server grants credit for ClientStreamData. | |
| 230 | + | message StreamWindowUpdate { | |
| 231 | + | BrokerStreamRef ref = 1; | |
| 232 | + | uint64 credit = 2; | |
| 233 | + | } | |
| 234 | + | ||
| 224 | 235 | // Registered confirms one accepted client connection and its assigned scope. | |
| 225 | 236 | message Registered { | |
| 226 | 237 | BrokerScope scope = 1; | |
| @@ -89,6 +89,7 @@ message BrokerClientEnvelope { | |||||
| 89 | UpdateHostPolicy update_host_policy = 111; | 89 | UpdateHostPolicy update_host_policy = 111; | ||
| 90 | StartPreview start_preview = 112; | 90 | StartPreview start_preview = 112; | ||
| 91 | CancelPreview cancel_preview = 113; | 91 | CancelPreview cancel_preview = 113; | ||
| 92 | + | StreamWindowUpdate stream_window_update = 114; | |||
| 92 | } | 93 | } | ||
| 93 | } | 94 | } | ||
| 94 | 95 | ||||
| @@ -106,6 +107,7 @@ message BrokerServerEnvelope { | |||||
| 106 | BrokerErrorMessage broker_error_message = 208; | 107 | BrokerErrorMessage broker_error_message = 208; | ||
| 107 | Shutdown shutdown = 209; | 108 | Shutdown shutdown = 209; | ||
| 108 | PreviewPublication preview_publication = 210; | 109 | PreviewPublication preview_publication = 210; | ||
| 110 | + | StreamWindowUpdate stream_window_update = 211; | |||
| 109 | } | 111 | } | ||
| 110 | } | 112 | } | ||
| 111 | 113 | ||||
| @@ -221,6 +223,15 @@ message CloseStream { | |||||
| 221 | BrokerStreamRef ref = 1; | 223 | BrokerStreamRef ref = 1; | ||
| 222 | } | 224 | } | ||
| 223 | 225 | ||||
| 226 | + | // StreamWindowUpdate grants the peer credit to send more stream data on one | |||
| 227 | + | // logical stream: the receiver consumed that much of its stream window. Each | |||
| 228 | + | // direction of a stream has its own window, so a client grants credit for | |||
| 229 | + | // ServerStreamData and the server grants credit for ClientStreamData. | |||
| 230 | + | message StreamWindowUpdate { | |||
| 231 | + | BrokerStreamRef ref = 1; | |||
| 232 | + | uint64 credit = 2; | |||
| 233 | + | } | |||
| 234 | + | ||||
| 224 | // Registered confirms one accepted client connection and its assigned scope. | 235 | // Registered confirms one accepted client connection and its assigned scope. | ||
| 225 | message Registered { | 236 | message Registered { | ||
| 226 | BrokerScope scope = 1; | 237 | BrokerScope scope = 1; | ||
M internal/usecase/daemon/attachment_output.go +86 -9
| @@ -23,14 +23,22 @@ import ( | |||
| 23 | 23 | // Callers serialize access with attachedClient.sendMu. A prepared transaction | |
| 24 | 24 | // must be sent or committed as a no-op before the next transaction is prepared. | |
| 25 | 25 | type attachmentOutput struct { | |
| 26 | - | renderer *renderer.Renderer | |
| 27 | - | epoch uint64 | |
| 28 | - | next uint64 | |
| 29 | - | acked uint64 | |
| 30 | - | generation uint64 | |
| 31 | - | maxOutstanding uint64 | |
| 32 | - | outstandingAtomic atomic.Uint64 | |
| 33 | - | maxOutstandingAtomic atomic.Uint64 | |
| 26 | + | renderer *renderer.Renderer | |
| 27 | + | epoch uint64 | |
| 28 | + | next uint64 | |
| 29 | + | acked uint64 | |
| 30 | + | generation uint64 | |
| 31 | + | maxOutstanding uint64 | |
| 32 | + | outstandingAtomic atomic.Uint64 | |
| 33 | + | maxOutstandingAtomic atomic.Uint64 | |
| 34 | + | // unackedBytes charges Output data sent and not yet acknowledged, keyed by | |
| 35 | + | // the state an ACK must reach to release it. Side-effect output carries no | |
| 36 | + | // state of its own, so it is charged to the next state. Charges stay in | |
| 37 | + | // non-decreasing state order and merge per state, so the slice holds at most | |
| 38 | + | // maxOutstanding+1 entries. Guarded by the view lock; unackedBytesAtomic | |
| 39 | + | // mirrors the total for lock-free capacity reads. | |
| 40 | + | unackedBytes []unackedOutputBytes | |
| 41 | + | unackedBytesAtomic atomic.Uint64 | |
| 34 | 42 | forceSnapshot bool | |
| 35 | 43 | initialized bool | |
| 36 | 44 | viewPublication uint64 | |
| @@ -43,6 +51,51 @@ type attachmentOutput struct { | |||
| 43 | 51 | stateMu sync.Mutex | |
| 44 | 52 | } | |
| 45 | 53 | ||
| 54 | + | // unackedOutputBytes is the Output data released once the client acknowledges | |
| 55 | + | // state. | |
| 56 | + | type unackedOutputBytes struct { | |
| 57 | + | state uint64 | |
| 58 | + | bytes uint64 | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // chargeBytesLocked records n bytes released once state is acknowledged. | |
| 62 | + | // Caller holds the view lock. | |
| 63 | + | func (s *attachmentOutput) chargeBytesLocked(state uint64, n int) { | |
| 64 | + | if n <= 0 { | |
| 65 | + | return | |
| 66 | + | } | |
| 67 | + | if last := len(s.unackedBytes) - 1; last >= 0 && s.unackedBytes[last].state == state { | |
| 68 | + | s.unackedBytes[last].bytes += uint64(n) | |
| 69 | + | } else { | |
| 70 | + | s.unackedBytes = append(s.unackedBytes, unackedOutputBytes{state: state, bytes: uint64(n)}) | |
| 71 | + | } | |
| 72 | + | s.unackedBytesAtomic.Add(uint64(n)) | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | // releaseBytesLocked drops every charge an acknowledgement of state covers. | |
| 76 | + | // Caller holds the view lock. | |
| 77 | + | func (s *attachmentOutput) releaseBytesLocked(state uint64) { | |
| 78 | + | released, kept := uint64(0), 0 | |
| 79 | + | for _, charge := range s.unackedBytes { | |
| 80 | + | if charge.state <= state { | |
| 81 | + | released += charge.bytes | |
| 82 | + | continue | |
| 83 | + | } | |
| 84 | + | s.unackedBytes[kept] = charge | |
| 85 | + | kept++ | |
| 86 | + | } | |
| 87 | + | clear(s.unackedBytes[kept:]) | |
| 88 | + | s.unackedBytes = s.unackedBytes[:kept] | |
| 89 | + | s.unackedBytesAtomic.Add(-released) | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | // resetBytesLocked forgets every charge: a new epoch acknowledges nothing the | |
| 93 | + | // old one sent. Caller holds the view lock. | |
| 94 | + | func (s *attachmentOutput) resetBytesLocked() { | |
| 95 | + | s.unackedBytes = s.unackedBytes[:0] | |
| 96 | + | s.unackedBytesAtomic.Store(0) | |
| 97 | + | } | |
| 98 | + | ||
| 46 | 99 | // reconfigureAttachmentOutput applies a replacement terminal's declared | |
| 47 | 100 | // capabilities to attachment-owned output state. Caller holds d.mu. | |
| 48 | 101 | func (d *Daemon) reconfigureAttachmentOutput(sess *session, ac *attachedClient, h protocol.Hello) { | |
| @@ -169,6 +222,9 @@ func (o *attachmentOutput) prepareFrame(d *Daemon, state *capturedRenderState, f | |||
| 169 | 222 | ansi.viewRevision = state.view.revision | |
| 170 | 223 | cursor := o.prepareCursorTail(desired, len(ansi.data) > 0) | |
| 171 | 224 | graphicsReset := reset || o.forceSnapshot || !o.initialized | |
| 225 | + | // Images share the frame's data limit. The unacknowledged byte window gates | |
| 226 | + | // the next frame rather than this one: an upload must fit one frame, so | |
| 227 | + | // clamping it here would drop large images and their deletes outright. | |
| 172 | 228 | graphicsBudget := max(protocol.MaxOutputDataLen-len(ansi.data)-len(cursor.data), 0) | |
| 173 | 229 | preparedGraphics, err := graphicsOutputDataWithDaemonLimit(d, state, o.attachment, graphicsReset, graphicsBudget) | |
| 174 | 230 | if err != nil { | |
| @@ -374,6 +430,7 @@ func (p *preparedOutput) send(data []byte, echoAck uint64, send func(protocol.Ou | |||
| 374 | 430 | p.boundary = protocol.UIReceipt{Epoch: p.epoch, State: p.next, ViewPublication: context.Publication, Outcome: protocol.UIReceiptProcessed} | |
| 375 | 431 | } | |
| 376 | 432 | p.stream.next = p.next | |
| 433 | + | p.stream.chargeBytesLocked(p.next, len(data)) | |
| 377 | 434 | p.stream.publishOutstanding() | |
| 378 | 435 | p.sent = true | |
| 379 | 436 | return nil | |
| @@ -510,6 +567,17 @@ func (s *attachmentOutput) sideEffectLocked(data []byte, echoAck uint64) (protoc | |||
| 510 | 567 | return marshalOutputState(data, epoch, 0, 0, echoAck, viewRevision, size, false, nil) | |
| 511 | 568 | } | |
| 512 | 569 | ||
| 570 | + | // sideEffectSentLocked charges a side effect that reached the transport. It has | |
| 571 | + | // no state of its own, so the ACK of the next state frame releases it: every | |
| 572 | + | // state frame is sent after it on the same ordered stream. Caller holds the view | |
| 573 | + | // lock that built output, so the epoch cannot have moved. | |
| 574 | + | func (s *attachmentOutput) sideEffectSentLocked(output protocol.Output) { | |
| 575 | + | if s == nil || output.Epoch != s.currentEpochLocked() { | |
| 576 | + | return | |
| 577 | + | } | |
| 578 | + | s.chargeBytesLocked(s.next+1, len(output.Data)) | |
| 579 | + | } | |
| 580 | + | ||
| 513 | 581 | func (s *attachmentOutput) ack(epoch, state uint64) bool { | |
| 514 | 582 | if s == nil { | |
| 515 | 583 | return false | |
| @@ -520,6 +588,7 @@ func (s *attachmentOutput) ack(epoch, state uint64) bool { | |||
| 520 | 588 | return false | |
| 521 | 589 | } | |
| 522 | 590 | s.acked = state | |
| 591 | + | s.releaseBytesLocked(state) | |
| 523 | 592 | s.publishOutstanding() | |
| 524 | 593 | return true | |
| 525 | 594 | } | |
| @@ -556,6 +625,7 @@ func (s *attachmentOutput) rebaseLocked() { | |||
| 556 | 625 | s.epoch = epoch + 1 | |
| 557 | 626 | s.next = 0 | |
| 558 | 627 | s.acked = 0 | |
| 628 | + | s.resetBytesLocked() | |
| 559 | 629 | s.generation++ | |
| 560 | 630 | s.publishOutstanding() | |
| 561 | 631 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | |
| @@ -591,6 +661,11 @@ func (s *attachmentOutput) setWindow(window uint8) { | |||
| 591 | 661 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | |
| 592 | 662 | } | |
| 593 | 663 | ||
| 664 | + | // atCapacity reports whether the next state frame must wait for an ACK: the | |
| 665 | + | // frame window is full, or unacknowledged Output data reached | |
| 666 | + | // protocol.MaxOutputWindowBytes while a state frame is in flight. With no state | |
| 667 | + | // frame in flight the next frame is always allowed, since only its ACK can | |
| 668 | + | // release bytes charged by side effects. | |
| 594 | 669 | func (s *attachmentOutput) atCapacity() bool { | |
| 595 | 670 | if s == nil { | |
| 596 | 671 | return false | |
| @@ -599,7 +674,9 @@ func (s *attachmentOutput) atCapacity() bool { | |||
| 599 | 674 | if maxOutstanding == 0 { | |
| 600 | 675 | maxOutstanding = s.maxOutstanding | |
| 601 | 676 | } | |
| 602 | - | return s.outstandingAtomic.Load() >= maxOutstanding | |
| 677 | + | outstanding := s.outstandingAtomic.Load() | |
| 678 | + | return outstanding >= maxOutstanding || | |
| 679 | + | (outstanding > 0 && s.unackedBytesAtomic.Load() >= protocol.MaxOutputWindowBytes) | |
| 603 | 680 | } | |
| 604 | 681 | ||
| 605 | 682 | // committedState reports the latest committed output state number. It takes | |
| @@ -23,14 +23,22 @@ import ( | |||||
| 23 | // Callers serialize access with attachedClient.sendMu. A prepared transaction | 23 | // Callers serialize access with attachedClient.sendMu. A prepared transaction | ||
| 24 | // must be sent or committed as a no-op before the next transaction is prepared. | 24 | // must be sent or committed as a no-op before the next transaction is prepared. | ||
| 25 | type attachmentOutput struct { | 25 | type attachmentOutput struct { | ||
| 26 | - | renderer *renderer.Renderer | 26 | + | renderer *renderer.Renderer |
| 27 | - | epoch uint64 | 27 | + | epoch uint64 |
| 28 | - | next uint64 | 28 | + | next uint64 |
| 29 | - | acked uint64 | 29 | + | acked uint64 |
| 30 | - | generation uint64 | 30 | + | generation uint64 |
| 31 | - | maxOutstanding uint64 | 31 | + | maxOutstanding uint64 |
| 32 | - | outstandingAtomic atomic.Uint64 | 32 | + | outstandingAtomic atomic.Uint64 |
| 33 | - | maxOutstandingAtomic atomic.Uint64 | 33 | + | maxOutstandingAtomic atomic.Uint64 |
| 34 | + | // unackedBytes charges Output data sent and not yet acknowledged, keyed by | |||
| 35 | + | // the state an ACK must reach to release it. Side-effect output carries no | |||
| 36 | + | // state of its own, so it is charged to the next state. Charges stay in | |||
| 37 | + | // non-decreasing state order and merge per state, so the slice holds at most | |||
| 38 | + | // maxOutstanding+1 entries. Guarded by the view lock; unackedBytesAtomic | |||
| 39 | + | // mirrors the total for lock-free capacity reads. | |||
| 40 | + | unackedBytes []unackedOutputBytes | |||
| 41 | + | unackedBytesAtomic atomic.Uint64 | |||
| 34 | forceSnapshot bool | 42 | forceSnapshot bool | ||
| 35 | initialized bool | 43 | initialized bool | ||
| 36 | viewPublication uint64 | 44 | viewPublication uint64 | ||
| @@ -43,6 +51,51 @@ type attachmentOutput struct { | |||||
| 43 | stateMu sync.Mutex | 51 | stateMu sync.Mutex | ||
| 44 | } | 52 | } | ||
| 45 | 53 | ||||
| 54 | + | // unackedOutputBytes is the Output data released once the client acknowledges | |||
| 55 | + | // state. | |||
| 56 | + | type unackedOutputBytes struct { | |||
| 57 | + | state uint64 | |||
| 58 | + | bytes uint64 | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | // chargeBytesLocked records n bytes released once state is acknowledged. | |||
| 62 | + | // Caller holds the view lock. | |||
| 63 | + | func (s *attachmentOutput) chargeBytesLocked(state uint64, n int) { | |||
| 64 | + | if n <= 0 { | |||
| 65 | + | return | |||
| 66 | + | } | |||
| 67 | + | if last := len(s.unackedBytes) - 1; last >= 0 && s.unackedBytes[last].state == state { | |||
| 68 | + | s.unackedBytes[last].bytes += uint64(n) | |||
| 69 | + | } else { | |||
| 70 | + | s.unackedBytes = append(s.unackedBytes, unackedOutputBytes{state: state, bytes: uint64(n)}) | |||
| 71 | + | } | |||
| 72 | + | s.unackedBytesAtomic.Add(uint64(n)) | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | // releaseBytesLocked drops every charge an acknowledgement of state covers. | |||
| 76 | + | // Caller holds the view lock. | |||
| 77 | + | func (s *attachmentOutput) releaseBytesLocked(state uint64) { | |||
| 78 | + | released, kept := uint64(0), 0 | |||
| 79 | + | for _, charge := range s.unackedBytes { | |||
| 80 | + | if charge.state <= state { | |||
| 81 | + | released += charge.bytes | |||
| 82 | + | continue | |||
| 83 | + | } | |||
| 84 | + | s.unackedBytes[kept] = charge | |||
| 85 | + | kept++ | |||
| 86 | + | } | |||
| 87 | + | clear(s.unackedBytes[kept:]) | |||
| 88 | + | s.unackedBytes = s.unackedBytes[:kept] | |||
| 89 | + | s.unackedBytesAtomic.Add(-released) | |||
| 90 | + | } | |||
| 91 | + | ||||
| 92 | + | // resetBytesLocked forgets every charge: a new epoch acknowledges nothing the | |||
| 93 | + | // old one sent. Caller holds the view lock. | |||
| 94 | + | func (s *attachmentOutput) resetBytesLocked() { | |||
| 95 | + | s.unackedBytes = s.unackedBytes[:0] | |||
| 96 | + | s.unackedBytesAtomic.Store(0) | |||
| 97 | + | } | |||
| 98 | + | ||||
| 46 | // reconfigureAttachmentOutput applies a replacement terminal's declared | 99 | // reconfigureAttachmentOutput applies a replacement terminal's declared | ||
| 47 | // capabilities to attachment-owned output state. Caller holds d.mu. | 100 | // capabilities to attachment-owned output state. Caller holds d.mu. | ||
| 48 | func (d *Daemon) reconfigureAttachmentOutput(sess *session, ac *attachedClient, h protocol.Hello) { | 101 | func (d *Daemon) reconfigureAttachmentOutput(sess *session, ac *attachedClient, h protocol.Hello) { | ||
| @@ -169,6 +222,9 @@ func (o *attachmentOutput) prepareFrame(d *Daemon, state *capturedRenderState, f | |||||
| 169 | ansi.viewRevision = state.view.revision | 222 | ansi.viewRevision = state.view.revision | ||
| 170 | cursor := o.prepareCursorTail(desired, len(ansi.data) > 0) | 223 | cursor := o.prepareCursorTail(desired, len(ansi.data) > 0) | ||
| 171 | graphicsReset := reset || o.forceSnapshot || !o.initialized | 224 | graphicsReset := reset || o.forceSnapshot || !o.initialized | ||
| 225 | + | // Images share the frame's data limit. The unacknowledged byte window gates | |||
| 226 | + | // the next frame rather than this one: an upload must fit one frame, so | |||
| 227 | + | // clamping it here would drop large images and their deletes outright. | |||
| 172 | graphicsBudget := max(protocol.MaxOutputDataLen-len(ansi.data)-len(cursor.data), 0) | 228 | graphicsBudget := max(protocol.MaxOutputDataLen-len(ansi.data)-len(cursor.data), 0) | ||
| 173 | preparedGraphics, err := graphicsOutputDataWithDaemonLimit(d, state, o.attachment, graphicsReset, graphicsBudget) | 229 | preparedGraphics, err := graphicsOutputDataWithDaemonLimit(d, state, o.attachment, graphicsReset, graphicsBudget) | ||
| 174 | if err != nil { | 230 | if err != nil { | ||
| @@ -374,6 +430,7 @@ func (p *preparedOutput) send(data []byte, echoAck uint64, send func(protocol.Ou | |||||
| 374 | p.boundary = protocol.UIReceipt{Epoch: p.epoch, State: p.next, ViewPublication: context.Publication, Outcome: protocol.UIReceiptProcessed} | 430 | p.boundary = protocol.UIReceipt{Epoch: p.epoch, State: p.next, ViewPublication: context.Publication, Outcome: protocol.UIReceiptProcessed} | ||
| 375 | } | 431 | } | ||
| 376 | p.stream.next = p.next | 432 | p.stream.next = p.next | ||
| 433 | + | p.stream.chargeBytesLocked(p.next, len(data)) | |||
| 377 | p.stream.publishOutstanding() | 434 | p.stream.publishOutstanding() | ||
| 378 | p.sent = true | 435 | p.sent = true | ||
| 379 | return nil | 436 | return nil | ||
| @@ -510,6 +567,17 @@ func (s *attachmentOutput) sideEffectLocked(data []byte, echoAck uint64) (protoc | |||||
| 510 | return marshalOutputState(data, epoch, 0, 0, echoAck, viewRevision, size, false, nil) | 567 | return marshalOutputState(data, epoch, 0, 0, echoAck, viewRevision, size, false, nil) | ||
| 511 | } | 568 | } | ||
| 512 | 569 | ||||
| 570 | + | // sideEffectSentLocked charges a side effect that reached the transport. It has | |||
| 571 | + | // no state of its own, so the ACK of the next state frame releases it: every | |||
| 572 | + | // state frame is sent after it on the same ordered stream. Caller holds the view | |||
| 573 | + | // lock that built output, so the epoch cannot have moved. | |||
| 574 | + | func (s *attachmentOutput) sideEffectSentLocked(output protocol.Output) { | |||
| 575 | + | if s == nil || output.Epoch != s.currentEpochLocked() { | |||
| 576 | + | return | |||
| 577 | + | } | |||
| 578 | + | s.chargeBytesLocked(s.next+1, len(output.Data)) | |||
| 579 | + | } | |||
| 580 | + | ||||
| 513 | func (s *attachmentOutput) ack(epoch, state uint64) bool { | 581 | func (s *attachmentOutput) ack(epoch, state uint64) bool { | ||
| 514 | if s == nil { | 582 | if s == nil { | ||
| 515 | return false | 583 | return false | ||
| @@ -520,6 +588,7 @@ func (s *attachmentOutput) ack(epoch, state uint64) bool { | |||||
| 520 | return false | 588 | return false | ||
| 521 | } | 589 | } | ||
| 522 | s.acked = state | 590 | s.acked = state | ||
| 591 | + | s.releaseBytesLocked(state) | |||
| 523 | s.publishOutstanding() | 592 | s.publishOutstanding() | ||
| 524 | return true | 593 | return true | ||
| 525 | } | 594 | } | ||
| @@ -556,6 +625,7 @@ func (s *attachmentOutput) rebaseLocked() { | |||||
| 556 | s.epoch = epoch + 1 | 625 | s.epoch = epoch + 1 | ||
| 557 | s.next = 0 | 626 | s.next = 0 | ||
| 558 | s.acked = 0 | 627 | s.acked = 0 | ||
| 628 | + | s.resetBytesLocked() | |||
| 559 | s.generation++ | 629 | s.generation++ | ||
| 560 | s.publishOutstanding() | 630 | s.publishOutstanding() | ||
| 561 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | 631 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | ||
| @@ -591,6 +661,11 @@ func (s *attachmentOutput) setWindow(window uint8) { | |||||
| 591 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | 661 | s.maxOutstandingAtomic.Store(s.maxOutstanding) | ||
| 592 | } | 662 | } | ||
| 593 | 663 | ||||
| 664 | + | // atCapacity reports whether the next state frame must wait for an ACK: the | |||
| 665 | + | // frame window is full, or unacknowledged Output data reached | |||
| 666 | + | // protocol.MaxOutputWindowBytes while a state frame is in flight. With no state | |||
| 667 | + | // frame in flight the next frame is always allowed, since only its ACK can | |||
| 668 | + | // release bytes charged by side effects. | |||
| 594 | func (s *attachmentOutput) atCapacity() bool { | 669 | func (s *attachmentOutput) atCapacity() bool { | ||
| 595 | if s == nil { | 670 | if s == nil { | ||
| 596 | return false | 671 | return false | ||
| @@ -599,7 +674,9 @@ func (s *attachmentOutput) atCapacity() bool { | |||||
| 599 | if maxOutstanding == 0 { | 674 | if maxOutstanding == 0 { | ||
| 600 | maxOutstanding = s.maxOutstanding | 675 | maxOutstanding = s.maxOutstanding | ||
| 601 | } | 676 | } | ||
| 602 | - | return s.outstandingAtomic.Load() >= maxOutstanding | 677 | + | outstanding := s.outstandingAtomic.Load() |
| 678 | + | return outstanding >= maxOutstanding || | |||
| 679 | + | (outstanding > 0 && s.unackedBytesAtomic.Load() >= protocol.MaxOutputWindowBytes) | |||
| 603 | } | 680 | } | ||
| 604 | 681 | ||||
| 605 | // committedState reports the latest committed output state number. It takes | 682 | // committedState reports the latest committed output state number. It takes | ||
M internal/usecase/daemon/attachment_output_test.go +109 -0
| @@ -519,3 +519,112 @@ func TestNormalizeOutputWindow(t *testing.T) { | |||
| 519 | 519 | require.Equal(t, tt.want, normalizeOutputWindow(tt.input)) | |
| 520 | 520 | } | |
| 521 | 521 | } | |
| 522 | + | ||
| 523 | + | // TestAttachmentOutputByteWindow covers the unacknowledged byte budget that | |
| 524 | + | // runs alongside the frame window: it fills on large frames and side effects, | |
| 525 | + | // never blocks the only frame that can release it, and an ACK or a new epoch | |
| 526 | + | // releases exactly what it covers. | |
| 527 | + | func TestAttachmentOutputByteWindow(t *testing.T) { | |
| 528 | + | const big = protocol.MaxOutputWindowBytes / 2 | |
| 529 | + | type step struct { | |
| 530 | + | frame int // state frame of this many data bytes | |
| 531 | + | sideEffect int // side effect of this many data bytes | |
| 532 | + | ack uint64 | |
| 533 | + | rebase bool | |
| 534 | + | } | |
| 535 | + | tests := []struct { | |
| 536 | + | name string | |
| 537 | + | steps []step | |
| 538 | + | wantBytes uint64 | |
| 539 | + | wantCapacity bool | |
| 540 | + | }{ | |
| 541 | + | {name: "small frames stay under the byte window", steps: []step{{frame: 100}, {frame: 100}}, wantBytes: 200}, | |
| 542 | + | {name: "large frames fill the byte window", steps: []step{{frame: big}, {frame: big}}, wantBytes: 2 * big, wantCapacity: true}, | |
| 543 | + | {name: "one oversized frame in flight blocks the next", steps: []step{{frame: protocol.MaxOutputWindowBytes + 1}}, wantBytes: protocol.MaxOutputWindowBytes + 1, wantCapacity: true}, | |
| 544 | + | {name: "side effects alone never block a state frame", steps: []step{{sideEffect: protocol.MaxOutputWindowBytes}}, wantBytes: protocol.MaxOutputWindowBytes}, | |
| 545 | + | {name: "side effects count once a frame is in flight", steps: []step{{frame: 100}, {sideEffect: protocol.MaxOutputWindowBytes}}, wantBytes: protocol.MaxOutputWindowBytes + 100, wantCapacity: true}, | |
| 546 | + | {name: "ack releases the frames it covers", steps: []step{{frame: big}, {frame: big}, {ack: 1}}, wantBytes: big}, | |
| 547 | + | {name: "a side effect is released by the next state's ack", steps: []step{{sideEffect: 50}, {frame: 100}, {ack: 1}}, wantBytes: 0}, | |
| 548 | + | {name: "a new epoch forgets every charge", steps: []step{{frame: big}, {frame: big}, {rebase: true}}, wantBytes: 0}, | |
| 549 | + | } | |
| 550 | + | for _, tt := range tests { | |
| 551 | + | t.Run(tt.name, func(t *testing.T) { | |
| 552 | + | stream := newOutputStateStream() | |
| 553 | + | stream.ack(1, 0) | |
| 554 | + | for _, s := range tt.steps { | |
| 555 | + | switch { | |
| 556 | + | case s.frame > 0: | |
| 557 | + | stream.lockView() | |
| 558 | + | stream.next++ | |
| 559 | + | stream.chargeBytesLocked(stream.next, s.frame) | |
| 560 | + | stream.publishOutstanding() | |
| 561 | + | stream.unlockView() | |
| 562 | + | case s.sideEffect > 0: | |
| 563 | + | out, err := stream.sideEffect(make([]byte, s.sideEffect), 0) | |
| 564 | + | require.NoError(t, err) | |
| 565 | + | stream.lockView() | |
| 566 | + | stream.sideEffectSentLocked(out) | |
| 567 | + | stream.unlockView() | |
| 568 | + | case s.ack > 0: | |
| 569 | + | require.True(t, stream.ack(stream.currentEpoch(), s.ack)) | |
| 570 | + | case s.rebase: | |
| 571 | + | stream.rebase() | |
| 572 | + | } | |
| 573 | + | } | |
| 574 | + | require.Equal(t, tt.wantBytes, stream.unackedBytesAtomic.Load()) | |
| 575 | + | require.Equal(t, tt.wantCapacity, stream.atCapacity()) | |
| 576 | + | }) | |
| 577 | + | } | |
| 578 | + | } | |
| 579 | + | ||
| 580 | + | // TestAttachmentOutputFullByteWindowKeepsImages proves the byte window gates | |
| 581 | + | // when a frame goes out, never what it carries: an upload must fit one frame, | |
| 582 | + | // so trimming images to the remaining byte budget would drop them for good. | |
| 583 | + | func TestAttachmentOutputFullByteWindowKeepsImages(t *testing.T) { | |
| 584 | + | scene := graphics.NewScene(graphics.Limits{}) | |
| 585 | + | asset, err := scene.AddAsset(graphics.AssetBlob{Encoded: []byte("asset"), Width: 1, Height: 1}) | |
| 586 | + | require.NoError(t, err) | |
| 587 | + | _, err = scene.PlaceAsset(asset, graphics.PixelRect{Width: 1, Height: 1}) | |
| 588 | + | require.NoError(t, err) | |
| 589 | + | state := &capturedRenderState{ | |
| 590 | + | route: protocol.CommittedRouteIdentity{Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: "work"}}, | |
| 591 | + | view: attachmentView{tabID: "tab-1"}, | |
| 592 | + | panes: []capturedPaneRenderState{{ | |
| 593 | + | stableID: "pane-1", focused: true, | |
| 594 | + | graphics: scene.Snapshot(), | |
| 595 | + | graphicsGeometry: domain.Geometry{Size: domain.Size{Cols: 1, Rows: 1}, PixelWidth: 1, PixelHeight: 1}, | |
| 596 | + | placement: layout.Placement{Content: domain.Rect{Width: 1, Height: 1}}, | |
| 597 | + | }}} | |
| 598 | + | output := attachmentOutputWithGraphics(newGraphicsOutputState()) | |
| 599 | + | ac := &attachedClient{output: output, terminalCapabilities: terminalcap.Capabilities{KittyGraphics: true}} | |
| 600 | + | output.attachment = ac | |
| 601 | + | // Side effects filled the byte window while no state frame is in flight. | |
| 602 | + | output.lockView() | |
| 603 | + | output.chargeBytesLocked(output.next+1, protocol.MaxOutputWindowBytes) | |
| 604 | + | output.unlockView() | |
| 605 | + | require.False(t, output.atCapacity(), "the only frame that can release side-effect bytes is allowed") | |
| 606 | + | ||
| 607 | + | prepared, err := output.prepareFrame(nil, state, renderer.NewFrame(1, 1), []renderer.Damage{renderer.FullRedraw()}, true, cursorOut{}) | |
| 608 | + | require.NoError(t, err) | |
| 609 | + | require.NotNil(t, prepared.graphics, "a full byte window must not drop the frame's images") | |
| 610 | + | require.NotEmpty(t, prepared.graphics.data) | |
| 611 | + | require.Equal(t, protocol.MaxOutputDataLen, prepared.graphics.maxBytes+len(prepared.data)-len(prepared.graphics.data), | |
| 612 | + | "images keep the whole frame data limit") | |
| 613 | + | } | |
| 614 | + | ||
| 615 | + | func TestAttachmentOutputSentFrameChargesItsBytes(t *testing.T) { | |
| 616 | + | stream := newOutputStateStream() | |
| 617 | + | frame := renderer.NewFrame(3, 1) | |
| 618 | + | fillOutputStateRows(frame, []string{"abc"}) | |
| 619 | + | prepared, err := stream.prepareTestOutput(frame, nil, true) | |
| 620 | + | require.NoError(t, err) | |
| 621 | + | require.NoError(t, prepared.send(prepared.data, 0, outputFrameSender(func(wire.Envelope) error { return nil }))) | |
| 622 | + | require.Equal(t, uint64(len(prepared.data)), stream.unackedBytesAtomic.Load()) | |
| 623 | + | require.True(t, stream.ack(stream.currentEpoch(), stream.next)) | |
| 624 | + | require.Zero(t, stream.unackedBytesAtomic.Load()) | |
| 625 | + | ||
| 626 | + | failed, err := stream.prepareTestOutput(frame, nil, true) | |
| 627 | + | require.NoError(t, err) | |
| 628 | + | require.Error(t, failed.send(failed.data, 0, outputFrameSender(func(wire.Envelope) error { return errors.New("send failed") }))) | |
| 629 | + | require.Zero(t, stream.unackedBytesAtomic.Load(), "a frame that never left must not be charged") | |
| 630 | + | } | |
| @@ -519,3 +519,112 @@ func TestNormalizeOutputWindow(t *testing.T) { | |||||
| 519 | require.Equal(t, tt.want, normalizeOutputWindow(tt.input)) | 519 | require.Equal(t, tt.want, normalizeOutputWindow(tt.input)) | ||
| 520 | } | 520 | } | ||
| 521 | } | 521 | } | ||
| 522 | + | ||||
| 523 | + | // TestAttachmentOutputByteWindow covers the unacknowledged byte budget that | |||
| 524 | + | // runs alongside the frame window: it fills on large frames and side effects, | |||
| 525 | + | // never blocks the only frame that can release it, and an ACK or a new epoch | |||
| 526 | + | // releases exactly what it covers. | |||
| 527 | + | func TestAttachmentOutputByteWindow(t *testing.T) { | |||
| 528 | + | const big = protocol.MaxOutputWindowBytes / 2 | |||
| 529 | + | type step struct { | |||
| 530 | + | frame int // state frame of this many data bytes | |||
| 531 | + | sideEffect int // side effect of this many data bytes | |||
| 532 | + | ack uint64 | |||
| 533 | + | rebase bool | |||
| 534 | + | } | |||
| 535 | + | tests := []struct { | |||
| 536 | + | name string | |||
| 537 | + | steps []step | |||
| 538 | + | wantBytes uint64 | |||
| 539 | + | wantCapacity bool | |||
| 540 | + | }{ | |||
| 541 | + | {name: "small frames stay under the byte window", steps: []step{{frame: 100}, {frame: 100}}, wantBytes: 200}, | |||
| 542 | + | {name: "large frames fill the byte window", steps: []step{{frame: big}, {frame: big}}, wantBytes: 2 * big, wantCapacity: true}, | |||
| 543 | + | {name: "one oversized frame in flight blocks the next", steps: []step{{frame: protocol.MaxOutputWindowBytes + 1}}, wantBytes: protocol.MaxOutputWindowBytes + 1, wantCapacity: true}, | |||
| 544 | + | {name: "side effects alone never block a state frame", steps: []step{{sideEffect: protocol.MaxOutputWindowBytes}}, wantBytes: protocol.MaxOutputWindowBytes}, | |||
| 545 | + | {name: "side effects count once a frame is in flight", steps: []step{{frame: 100}, {sideEffect: protocol.MaxOutputWindowBytes}}, wantBytes: protocol.MaxOutputWindowBytes + 100, wantCapacity: true}, | |||
| 546 | + | {name: "ack releases the frames it covers", steps: []step{{frame: big}, {frame: big}, {ack: 1}}, wantBytes: big}, | |||
| 547 | + | {name: "a side effect is released by the next state's ack", steps: []step{{sideEffect: 50}, {frame: 100}, {ack: 1}}, wantBytes: 0}, | |||
| 548 | + | {name: "a new epoch forgets every charge", steps: []step{{frame: big}, {frame: big}, {rebase: true}}, wantBytes: 0}, | |||
| 549 | + | } | |||
| 550 | + | for _, tt := range tests { | |||
| 551 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 552 | + | stream := newOutputStateStream() | |||
| 553 | + | stream.ack(1, 0) | |||
| 554 | + | for _, s := range tt.steps { | |||
| 555 | + | switch { | |||
| 556 | + | case s.frame > 0: | |||
| 557 | + | stream.lockView() | |||
| 558 | + | stream.next++ | |||
| 559 | + | stream.chargeBytesLocked(stream.next, s.frame) | |||
| 560 | + | stream.publishOutstanding() | |||
| 561 | + | stream.unlockView() | |||
| 562 | + | case s.sideEffect > 0: | |||
| 563 | + | out, err := stream.sideEffect(make([]byte, s.sideEffect), 0) | |||
| 564 | + | require.NoError(t, err) | |||
| 565 | + | stream.lockView() | |||
| 566 | + | stream.sideEffectSentLocked(out) | |||
| 567 | + | stream.unlockView() | |||
| 568 | + | case s.ack > 0: | |||
| 569 | + | require.True(t, stream.ack(stream.currentEpoch(), s.ack)) | |||
| 570 | + | case s.rebase: | |||
| 571 | + | stream.rebase() | |||
| 572 | + | } | |||
| 573 | + | } | |||
| 574 | + | require.Equal(t, tt.wantBytes, stream.unackedBytesAtomic.Load()) | |||
| 575 | + | require.Equal(t, tt.wantCapacity, stream.atCapacity()) | |||
| 576 | + | }) | |||
| 577 | + | } | |||
| 578 | + | } | |||
| 579 | + | ||||
| 580 | + | // TestAttachmentOutputFullByteWindowKeepsImages proves the byte window gates | |||
| 581 | + | // when a frame goes out, never what it carries: an upload must fit one frame, | |||
| 582 | + | // so trimming images to the remaining byte budget would drop them for good. | |||
| 583 | + | func TestAttachmentOutputFullByteWindowKeepsImages(t *testing.T) { | |||
| 584 | + | scene := graphics.NewScene(graphics.Limits{}) | |||
| 585 | + | asset, err := scene.AddAsset(graphics.AssetBlob{Encoded: []byte("asset"), Width: 1, Height: 1}) | |||
| 586 | + | require.NoError(t, err) | |||
| 587 | + | _, err = scene.PlaceAsset(asset, graphics.PixelRect{Width: 1, Height: 1}) | |||
| 588 | + | require.NoError(t, err) | |||
| 589 | + | state := &capturedRenderState{ | |||
| 590 | + | route: protocol.CommittedRouteIdentity{Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: "work"}}, | |||
| 591 | + | view: attachmentView{tabID: "tab-1"}, | |||
| 592 | + | panes: []capturedPaneRenderState{{ | |||
| 593 | + | stableID: "pane-1", focused: true, | |||
| 594 | + | graphics: scene.Snapshot(), | |||
| 595 | + | graphicsGeometry: domain.Geometry{Size: domain.Size{Cols: 1, Rows: 1}, PixelWidth: 1, PixelHeight: 1}, | |||
| 596 | + | placement: layout.Placement{Content: domain.Rect{Width: 1, Height: 1}}, | |||
| 597 | + | }}} | |||
| 598 | + | output := attachmentOutputWithGraphics(newGraphicsOutputState()) | |||
| 599 | + | ac := &attachedClient{output: output, terminalCapabilities: terminalcap.Capabilities{KittyGraphics: true}} | |||
| 600 | + | output.attachment = ac | |||
| 601 | + | // Side effects filled the byte window while no state frame is in flight. | |||
| 602 | + | output.lockView() | |||
| 603 | + | output.chargeBytesLocked(output.next+1, protocol.MaxOutputWindowBytes) | |||
| 604 | + | output.unlockView() | |||
| 605 | + | require.False(t, output.atCapacity(), "the only frame that can release side-effect bytes is allowed") | |||
| 606 | + | ||||
| 607 | + | prepared, err := output.prepareFrame(nil, state, renderer.NewFrame(1, 1), []renderer.Damage{renderer.FullRedraw()}, true, cursorOut{}) | |||
| 608 | + | require.NoError(t, err) | |||
| 609 | + | require.NotNil(t, prepared.graphics, "a full byte window must not drop the frame's images") | |||
| 610 | + | require.NotEmpty(t, prepared.graphics.data) | |||
| 611 | + | require.Equal(t, protocol.MaxOutputDataLen, prepared.graphics.maxBytes+len(prepared.data)-len(prepared.graphics.data), | |||
| 612 | + | "images keep the whole frame data limit") | |||
| 613 | + | } | |||
| 614 | + | ||||
| 615 | + | func TestAttachmentOutputSentFrameChargesItsBytes(t *testing.T) { | |||
| 616 | + | stream := newOutputStateStream() | |||
| 617 | + | frame := renderer.NewFrame(3, 1) | |||
| 618 | + | fillOutputStateRows(frame, []string{"abc"}) | |||
| 619 | + | prepared, err := stream.prepareTestOutput(frame, nil, true) | |||
| 620 | + | require.NoError(t, err) | |||
| 621 | + | require.NoError(t, prepared.send(prepared.data, 0, outputFrameSender(func(wire.Envelope) error { return nil }))) | |||
| 622 | + | require.Equal(t, uint64(len(prepared.data)), stream.unackedBytesAtomic.Load()) | |||
| 623 | + | require.True(t, stream.ack(stream.currentEpoch(), stream.next)) | |||
| 624 | + | require.Zero(t, stream.unackedBytesAtomic.Load()) | |||
| 625 | + | ||||
| 626 | + | failed, err := stream.prepareTestOutput(frame, nil, true) | |||
| 627 | + | require.NoError(t, err) | |||
| 628 | + | require.Error(t, failed.send(failed.data, 0, outputFrameSender(func(wire.Envelope) error { return errors.New("send failed") }))) | |||
| 629 | + | require.Zero(t, stream.unackedBytesAtomic.Load(), "a frame that never left must not be charged") | |||
| 630 | + | } | |||
M internal/usecase/daemon/client.go +8 -1
| @@ -591,6 +591,9 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||
| 591 | 591 | if err == nil { | |
| 592 | 592 | err = expected.transport.SendOutputSynchronous(output) | |
| 593 | 593 | } | |
| 594 | + | if err == nil { | |
| 595 | + | ac.output.sideEffectSentLocked(output) | |
| 596 | + | } | |
| 594 | 597 | return expected.transport, err | |
| 595 | 598 | } | |
| 596 | 599 | return d.boundedSendWith(expected.transport, func() error { | |
| @@ -605,7 +608,11 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||
| 605 | 608 | if err != nil { | |
| 606 | 609 | return err | |
| 607 | 610 | } | |
| 608 | - | return expected.transport.SendOutput(output) | |
| 611 | + | if err := expected.transport.SendOutput(output); err != nil { | |
| 612 | + | return err | |
| 613 | + | } | |
| 614 | + | ac.output.sideEffectSentLocked(output) | |
| 615 | + | return nil | |
| 609 | 616 | }) | |
| 610 | 617 | } | |
| 611 | 618 | ||
| @@ -591,6 +591,9 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||||
| 591 | if err == nil { | 591 | if err == nil { | ||
| 592 | err = expected.transport.SendOutputSynchronous(output) | 592 | err = expected.transport.SendOutputSynchronous(output) | ||
| 593 | } | 593 | } | ||
| 594 | + | if err == nil { | |||
| 595 | + | ac.output.sideEffectSentLocked(output) | |||
| 596 | + | } | |||
| 594 | return expected.transport, err | 597 | return expected.transport, err | ||
| 595 | } | 598 | } | ||
| 596 | return d.boundedSendWith(expected.transport, func() error { | 599 | return d.boundedSendWith(expected.transport, func() error { | ||
| @@ -605,7 +608,11 @@ func (d *Daemon) boundedSendOutputErrTransport(ac *attachedClient, b []byte) (po | |||||
| 605 | if err != nil { | 608 | if err != nil { | ||
| 606 | return err | 609 | return err | ||
| 607 | } | 610 | } | ||
| 608 | - | return expected.transport.SendOutput(output) | 611 | + | if err := expected.transport.SendOutput(output); err != nil { |
| 612 | + | return err | |||
| 613 | + | } | |||
| 614 | + | ac.output.sideEffectSentLocked(output) | |||
| 615 | + | return nil | |||
| 609 | }) | 616 | }) | ||
| 610 | } | 617 | } | ||
| 611 | 618 | ||||
M internal/usecase/daemon/clipboard.go +3 -0
| @@ -293,6 +293,9 @@ func (d *Daemon) boundedSendClipboardForward(item clipboardForward, ticket *atta | |||
| 293 | 293 | err = expected.transport.SendOutput(output) | |
| 294 | 294 | } | |
| 295 | 295 | } | |
| 296 | + | if err == nil { | |
| 297 | + | ac.output.sideEffectSentLocked(output) | |
| 298 | + | } | |
| 296 | 299 | if err != nil { | |
| 297 | 300 | ticket.reportTransportFailure(expected) | |
| 298 | 301 | } | |
| @@ -293,6 +293,9 @@ func (d *Daemon) boundedSendClipboardForward(item clipboardForward, ticket *atta | |||||
| 293 | err = expected.transport.SendOutput(output) | 293 | err = expected.transport.SendOutput(output) | ||
| 294 | } | 294 | } | ||
| 295 | } | 295 | } | ||
| 296 | + | if err == nil { | |||
| 297 | + | ac.output.sideEffectSentLocked(output) | |||
| 298 | + | } | |||
| 296 | if err != nil { | 299 | if err != nil { | ||
| 297 | ticket.reportTransportFailure(expected) | 300 | ticket.reportTransportFailure(expected) | ||
| 298 | } | 301 | } | ||
M internal/usecase/daemon/graphics_output.go +3 -0
| @@ -943,6 +943,9 @@ func (d *Daemon) cleanupAttachmentOutput(ac *attachedClient) error { | |||
| 943 | 943 | _ = ac.closeCapturedTransport(expected.transport) | |
| 944 | 944 | } | |
| 945 | 945 | } | |
| 946 | + | if sendErr == nil { | |
| 947 | + | ac.output.sideEffectSentLocked(output) | |
| 948 | + | } | |
| 946 | 949 | ac.output.unlockView() | |
| 947 | 950 | if sendErr == nil { | |
| 948 | 951 | prepared.commit() | |
| @@ -943,6 +943,9 @@ func (d *Daemon) cleanupAttachmentOutput(ac *attachedClient) error { | |||||
| 943 | _ = ac.closeCapturedTransport(expected.transport) | 943 | _ = ac.closeCapturedTransport(expected.transport) | ||
| 944 | } | 944 | } | ||
| 945 | } | 945 | } | ||
| 946 | + | if sendErr == nil { | |||
| 947 | + | ac.output.sideEffectSentLocked(output) | |||
| 948 | + | } | |||
| 946 | ac.output.unlockView() | 949 | ac.output.unlockView() | ||
| 947 | if sendErr == nil { | 950 | if sendErr == nil { | ||
| 948 | prepared.commit() | 951 | prepared.commit() | ||