Skip to content

Commit 3467fa8

Browse files
committed
fix(generic-worker): remove redundant file copy during artifact upload process
1 parent eff03ad commit 3467fa8

File tree

4 files changed

+31
-54
lines changed

4 files changed

+31
-54
lines changed

changelog/issue-7368.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
audience: worker-deployers
2+
level: patch
3+
reference: issue 7368
4+
---
5+
Generic Worker: increase performance of artifact uploads by removing a redundant file copy operation.

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ require (
1414
github.com/elastic/go-sysinfo v1.15.3
1515
github.com/fatih/camelcase v1.0.0
1616
github.com/getsentry/raven-go v0.2.0
17-
github.com/gofrs/flock v0.12.1
1817
github.com/golang-jwt/jwt/v4 v4.5.2
1918
github.com/gorilla/mux v1.8.1
2019
github.com/gorilla/websocket v1.5.3
@@ -66,7 +65,7 @@ require (
6665
github.com/inconshreveable/mousetrap v1.1.0 // indirect
6766
github.com/klauspost/compress v1.16.7 // indirect
6867
github.com/klauspost/pgzip v1.2.6 // indirect
69-
github.com/kr/text v0.2.0 // indirect
68+
github.com/kr/pretty v0.3.1 // indirect
7069
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
7170
github.com/nwaples/rardecode v1.1.3 // indirect
7271
github.com/pierrec/lz4/v4 v4.1.18 // indirect
@@ -85,6 +84,7 @@ require (
8584
golang.org/x/mod v0.24.0 // indirect
8685
golang.org/x/sync v0.13.0 // indirect
8786
golang.org/x/text v0.24.0 // indirect
87+
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
8888
howett.net/plist v1.0.0 // indirect
8989
launchpad.net/gocheck v0.0.0-20140225173054-000000000087 // indirect
9090
)

go.sum

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2
9595
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
9696
github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY=
9797
github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0=
98-
github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E=
99-
github.com/gofrs/flock v0.12.1/go.mod h1:9zxTsyu5xtJ9DK+1tFZyibEV7y3uwDxPPfbxeeHCoD0=
10098
github.com/golang-jwt/jwt/v4 v4.5.2 h1:YtQM7lnr8iZ+j5q71MGKkNw9Mn7AjHM68uc9g5fXeUI=
10199
github.com/golang-jwt/jwt/v4 v4.5.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
102100
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
@@ -192,6 +190,7 @@ github.com/klauspost/pgzip v1.2.6 h1:8RXeL5crjEUFnR2/Sn6GJNWtSQ3Dk8pq4CL3jvdDyjU
192190
github.com/klauspost/pgzip v1.2.6/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
193191
github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg=
194192
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
193+
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
195194
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
196195
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
197196
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
@@ -224,6 +223,7 @@ github.com/pierrec/lz4/v4 v4.1.18 h1:xaKrnTkyoqfh1YItXl56+6KJNVYWlEEPuAQW9xsplYQ
224223
github.com/pierrec/lz4/v4 v4.1.18/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
225224
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
226225
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
226+
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
227227
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
228228
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
229229
github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg=
@@ -235,6 +235,7 @@ github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:
235235
github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc=
236236
github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk=
237237
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
238+
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
238239
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
239240
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
240241
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=

workers/generic-worker/artifacts/s3.go

Lines changed: 21 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ import (
99
"net/http/httputil"
1010
"os"
1111
"path/filepath"
12-
"runtime"
1312

14-
"github.com/gofrs/flock"
1513
"github.com/taskcluster/httpbackoff/v3"
1614
"github.com/taskcluster/taskcluster/v83/clients/client-go/tcqueue"
1715
"github.com/taskcluster/taskcluster/v83/internal/mocktc/tc"
@@ -31,40 +29,11 @@ type S3Artifact struct {
3129
ContentType string
3230
}
3331

34-
// createTempFileForPUTBody first tries to put a file lock on
35-
// the artifact to prevent further writes to it (posix only).
36-
// The file path of the locked file is returned. If this is unsuccessful,
37-
// it will fall back to copying to a temporary file.
38-
// This method will also gzip-compress the file at Path and writes
39-
// it to a temporary file in the same directory. The file path of the
40-
// generated temporary file is returned.
41-
// A callback function is also returned that should be used to
42-
// cleanup the resources (e.g. unlock the file lock or delete the
43-
// temp file). It is the responsibility of the caller to
44-
// use this cleanup function.
45-
func (s3Artifact *S3Artifact) createTempFileForPUTBody() (string, func()) {
46-
var fileLock *flock.Flock
47-
removeFileLock := func() {
48-
log.Printf("Removing exclusive file lock on %v...", s3Artifact.ContentPath)
49-
_ = fileLock.Unlock()
50-
}
51-
52-
if runtime.GOOS != "windows" {
53-
fileLock = flock.New(s3Artifact.ContentPath)
54-
log.Printf("Attempting to get exclusive file lock on %v...", s3Artifact.ContentPath)
55-
locked, _ := fileLock.TryLock()
56-
if locked {
57-
log.Printf("Got exclusive file lock on %v", s3Artifact.ContentPath)
58-
if s3Artifact.ContentEncoding != "gzip" {
59-
log.Printf("No need to copy %v to temp file", s3Artifact.ContentPath)
60-
return s3Artifact.ContentPath, removeFileLock
61-
}
62-
log.Printf("Need to gzip-compress %v...", s3Artifact.ContentPath)
63-
} else {
64-
log.Printf("Failed to get exclusive file lock on %v", s3Artifact.ContentPath)
65-
}
66-
}
67-
32+
// createTempFileForPUTBody gzip-compresses the file at Path and
33+
// writes it to a temporary file in the same directory. The file path of the
34+
// generated temporary file is returned. It is the responsibility of the
35+
// caller to delete the temporary file.
36+
func (s3Artifact *S3Artifact) createTempFileForPUTBody() string {
6837
baseName := filepath.Base(s3Artifact.Path)
6938
tmpFile, err := os.CreateTemp("", baseName)
7039
if err != nil {
@@ -84,28 +53,30 @@ func (s3Artifact *S3Artifact) createTempFileForPUTBody() (string, func()) {
8453
}
8554
defer source.Close()
8655
_, _ = io.Copy(target, source)
87-
return tmpFile.Name(), func() {
88-
if runtime.GOOS != "windows" {
89-
removeFileLock()
90-
}
91-
os.Remove(tmpFile.Name())
92-
}
56+
return tmpFile.Name()
9357
}
9458

9559
func (s3Artifact *S3Artifact) ProcessResponse(resp any, logger Logger, serviceFactory tc.ServiceFactory, config *gwconfig.Config) (err error) {
9660
response := resp.(*tcqueue.S3ArtifactResponse)
9761

9862
logger.Infof("Uploading artifact %v from file %v with content encoding %q, mime type %q and expiry %v", s3Artifact.Name, s3Artifact.Path, s3Artifact.ContentEncoding, s3Artifact.ContentType, s3Artifact.Expires)
9963

100-
transferContentFile, cleanup := s3Artifact.createTempFileForPUTBody()
101-
defer cleanup()
64+
// task user copies artifact at Path to ContentPath for task artifacts
65+
// and this should be cleaned up after artifact upload
66+
tempFileCreated := s3Artifact.Path != s3Artifact.ContentPath
67+
if tempFileCreated {
68+
defer os.Remove(s3Artifact.ContentPath)
69+
}
10270

103-
defer func() {
104-
if s3Artifact.Path != s3Artifact.ContentPath {
105-
// If we created a temporary file, delete it.
106-
os.Remove(s3Artifact.ContentPath)
107-
}
108-
}()
71+
var transferContentFile string
72+
if tempFileCreated && s3Artifact.ContentEncoding != "gzip" {
73+
log.Printf("Not copying %v to temp file", s3Artifact.ContentPath)
74+
transferContentFile = s3Artifact.ContentPath
75+
} else {
76+
log.Printf("Copying %v to temp file...", s3Artifact.ContentPath)
77+
transferContentFile = s3Artifact.createTempFileForPUTBody()
78+
defer os.Remove(transferContentFile)
79+
}
10980

11081
// perform http PUT to upload to S3...
11182
httpClient := &http.Client{}

0 commit comments

Comments
 (0)