mirror of
https://github.com/coder/coder.git
synced 2026-11-08 05:32:55 +08:00
## Problem Subscribers connecting to a different replica than the one running the chat see full messages appear but no streaming partials (`message_part` events). The relay mechanism that forwards ephemeral parts across replicas had several bugs. ## Root Causes 1. **`openRelay()` blocked the event loop** — The WebSocket dial (TCP + TLS + HTTP upgrade) to the worker replica ran synchronously inside the select loop. While dialing, no events could be processed, channels filled up, and parts were silently dropped. 2. **Relay drops were permanent** — When the relay WebSocket closed mid-stream, `relayParts` was set to nil and never reopened. No status notification would re-trigger it since the chat was still running on the same worker. 3. **`drainInitial` snapshot race** — The `default` case in the initial drain loop caused the snapshot to be empty if the remote hadn't flushed data yet (common immediately after WebSocket connect). 4. **Duplicate event delivery** — The `preloaded` slice caused snapshot events to be sent both in the return value and re-sent through the channel goroutine. ## Fixes ### `coderd/chatd/chatd.go` (Subscribe method) - **Async relay dial**: `openRelayAsync()` spawns a goroutine to dial the remote replica. The result (channel + cancel func) is delivered on a `relayReadyCh` channel that the select loop reads without blocking. - **Relay reconnection**: When the relay channel closes, a 500ms timer fires. The handler re-checks chat status from the DB and reopens the relay if the chat is still running on a remote worker. - **Snapshot parts via channel**: Relay snapshot + live parts are wrapped into a single channel so they flow through the same path, avoiding races with the select loop. ### `enterprise/coderd/chats.go` (newRemotePartsProvider) - **Timer-based drain**: Replaced `default` with a 1-second timer. After the first event, `Reset(0)` switches to non-blocking drain for remaining buffered events. - **Remove preloaded duplication**: The goroutine now only forwards new events; snapshot events are returned to the caller directly. ## Testing All existing tests pass: - `TestInterruptChatBroadcastsStatusAcrossInstances` - `TestSubscribeSnapshotIncludesStatusEvent` - `TestSubscribeNoPubsubNoDuplicateMessageParts` - `TestSubscribeAfterMessageID` - `TestChatStreamRelay/RelayMessagePartsAcrossReplicas`
178 lines
4.3 KiB
Go
178 lines
4.3 KiB
Go
package coderd
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/url"
|
|
"time"
|
|
|
|
"github.com/google/uuid"
|
|
"golang.org/x/xerrors"
|
|
|
|
"github.com/coder/coder/v2/coderd/chatd"
|
|
"github.com/coder/coder/v2/codersdk"
|
|
"github.com/coder/websocket"
|
|
)
|
|
|
|
// RelaySourceHeader marks replica-relayed stream requests.
|
|
const RelaySourceHeader = "X-Coder-Relay-Source-Replica"
|
|
|
|
const (
|
|
authorizationHeader = "Authorization"
|
|
cookieHeader = "Cookie"
|
|
)
|
|
|
|
// newRemotePartsProvider creates a RemotePartsProvider that dials a remote
|
|
// replica's stream endpoint to fetch message_part events. It filters to only
|
|
// forward message_part events since durable events come via pubsub.
|
|
func newRemotePartsProvider(
|
|
resolveReplicaAddress func(context.Context, uuid.UUID) (string, bool),
|
|
replicaHTTPClient *http.Client,
|
|
replicaID uuid.UUID,
|
|
) chatd.RemotePartsProvider {
|
|
return func(
|
|
ctx context.Context,
|
|
chatID uuid.UUID,
|
|
workerID uuid.UUID,
|
|
requestHeader http.Header,
|
|
) (
|
|
[]codersdk.ChatStreamEvent,
|
|
<-chan codersdk.ChatStreamEvent,
|
|
func(),
|
|
error,
|
|
) {
|
|
address, ok := resolveReplicaAddress(ctx, workerID)
|
|
if !ok {
|
|
return nil, nil, nil, xerrors.New("worker replica not found")
|
|
}
|
|
|
|
baseURL, err := url.Parse(address)
|
|
if err != nil {
|
|
return nil, nil, nil, xerrors.Errorf("parse relay address %q: %w", address, err)
|
|
}
|
|
relayCtx, relayCancel := context.WithCancel(ctx)
|
|
sdkClient := codersdk.New(baseURL)
|
|
sdkClient.HTTPClient = replicaHTTPClient
|
|
sdkClient.SessionTokenProvider = relayHeaderTokenProvider{
|
|
header: relayHeaders(requestHeader, replicaID),
|
|
}
|
|
sourceEvents, sourceStream, err := sdkClient.StreamChat(relayCtx, chatID)
|
|
if err != nil {
|
|
relayCancel()
|
|
return nil, nil, nil, xerrors.Errorf("dial relay stream: %w", err)
|
|
}
|
|
|
|
snapshot := make([]codersdk.ChatStreamEvent, 0, 100)
|
|
|
|
// Wait briefly for the first event to handle the common
|
|
// case where the remote side has buffered parts but hasn't
|
|
// flushed them to the WebSocket yet.
|
|
const drainTimeout = time.Second
|
|
drainTimer := time.NewTimer(drainTimeout)
|
|
defer drainTimer.Stop()
|
|
|
|
drainInitial:
|
|
for len(snapshot) < cap(snapshot) {
|
|
select {
|
|
case <-relayCtx.Done():
|
|
_ = sourceStream.Close()
|
|
relayCancel()
|
|
return nil, nil, nil, xerrors.Errorf("dial relay stream: %w", relayCtx.Err())
|
|
case event, ok := <-sourceEvents:
|
|
if !ok {
|
|
break drainInitial
|
|
}
|
|
if event.Type != codersdk.ChatStreamEventTypeMessagePart {
|
|
continue
|
|
}
|
|
snapshot = append(snapshot, event)
|
|
// After getting the first event, switch to
|
|
// non-blocking drain for remaining buffered events.
|
|
drainTimer.Stop()
|
|
drainTimer.Reset(0)
|
|
case <-drainTimer.C:
|
|
break drainInitial
|
|
}
|
|
}
|
|
|
|
events := make(chan codersdk.ChatStreamEvent, 128)
|
|
|
|
go func() {
|
|
defer close(events)
|
|
defer relayCancel()
|
|
defer func() {
|
|
_ = sourceStream.Close()
|
|
}()
|
|
|
|
// No need to re-send snapshot events — they're
|
|
// returned to the caller directly.
|
|
for {
|
|
select {
|
|
case <-relayCtx.Done():
|
|
return
|
|
case event, ok := <-sourceEvents:
|
|
if !ok {
|
|
return
|
|
}
|
|
if event.Type != codersdk.ChatStreamEventTypeMessagePart {
|
|
continue
|
|
}
|
|
select {
|
|
case events <- event:
|
|
case <-relayCtx.Done():
|
|
return
|
|
}
|
|
}
|
|
}
|
|
}()
|
|
|
|
cancel := func() {
|
|
relayCancel()
|
|
_ = sourceStream.Close()
|
|
}
|
|
return snapshot, events, cancel, nil
|
|
}
|
|
}
|
|
|
|
type relayHeaderTokenProvider struct {
|
|
header http.Header
|
|
}
|
|
|
|
func (p relayHeaderTokenProvider) AsRequestOption() codersdk.RequestOption {
|
|
return func(req *http.Request) {
|
|
for key, values := range p.header {
|
|
for _, value := range values {
|
|
req.Header.Add(key, value)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
func (p relayHeaderTokenProvider) SetDialOption(opts *websocket.DialOptions) {
|
|
if opts.HTTPHeader == nil {
|
|
opts.HTTPHeader = make(http.Header)
|
|
}
|
|
for key, values := range p.header {
|
|
for _, value := range values {
|
|
opts.HTTPHeader.Add(key, value)
|
|
}
|
|
}
|
|
}
|
|
|
|
func (p relayHeaderTokenProvider) GetSessionToken() string {
|
|
return p.header.Get(codersdk.SessionTokenHeader)
|
|
}
|
|
|
|
func relayHeaders(source http.Header, replicaID uuid.UUID) http.Header {
|
|
header := make(http.Header)
|
|
if source != nil {
|
|
for _, key := range []string{codersdk.SessionTokenHeader, authorizationHeader, cookieHeader} {
|
|
for _, value := range source.Values(key) {
|
|
header.Add(key, value)
|
|
}
|
|
}
|
|
}
|
|
header.Set(RelaySourceHeader, replicaID.String())
|
|
return header
|
|
}
|