Skip to content

Commit 10cf0de

Browse files
committed
Redownload CRDB if previous download got killed in the middle
Resolve #102 This commit is to add a flock to the local file when downloading CRDB binary. If such localfile exists but can be flocked, it means the previous download process was killed before it finished. In this case, remove the existing local file and redownload CRDB binary.
1 parent 85896ca commit 10cf0de

File tree

5 files changed

+243
-13
lines changed

5 files changed

+243
-13
lines changed

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ module github.com/cockroachdb/cockroach-go/v2
33
go 1.13
44

55
require (
6+
github.com/gofrs/flock v0.8.1 // indirect
67
github.com/jackc/pgx/v4 v4.10.1
78
github.com/jmoiron/sqlx v1.3.1
89
github.com/lib/pq v1.10.0
910
github.com/pkg/errors v0.9.1
1011
github.com/shopspring/decimal v1.2.0 // indirect
1112
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2 // indirect
13+
golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect
1214
golang.org/x/text v0.3.5 // indirect
1315
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
1416
gopkg.in/yaml.v2 v2.4.0 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
1010
github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs=
1111
github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
1212
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
13+
github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
14+
github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
1315
github.com/gofrs/uuid v3.2.0+incompatible h1:y12jRkkFxsd7GpqdSZ+/KCs/fJbqpEXSGd4+jfEaewE=
1416
github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
1517
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
@@ -158,6 +160,8 @@ golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7w
158160
golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
159161
golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
160162
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
163+
golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf h1:2ucpDCmfkl8Bd/FsLtiD653Wf96cW37s+iGx93zsu4k=
164+
golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
161165
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
162166
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
163167
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=

testserver/binaries.go

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package testserver
1616

1717
import (
18+
"errors"
1819
"fmt"
1920
"io"
2021
"log"
@@ -28,22 +29,42 @@ import (
2829
"regexp"
2930
"runtime"
3031
"time"
32+
33+
"github.com/gofrs/flock"
3134
)
3235

3336
const (
3437
latestSuffix = "LATEST"
3538
finishedFileMode = 0555
39+
writingFileMode = 0600 // Allow reads so that another process can check if there's a flock.
3640
)
3741

38-
func downloadFile(response *http.Response, filePath string) error {
39-
output, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0200)
42+
func downloadFile(response *http.Response, filePath string, tc *TestConfig) error {
43+
output, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, writingFileMode)
4044
if err != nil {
4145
return fmt.Errorf("error creating %s: %s", filePath, err)
4246
}
4347
defer func() { _ = output.Close() }()
4448

4549
log.Printf("saving %s to %s, this may take some time", response.Request.URL, filePath)
4650

51+
// Assign a flock to the local file.
52+
// If the downloading process is killed in the middle,
53+
// the lock will be automatically dropped.
54+
localFileLock := flock.New(filePath)
55+
56+
if _, err := localFileLock.TryLock(); err != nil {
57+
return err
58+
}
59+
60+
defer func() { _ = localFileLock.Unlock() }()
61+
62+
if tc.IsTest && tc.StopDownloadInMiddle {
63+
log.Printf("download process killed")
64+
output.Close()
65+
return errStoppedInMiddle
66+
}
67+
4768
if _, err := io.Copy(output, response.Body); err != nil {
4869
return fmt.Errorf("problem saving %s to %s: %s", response.Request.URL, filePath, err)
4970
}
@@ -53,14 +74,21 @@ func downloadFile(response *http.Response, filePath string) error {
5374
return err
5475
}
5576

77+
if err := localFileLock.Unlock(); err != nil {
78+
return err
79+
}
80+
5681
// We explicitly close here to ensure the error is checked; the deferred
5782
// close above will likely error in this case, but that's harmless.
5883
return output.Close()
5984
}
6085

6186
var muslRE = regexp.MustCompile(`(?i)\bmusl\b`)
6287

63-
func downloadLatestBinary() (string, error) {
88+
// GetDownloadResponse return the http response of a CRDB download.
89+
// It creates the url for downloading a CRDB binary for current runtime OS,
90+
// makes a request to this url, and return the response.
91+
func GetDownloadResponse() (*http.Response, error) {
6492
goos := runtime.GOOS
6593
if goos == "linux" {
6694
goos += func() string {
@@ -88,14 +116,17 @@ func downloadLatestBinary() (string, error) {
88116
log.Printf("GET %s", url)
89117
response, err := http.Get(url.String())
90118
if err != nil {
91-
return "", err
119+
return nil, err
92120
}
93-
defer func() { _ = response.Body.Close() }()
94121

95122
if response.StatusCode != 200 {
96-
return "", fmt.Errorf("error downloading %s: %d (%s)", url, response.StatusCode, response.Status)
123+
return nil, fmt.Errorf("error downloading %s: %d (%s)", url,
124+
response.StatusCode, response.Status)
97125
}
126+
return response, nil
127+
}
98128

129+
func GetDownloadFilename(response *http.Response) (string, error) {
99130
const contentDisposition = "Content-Disposition"
100131
_, disposition, err := mime.ParseMediaType(response.Header.Get(contentDisposition))
101132
if err != nil {
@@ -106,6 +137,20 @@ func downloadLatestBinary() (string, error) {
106137
if !ok {
107138
return "", fmt.Errorf("content disposition header %s did not contain filename", disposition)
108139
}
140+
return filename, nil
141+
}
142+
143+
func downloadLatestBinary(tc *TestConfig) (string, error) {
144+
response, err := GetDownloadResponse()
145+
if err != nil {
146+
return "", err
147+
}
148+
defer func() { _ = response.Body.Close() }()
149+
150+
filename, err := GetDownloadFilename(response)
151+
if err != nil {
152+
return "", err
153+
}
109154
localFile := filepath.Join(os.TempDir(), filename)
110155
for {
111156
info, err := os.Stat(localFile)
@@ -120,13 +165,35 @@ func downloadLatestBinary() (string, error) {
120165
if info.Mode().Perm() == finishedFileMode {
121166
return localFile, nil
122167
}
168+
169+
localFileLock := flock.New(localFile)
170+
// If there's a process downloading the binary, local file cannot be flocked.
171+
locked, err := localFileLock.TryLock()
172+
if err != nil {
173+
return "", err
174+
}
175+
176+
if locked {
177+
// If local file can be locked, it means the previous download was
178+
// killed in the middle. Delete local file and re-download.
179+
log.Printf("previous download failed in the middle, deleting and re-downloading")
180+
if err := os.Remove(localFile); err != nil {
181+
log.Printf("failed to remove partial download %s: %v", localFile, err)
182+
return "", err
183+
}
184+
break
185+
}
186+
123187
log.Printf("waiting for download of %s", localFile)
124188
time.Sleep(time.Millisecond * 10)
125189
}
126190

127-
if err := downloadFile(response, localFile); err != nil {
128-
if err := os.Remove(localFile); err != nil {
129-
log.Printf("failed to remove %s: %s", localFile, err)
191+
err = downloadFile(response, localFile, tc)
192+
if err != nil {
193+
if !errors.Is(err, errStoppedInMiddle) {
194+
if err := os.Remove(localFile); err != nil {
195+
log.Printf("failed to remove %s: %s", localFile, err)
196+
}
130197
}
131198
return "", err
132199
}

testserver/testserver.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,15 @@ func NewDBForTestWithDatabase(
146146
t.Helper()
147147
ts, err := NewTestServer(opts...)
148148
if err != nil {
149+
if errors.Is(err, errStoppedInMiddle) {
150+
// If the testserver is intentionally killed in the middle,
151+
// make sure it is stopped.
152+
return nil, func() {
153+
if ts != nil {
154+
ts.Stop()
155+
}
156+
}
157+
}
149158
t.Fatal(err)
150159
}
151160
url := ts.PGURL()
@@ -167,11 +176,17 @@ func NewDBForTestWithDatabase(
167176
// TestServerOpt is passed to NewTestServer.
168177
type TestServerOpt func(args *testServerArgs)
169178

179+
type TestConfig struct {
180+
IsTest bool
181+
StopDownloadInMiddle bool
182+
}
183+
170184
type testServerArgs struct {
171185
secure bool
172186
rootPW string // if nonempty, set as pw for root
173187
storeOnDisk bool // to save database in disk
174188
storeMemSize float64 // the proportion of available memory allocated to test server
189+
testConfig TestConfig
175190
}
176191

177192
// SecureOpt is a TestServer option that can be passed to NewTestServer to
@@ -212,11 +227,23 @@ func RootPasswordOpt(pw string) TestServerOpt {
212227
}
213228
}
214229

230+
// StopDownloadInMiddleOpt is a TestServer option used only in testing.
231+
// It is used to test the flock over downloaded CRDB binary.
232+
// It should not be used in production.
233+
func StopDownloadInMiddleOpt() TestServerOpt {
234+
return func(args *testServerArgs) {
235+
tc := TestConfig{IsTest: true, StopDownloadInMiddle: true}
236+
args.testConfig = tc
237+
}
238+
}
239+
215240
const (
216241
logsDirName = "logs"
217242
certsDirName = "certs"
218243
)
219244

245+
var errStoppedInMiddle = errors.New("download stopped in middle")
246+
220247
// NewTestServer creates a new TestServer and starts it.
221248
// It also waits until the server is ready to accept clients,
222249
// so it safe to connect to the server returned by this function right away.
@@ -241,11 +268,19 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) {
241268
var err error
242269
if cockroachBinary != "" {
243270
log.Printf("Using custom cockroach binary: %s", cockroachBinary)
244-
} else if cockroachBinary, err = downloadLatestBinary(); err != nil {
245-
log.Printf("Failed to fetch latest binary: %s, attempting to use cockroach binary from your PATH", err)
246-
cockroachBinary = "cockroach"
247271
} else {
248-
log.Printf("Using automatically-downloaded binary: %s", cockroachBinary)
272+
cockroachBinary, err = downloadLatestBinary(&serverArgs.testConfig)
273+
if err != nil {
274+
if errors.Is(err, errStoppedInMiddle) {
275+
// If the testserver is intentionally killed in the middle of downloading,
276+
// return error.
277+
return nil, err
278+
}
279+
log.Printf("Failed to fetch latest binary: %s, attempting to use cockroach binary from your PATH", err)
280+
cockroachBinary = "cockroach"
281+
} else {
282+
log.Printf("Using automatically-downloaded binary: %s", cockroachBinary)
283+
}
249284
}
250285

251286
// Force "/tmp/" so avoid OSX's really long temp directory names

0 commit comments

Comments
 (0)