Skip to content

Commit 43cad0b

Browse files
ttaylorrttaylorr
authored
Merge pull request git-lfs#1971 from git-lfs/verify-configurable-retry
tq: retry verify requests a configurable number of times
2 parents 3cd3a41 + bac0312 commit 43cad0b

File tree

6 files changed

+209
-10
lines changed

6 files changed

+209
-10
lines changed

‎appveyor.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
skip_branch_with_pr: true
22

33
environment:
4-
PATH: c:\tools\go\bin;$(PATH)
54
GOPATH: $(HOMEDRIVE)$(HOMEPATH)\go
65
MSYSTEM: MINGW64
76

87
clone_folder: $(GOPATH)\src\github.com\git-lfs\git-lfs
98

109
install:
10+
- rd C:\Go /s /q
1111
- cinst golang --version 1.8.0 -y
1212
- cinst InnoSetup -y
1313
- refreshenv

‎docs/man/git-lfs-config.5.ronn

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ lfs option can be scoped inside the configuration for a remote.
102102
not an integer, is less than one, or is not given, a value of eight will be
103103
used instead.
104104

105+
* `lfs.transfer.maxverifies`
106+
107+
Specifies how many verification requests LFS will attempt per OID before
108+
marking the transfer as failed, if the object has a verification action
109+
associated with it. Must be an integer which is at least one. If the value is
110+
not an integer, is less than one, or is not given, a default value of three
111+
will be used instead.
112+
105113
### Fetch settings
106114

107115
* `lfs.fetchinclude`

‎test/cmd/lfstest-gitserver.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ var (
5959
"status-storage-403", "status-storage-404", "status-storage-410", "status-storage-422", "status-storage-500", "status-storage-503",
6060
"status-batch-resume-206", "batch-resume-fail-fallback", "return-expired-action", "return-expired-action-forever", "return-invalid-size",
6161
"object-authenticated", "storage-download-retry", "storage-upload-retry", "unknown-oid",
62+
"send-verify-action",
6263
}
6364
)
6465

@@ -98,6 +99,7 @@ func main() {
9899
})
99100

100101
mux.HandleFunc("/storage/", storageHandler)
102+
mux.HandleFunc("/verify", verifyHandler)
101103
mux.HandleFunc("/redirect307/", redirect307Handler)
102104
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
103105
id, ok := reqId(w)
@@ -402,6 +404,15 @@ func lfsBatchHandler(w http.ResponseWriter, r *http.Request, id, repo string) {
402404
}
403405
o.Actions[action] = a
404406
}
407+
408+
if handler == "send-verify-action" {
409+
o.Actions["verify"] = lfsLink{
410+
Href: server.URL + "/verify",
411+
Header: map[string]string{
412+
"repo": repo,
413+
},
414+
}
415+
}
405416
}
406417

407418
if testingChunked && addAction {
@@ -458,6 +469,46 @@ func serveExpired(repo string) {
458469
var batchResumeFailFallbackStorageAttempts = 0
459470
var tusStorageAttempts = 0
460471

472+
var (
473+
vmu sync.Mutex
474+
verifyCounts = make(map[string]int)
475+
verifyRetryRe = regexp.MustCompile(`verify-fail-(\d+)-times?$`)
476+
)
477+
478+
func verifyHandler(w http.ResponseWriter, r *http.Request) {
479+
repo := r.Header.Get("repo")
480+
var payload struct {
481+
Oid string `json:"oid"`
482+
Size int64 `json:"size"`
483+
}
484+
485+
if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
486+
writeLFSError(w, http.StatusUnprocessableEntity, err.Error())
487+
return
488+
}
489+
490+
var max int
491+
if matches := verifyRetryRe.FindStringSubmatch(repo); len(matches) < 2 {
492+
return
493+
} else {
494+
max, _ = strconv.Atoi(matches[1])
495+
}
496+
497+
key := strings.Join([]string{repo, payload.Oid}, ":")
498+
499+
vmu.Lock()
500+
verifyCounts[key] = verifyCounts[key] + 1
501+
count := verifyCounts[key]
502+
vmu.Unlock()
503+
504+
if count < max {
505+
writeLFSError(w, http.StatusServiceUnavailable, fmt.Sprintf(
506+
"intentionally failing verify request %d (out of %d)", count, max,
507+
))
508+
return
509+
}
510+
}
511+
461512
// handles any /storage/{oid} requests
462513
func storageHandler(w http.ResponseWriter, r *http.Request) {
463514
id, ok := reqId(w)

‎test/test-verify.sh

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/usr/bin/env bash
2+
3+
. "test/testlib.sh"
4+
5+
begin_test "verify with retries"
6+
(
7+
set -e
8+
9+
reponame="verify-fail-2-times"
10+
setup_remote_repo "$reponame"
11+
clone_repo "$reponame" "$reponame"
12+
13+
git lfs track "*.dat"
14+
git add .gitattributes
15+
git commit -m "initial commit"
16+
17+
contents="send-verify-action"
18+
contents_oid="$(calc_oid "$contents")"
19+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
20+
printf "$contents" > a.dat
21+
22+
git add a.dat
23+
git commit -m "add a.dat"
24+
25+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
26+
27+
[ "0" -eq "${PIPESTATUS[0]}" ]
28+
[ "2" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
29+
)
30+
end_test
31+
32+
begin_test "verify with retries (success without retry)"
33+
(
34+
set -e
35+
36+
reponame="verify-fail-0-times"
37+
setup_remote_repo "$reponame"
38+
clone_repo "$reponame" "$reponame"
39+
40+
git lfs track "*.dat"
41+
git add .gitattributes
42+
git commit -m "initial commit"
43+
44+
contents="send-verify-action"
45+
contents_oid="$(calc_oid "$contents")"
46+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
47+
printf "$contents" > a.dat
48+
49+
git add a.dat
50+
git commit -m "add a.dat"
51+
52+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
53+
54+
[ "0" -eq "${PIPESTATUS[0]}" ]
55+
[ "1" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
56+
)
57+
end_test
58+
59+
begin_test "verify with retries (insufficient retries)"
60+
(
61+
set -e
62+
63+
reponame="verify-fail-10-times"
64+
setup_remote_repo "$reponame"
65+
clone_repo "$reponame" "$reponame"
66+
67+
git lfs track "*.dat"
68+
git add .gitattributes
69+
git commit -m "initial commit"
70+
71+
contents="send-verify-action"
72+
contents_oid="$(calc_oid "$contents")"
73+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
74+
printf "$contents" > a.dat
75+
76+
git add a.dat
77+
git commit -m "add a.dat"
78+
79+
set +e
80+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
81+
if [ "0" -eq "${PIPESTATUS[0]}" ]; then
82+
echo >&2 "verify: expected \"git push\" to fail, didn't ..."
83+
exit 1
84+
fi
85+
set -e
86+
87+
[ "3" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
88+
)
89+
end_test
90+
91+
begin_test "verify with retries (bad .gitconfig)"
92+
(
93+
set -e
94+
95+
reponame="bad-config-verify-fail-2-times"
96+
setup_remote_repo "$reponame"
97+
clone_repo "$reponame" "$reponame"
98+
99+
# Invalid `lfs.transfer.maxverifies` will default to 3.
100+
git config "lfs.transfer.maxverifies" "-1"
101+
102+
git lfs track "*.dat"
103+
git add .gitattributes
104+
git commit -m "initial commit"
105+
106+
contents="send-verify-action"
107+
contents_oid="$(calc_oid "$contents")"
108+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
109+
printf "$contents" > a.dat
110+
111+
git add a.dat
112+
git commit -m "add a.dat"
113+
114+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
115+
116+
[ "0" -eq "${PIPESTATUS[0]}" ]
117+
[ "2" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
118+
)
119+
end_test

‎tq/verify.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import (
44
"net/http"
55

66
"github.com/git-lfs/git-lfs/lfsapi"
7+
"github.com/git-lfs/git-lfs/tools"
8+
"github.com/rubyist/tracerx"
9+
)
10+
11+
const (
12+
maxVerifiesConfigKey = "lfs.transfer.maxverifies"
13+
defaultMaxVerifyAttempts = 3
714
)
815

916
func verifyUpload(c *lfsapi.Client, t *Transfer) error {
@@ -33,10 +40,20 @@ func verifyUpload(c *lfsapi.Client, t *Transfer) error {
3340
}
3441
req.Header.Set("Content-Type", "application/vnd.git-lfs+json")
3542

36-
res, err := c.Do(req)
37-
if err != nil {
38-
return err
39-
}
43+
mv := c.GitEnv().Int(maxVerifiesConfigKey, defaultMaxVerifyAttempts)
44+
mv = tools.MaxInt(defaultMaxVerifyAttempts, mv)
4045

41-
return res.Body.Close()
46+
for i := 1; i <= mv; i++ {
47+
tracerx.Printf("tq: verify %s attempt #%d (max: %d)", t.Oid[:7], i, mv)
48+
49+
var res *http.Response
50+
51+
if res, err = c.Do(req); err != nil {
52+
tracerx.Printf("tq: verify err: %+v", err.Error())
53+
} else {
54+
err = res.Body.Close()
55+
break
56+
}
57+
}
58+
return err
4259
}

‎tq/verify_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/git-lfs/git-lfs/lfsapi"
1111
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1213
)
1314

1415
func TestVerifyWithoutAction(t *testing.T) {
@@ -33,19 +34,22 @@ func TestVerifySuccess(t *testing.T) {
3334

3435
assert.Equal(t, "POST", r.Method)
3536
assert.Equal(t, "bar", r.Header.Get("Foo"))
36-
assert.Equal(t, "24", r.Header.Get("Content-Length"))
37+
assert.Equal(t, "29", r.Header.Get("Content-Length"))
3738
assert.Equal(t, "application/vnd.git-lfs+json", r.Header.Get("Content-Type"))
3839

3940
var tr Transfer
4041
assert.Nil(t, json.NewDecoder(r.Body).Decode(&tr))
41-
assert.Equal(t, "abc", tr.Oid)
42+
assert.Equal(t, "abcd1234", tr.Oid)
4243
assert.EqualValues(t, 123, tr.Size)
4344
}))
4445
defer srv.Close()
4546

46-
c := &lfsapi.Client{}
47+
c, err := lfsapi.NewClient(nil, lfsapi.TestEnv{
48+
"lfs.transfer.maxverifies": "1",
49+
})
50+
require.Nil(t, err)
4751
tr := &Transfer{
48-
Oid: "abc",
52+
Oid: "abcd1234",
4953
Size: 123,
5054
Actions: map[string]*Action{
5155
"verify": &Action{

0 commit comments

Comments
 (0)