e39569b358
* MM-23800: remove goroutineID and stack printing Each hub has a goroutineID which is calculated with a known hack. The FAQ clearly explains why goroutines don't have an id: https://golang.org/doc/faq#no_goroutine_id. We only added that because sometimes the hub would be deadlocked and having the goroutineID would be useful when getting the stack trace. This is also problematic in stress tests because the hubs would frequently get overloaded and the logs would unnecessarily have stack traces. But that was in the past, and we have done extensive testing with load tests and fuzz testing to smooth any rough edges remaining. Including adding additional metrics for hub buffer size. Monitoring the metrics is a better way to approach this problem. Therefore, we remove these kludges from the code. * Also remove deadlock checking code There is no need for that anymore since we are getting rid of the stack printing anyways. Let's do a wholesale refactor and clean up the codebase. * MM-23805: Refactor web_hub This is a beginning of the refactoring of the websocket code. To start off with, we unexport some methods and constants which did not need to be exported. There are more remaining but some are out of scope for this PR. The main chunk of refactor is to unexport the webconn send channel which was the main cause of panics. Since we were directly sending to the connection from various parts of the codebase, it would be possible that the send channel would be closed and we could still send a message. This would crash the server. To fix this, we refactor the code to centralize all sending from the main hub goroutine. This means we can leverage the connections map to check if the connection exists or not, and only then send the message. We also move the cluster calls to cluster.go. * bring back cluster code inside hub * Incorporate review comments * Address review comments * rename index * MM-23807: Refactor web_conn - Unexport some struct fields and constants which are not necessary to be accessed from outside the package. This will help us moving the entire websocket handling code to a separate package later. - Change some empty string checks to check for empty string rather than doing a len check which is more idiomatic. Both of them compile to the same code. So it doesn't make a difference performance-wise. - Remove redundant ToJson calls to get the length. - Incorporate review comments - Unexport some more methods * Fix field name * Run make app-layers * Add note on hub check
109 строки
2.7 KiB
Go
109 строки
2.7 KiB
Go
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
// See LICENSE.txt for license information.
|
|
|
|
package app
|
|
|
|
import (
|
|
"net"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/gorilla/websocket"
|
|
goi18n "github.com/mattermost/go-i18n/i18n"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/mattermost/mattermost-server/v5/model"
|
|
)
|
|
|
|
func dummyWebsocketHandler(t *testing.T) http.HandlerFunc {
|
|
return func(w http.ResponseWriter, req *http.Request) {
|
|
upgrader := &websocket.Upgrader{
|
|
ReadBufferSize: 1024,
|
|
WriteBufferSize: 1024,
|
|
}
|
|
conn, err := upgrader.Upgrade(w, req, nil)
|
|
for err == nil {
|
|
_, _, err = conn.ReadMessage()
|
|
}
|
|
if _, ok := err.(*websocket.CloseError); !ok {
|
|
require.NoError(t, err)
|
|
}
|
|
}
|
|
}
|
|
|
|
func registerDummyWebConn(t *testing.T, a *App, addr net.Addr, userId string) *WebConn {
|
|
session, appErr := a.CreateSession(&model.Session{
|
|
UserId: userId,
|
|
})
|
|
require.Nil(t, appErr)
|
|
|
|
d := websocket.Dialer{}
|
|
c, _, err := d.Dial("ws://"+addr.String()+"/ws", nil)
|
|
require.NoError(t, err)
|
|
|
|
wc := a.NewWebConn(c, *session, goi18n.IdentityTfunc(), "en")
|
|
a.HubRegister(wc)
|
|
go wc.Pump()
|
|
return wc
|
|
}
|
|
|
|
func TestHubStopWithMultipleConnections(t *testing.T) {
|
|
th := Setup(t).InitBasic()
|
|
defer th.TearDown()
|
|
|
|
s := httptest.NewServer(dummyWebsocketHandler(t))
|
|
defer s.Close()
|
|
|
|
th.App.HubStart()
|
|
wc1 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
wc2 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
wc3 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
defer wc1.Close()
|
|
defer wc2.Close()
|
|
defer wc3.Close()
|
|
}
|
|
|
|
// TestHubStopRaceCondition verifies that attempts to use the hub after it has shutdown does not
|
|
// block the caller indefinitely.
|
|
func TestHubStopRaceCondition(t *testing.T) {
|
|
th := Setup(t).InitBasic()
|
|
defer th.TearDown()
|
|
|
|
s := httptest.NewServer(dummyWebsocketHandler(t))
|
|
|
|
th.App.HubStart()
|
|
wc1 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
defer wc1.Close()
|
|
|
|
hub := th.App.Srv().GetHubs()[0]
|
|
th.App.HubStop()
|
|
time.Sleep(5 * time.Second)
|
|
|
|
done := make(chan bool)
|
|
go func() {
|
|
wc4 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
wc5 := registerDummyWebConn(t, th.App, s.Listener.Addr(), th.BasicUser.Id)
|
|
hub.Register(wc4)
|
|
hub.Register(wc5)
|
|
|
|
hub.UpdateActivity("userId", "sessionToken", 0)
|
|
|
|
for i := 0; i <= broadcastQueueSize; i++ {
|
|
hub.Broadcast(model.NewWebSocketEvent("", "", "", "", nil))
|
|
}
|
|
|
|
hub.InvalidateUser("userId")
|
|
hub.Unregister(wc4)
|
|
hub.Unregister(wc5)
|
|
close(done)
|
|
}()
|
|
|
|
select {
|
|
case <-done:
|
|
case <-time.After(15 * time.Second):
|
|
require.FailNow(t, "hub call did not return within 15 seconds after stop")
|
|
}
|
|
}
|