fix: preserve cross-pod message order

server/cluster.go

@@ -489,14 +489,16 @@ func clusterReqShard(msg *ClusterReq, count int) int {
 	return int(h.Sum32() % uint32(count))
 }
 
-// masterToProxyAsync forwards response from topic master to topic proxy
-// in a fire-and-forget manner.
-func (n *ClusterNode) masterToProxyAsync(msg *ClusterResp) error {
-	var unused bool
-	if c := n.callAsync("Cluster.TopicProxy", msg, &unused, nil); c.Error != nil {
-		return c.Error
+// masterToProxy forwards responses synchronously. A multiplexing session drains
+// one topic in order, so waiting for each RPC prevents the RPC server from
+// concurrently enqueueing later sequence IDs before earlier ones.
+func (n *ClusterNode) masterToProxy(msg *ClusterResp) error {
+	var rejected bool
+	err := n.call("Cluster.TopicProxy", msg, &rejected)
+	if err == nil && rejected {
+		return errClusterOverloaded
 	}
-	return nil
+	return err
 }
 
 // route routes server message within the cluster.
@@ -1437,7 +1439,7 @@ func (sess *Session) clusterWriteLoop(forTopic string) {
 			srvMsg.RcptTo = forTopic
 			response.RcptTo = forTopic
 
-			if err := sess.clnode.masterToProxyAsync(response); err != nil {
+			if err := sess.clnode.masterToProxy(response); err != nil {
 				logs.Warn.Printf("cluster: response to proxy failed \"%s\": %s", sess.sid, err.Error())
 				return
 			}
@@ -1454,6 +1456,12 @@ func (sess *Session) clusterWriteLoop(forTopic string) {
 		case <-sess.detach:
 			return
 		default:
+			sess.clusterWriteScheduled.Store(false)
+			// An enqueue can race with releasing ownership. Either the enqueuer
+			// schedules the next drain or this check does; the CAS prevents both.
+			if len(sess.send) > 0 {
+				sess.scheduleClusterWriteLoop()
+			}
 			terminate = false
 			return
 		}

server/cluster_order_test.go

@@ -0,0 +1,85 @@
+package main
+
+import (
+	"net"
+	"net/rpc"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/tinode/chat/server/concurrency"
+	"github.com/tinode/chat/server/store/types"
+)
+
+type orderedTopicProxy struct {
+	mu       sync.Mutex
+	received []int64
+	done     chan struct{}
+}
+
+func (proxy *orderedTopicProxy) TopicProxy(msg *ClusterResp, rejected *bool) error {
+	if msg.SrvMsg.Data.SeqId == 1 {
+		time.Sleep(50 * time.Millisecond)
+	}
+	proxy.mu.Lock()
+	proxy.received = append(proxy.received, msg.SrvMsg.Data.SeqId)
+	if len(proxy.received) == 2 {
+		close(proxy.done)
+	}
+	proxy.mu.Unlock()
+	return nil
+}
+
+func TestClusterWriteLoopPreservesTopicOrder(t *testing.T) {
+	serverConn, clientConn := net.Pipe()
+	rpcServer := rpc.NewServer()
+	proxy := &orderedTopicProxy{done: make(chan struct{})}
+	if err := rpcServer.RegisterName("Cluster", proxy); err != nil {
+		t.Fatal(err)
+	}
+	go rpcServer.ServeConn(serverConn)
+
+	pool := concurrency.NewGoRoutinePool(4)
+	previousCluster := globals.cluster
+	globals.cluster = &Cluster{proxyEventQueue: pool}
+	t.Cleanup(func() {
+		globals.cluster = previousCluster
+		_ = clientConn.Close()
+		_ = serverConn.Close()
+		pool.Stop()
+	})
+
+	sess := &Session{
+		sid:          "multiplex-order-test",
+		proto:        MULTIPLEX,
+		proxiedTopic: "p2p-order-test",
+		send:         make(chan any, 2),
+		clnode:       &ClusterNode{endpoint: rpc.NewClient(clientConn), connected: true},
+	}
+	tenantID := types.TenantID(1)
+	sess.send <- &ServerComMessage{TenantID: tenantID, Data: &MsgServerData{SeqId: 1}}
+	sess.send <- &ServerComMessage{TenantID: tenantID, Data: &MsgServerData{SeqId: 2}}
+
+	// Scheduling twice used to start two concurrent drains for the same topic.
+	sess.scheduleClusterWriteLoop()
+	sess.scheduleClusterWriteLoop()
+
+	select {
+	case <-proxy.done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("timed out waiting for cluster responses")
+	}
+	deadline := time.Now().Add(2 * time.Second)
+	for sess.clusterWriteScheduled.Load() && time.Now().Before(deadline) {
+		time.Sleep(time.Millisecond)
+	}
+	if sess.clusterWriteScheduled.Load() {
+		t.Fatal("cluster write task did not release session ownership")
+	}
+
+	proxy.mu.Lock()
+	defer proxy.mu.Unlock()
+	if len(proxy.received) != 2 || proxy.received[0] != 1 || proxy.received[1] != 2 {
+		t.Fatalf("cluster responses arrived out of order: %v", proxy.received)
+	}
+}

server/session.go

@@ -168,6 +168,9 @@ type Session struct {
 
 	// Type of proxy to master request being handled.
 	proxyReq ProxyReqType
+	// Ensures that one multiplexing session is drained by at most one cluster
+	// write task at a time, preserving per-topic response order.
+	clusterWriteScheduled atomic.Bool
 }
 
 // Subscription is a mapper of sessions to topics.
@@ -332,7 +335,7 @@ func (s *Session) isCluster() bool {
 }
 
 func (s *Session) scheduleClusterWriteLoop() {
-	if globals.cluster != nil && globals.cluster.proxyEventQueue != nil {
+	if globals.cluster != nil && globals.cluster.proxyEventQueue != nil && s.clusterWriteScheduled.CompareAndSwap(false, true) {
 		globals.cluster.proxyEventQueue.Schedule(
 			func() { s.clusterWriteLoop(s.proxiedTopic) })
 	}