From a009ba710ed30448973ae4ce61564a00a248c0db Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 22 Jul 2025 18:48:46 +0000 Subject: [PATCH 1/4] handle conflict in postFile Signed-off-by: Callum Styan --- coderd/files.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/coderd/files.go b/coderd/files.go index f82d1aa926c22..0a7ba2be0cb33 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -118,11 +118,22 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { Data: data, }) if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error saving file.", - Detail: err.Error(), - }) - return + if database.IsUniqueViolation(err, database.UniqueFilesHashCreatedByKey) { + // The file was uploaded by some concurrent process since the last time we checked for it, fetch it again. + file, err = api.Database.GetFileByHashAndCreator(ctx, database.GetFileByHashAndCreatorParams{ + Hash: hash, + CreatedBy: apiKey.UserID, + }) + } + // At this point the first error was either not the UniqueViolation OR there's still an error even after we + // attempt to fetch the file again, so we should return here. + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error saving file.", + Detail: err.Error(), + }) + return + } } httpapi.Write(ctx, rw, http.StatusCreated, codersdk.UploadResponse{ From df947efe57e6bf66b36ff741d27979120a5190cf Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 22 Jul 2025 20:07:55 +0000 Subject: [PATCH 2/4] add test to ensure that concurrent file uploads don't result in an error Signed-off-by: Callum Styan --- coderd/files_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/coderd/files_test.go b/coderd/files_test.go index 974db6b18fc69..4cace81d31f91 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "net/http" + "sync" "testing" "github.com/google/uuid" @@ -69,6 +70,34 @@ func TestPostFiles(t *testing.T) { _, err = client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data)) require.NoError(t, err) }) + t.Run("InsertConcurrent", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + var wg sync.WaitGroup + var end sync.WaitGroup + wg.Add(1) + end.Add(3) + for range 3 { + go func() { + wg.Wait() + data := make([]byte, 1024) + _, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data)) + end.Done() + require.NoError(t, err) + }() + + } + wg.Done() + end.Wait() + + // _, err = client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data)) + // require.NoError(t, err) + }) } func TestDownload(t *testing.T) { From f6ae297c1f4f7303d24693c969dc4ab97dbf0b09 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 22 Jul 2025 21:40:51 +0000 Subject: [PATCH 3/4] fix linting Signed-off-by: Callum Styan --- coderd/files_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/coderd/files_test.go b/coderd/files_test.go index 4cace81d31f91..fb13cb30e48f1 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -90,13 +90,9 @@ func TestPostFiles(t *testing.T) { end.Done() require.NoError(t, err) }() - } wg.Done() end.Wait() - - // _, err = client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(data)) - // require.NoError(t, err) }) } From b71c58a056b0fd53edf00fb57d997b6344d466dc Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 29 Jul 2025 21:18:10 +0000 Subject: [PATCH 4/4] add log line if we hit the unique violation error case Signed-off-by: Callum Styan --- coderd/files.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/files.go b/coderd/files.go index 0a7ba2be0cb33..eaab00c401481 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -124,6 +124,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { Hash: hash, CreatedBy: apiKey.UserID, }) + 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)) } // At this point the first error was either not the UniqueViolation OR there's still an error even after we // attempt to fetch the file again, so we should return here.