Skip to content

Commit

Permalink
*: Fix data race between getting labels and setting labels in config (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot committed Jul 26, 2023
1 parent ff34755 commit 5a5cad1
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 5 deletions.
13 changes: 8 additions & 5 deletions server/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2207,12 +2207,15 @@ func (h labelHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {

if len(labels) > 0 {
cfg := *config.GetGlobalConfig()
if cfg.Labels == nil {
cfg.Labels = make(map[string]string, len(labels))
}
for k, v := range labels {
cfg.Labels[k] = v
// Be careful of data race. The key & value of cfg.Labels must not be changed.
if cfg.Labels != nil {
for k, v := range cfg.Labels {
if _, found := labels[k]; !found {
labels[k] = v
}
}
}
cfg.Labels = labels
config.StoreGlobalConfig(&cfg)
logutil.BgLogger().Info("update server labels", zap.Any("labels", cfg.Labels))
}
Expand Down
42 changes: 42 additions & 0 deletions server/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"encoding/json"
"fmt"
"io"
"math/rand"
"net"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -1197,3 +1198,44 @@ func TestSetLabels(t *testing.T) {
// reset the global variable
config.GetGlobalConfig().Labels = map[string]string{}
}

func TestSetLabelsConcurrentWithGetLabel(t *testing.T) {
ts := createBasicHTTPHandlerTestSuite()

ts.startServer(t)
defer ts.stopServer(t)

testUpdateLabels := func() {
labels := map[string]string{}
labels["zone"] = fmt.Sprintf("z-%v", rand.Intn(100000))
buffer := bytes.NewBuffer([]byte{})
require.Nil(t, json.NewEncoder(buffer).Encode(labels))
resp, err := ts.postStatus("/labels", "application/json", buffer)
require.NoError(t, err)
require.NotNil(t, resp)
defer func() {
require.NoError(t, resp.Body.Close())
}()
require.Equal(t, http.StatusOK, resp.StatusCode)
newLabels := config.GetGlobalConfig().Labels
require.Equal(t, newLabels, labels)
}
done := make(chan struct{})
go func() {
for {
select {
case <-done:
return
default:
config.GetGlobalConfig().GetTiKVConfig()
}
}
}()
for i := 0; i < 100; i++ {
testUpdateLabels()
}
close(done)

// reset the global variable
config.GetGlobalConfig().Labels = map[string]string{}
}

0 comments on commit 5a5cad1

Please sign in to comment.