Skip to content

Commit d736af1

Browse files
authored
fix: handle potential DB conflict due to concurrent upload requests in postFile (#19005)
This issue manifests when users have multiple templates which rely on the same files, for example see: #17442 --------- Signed-off-by: Callum Styan <callumstyan@gmail.com>
1 parent ffbfaf2 commit d736af1

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

coderd/files.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,23 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
118118
Data: data,
119119
})
120120
if err != nil {
121-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
122-
Message: "Internal error saving file.",
123-
Detail: err.Error(),
124-
})
125-
return
121+
if database.IsUniqueViolation(err, database.UniqueFilesHashCreatedByKey) {
122+
// The file was uploaded by some concurrent process since the last time we checked for it, fetch it again.
123+
file, err = api.Database.GetFileByHashAndCreator(ctx, database.GetFileByHashAndCreatorParams{
124+
Hash: hash,
125+
CreatedBy: apiKey.UserID,
126+
})
127+
api.Logger.Info(ctx, "postFile handler hit UniqueViolation trying to upload file after already checking for the file existence", slog.F("hash", hash), slog.F("created_by_id", apiKey.UserID))
128+
}
129+
// At this point the first error was either not the UniqueViolation OR there's still an error even after we
130+
// attempt to fetch the file again, so we should return here.
131+
if err != nil {
132+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
133+
Message: "Internal error saving file.",
134+
Detail: err.Error(),
135+
})
136+
return
137+
}
126138
}
127139

128140
httpapi.Write(ctx, rw, http.StatusCreated, codersdk.UploadResponse{

coderd/files_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"bytes"
66
"context"
77
"net/http"
8+
"sync"
89
"testing"
910

1011
"github.com/google/uuid"
@@ -69,6 +70,30 @@ func TestPostFiles(t *testing.T) {
6970
_, err = client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data))
7071
require.NoError(t, err)
7172
})
73+
t.Run("InsertConcurrent", func(t *testing.T) {
74+
t.Parallel()
75+
client := coderdtest.New(t, nil)
76+
_ = coderdtest.CreateFirstUser(t, client)
77+
78+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
79+
defer cancel()
80+
81+
var wg sync.WaitGroup
82+
var end sync.WaitGroup
83+
wg.Add(1)
84+
end.Add(3)
85+
for range 3 {
86+
go func() {
87+
wg.Wait()
88+
data := make([]byte, 1024)
89+
_, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data))
90+
end.Done()
91+
require.NoError(t, err)
92+
}()
93+
}
94+
wg.Done()
95+
end.Wait()
96+
})
7297
}
7398

7499
func TestDownload(t *testing.T) {

0 commit comments

Comments
 (0)