Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1564)

Delta Between Two Patch Sets: environs/ec2/storage.go

Issue 8545045: environs/ec2: retry s3 Put requests
Left Patch Set: Created 10 years, 11 months ago
Right Patch Set: environs/ec2: retry s3 Put requests Created 10 years, 11 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « environs/ec2/state.go ('k') | environs/interface.go » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 package ec2 1 package ec2
2 2
3 import ( 3 import (
4 "fmt" 4 "fmt"
5 "io" 5 "io"
6 "launchpad.net/goamz/s3" 6 "launchpad.net/goamz/s3"
7 "launchpad.net/juju-core/environs" 7 "launchpad.net/juju-core/environs"
8 "launchpad.net/juju-core/log" 8 "launchpad.net/juju-core/log"
9 "net" 9 "net"
10 "sync" 10 "sync"
(...skipping 15 matching lines...) Expand all
26 // makeBucket makes the environent's control bucket, the 26 // makeBucket makes the environent's control bucket, the
27 // place where bootstrap information and deployed charms 27 // place where bootstrap information and deployed charms
28 // are stored. To avoid two round trips on every PUT operation, 28 // are stored. To avoid two round trips on every PUT operation,
29 // we do this only once for each environ. 29 // we do this only once for each environ.
30 func (s *storage) makeBucket() error { 30 func (s *storage) makeBucket() error {
31 s.Lock() 31 s.Lock()
32 defer s.Unlock() 32 defer s.Unlock()
33 if s.madeBucket { 33 if s.madeBucket {
34 return nil 34 return nil
35 } 35 }
36 // TODO(rog) make this simpler when goamz does retries on PutBucket,
36 var err error 37 var err error
37 for a := shortAttempt.Start(); a.Next(); { 38 for a := shortAttempt.Start(); a.Next(); {
38 » » // PutBucket always return a 200 if we recreate an existing buck et for the 39 » » // PutBucket always returns a 200 if we recreate an existing buc ket for the
dimitern 2013/04/11 18:18:06 s/return/returns/ ?
rog 2013/04/15 23:19:56 Done.
39 // original s3.amazonaws.com endpoint. For all other endpoints P utBucket 40 // original s3.amazonaws.com endpoint. For all other endpoints P utBucket
40 // returns 409 with a known subcode. 41 // returns 409 with a known subcode.
41 err = s.bucket.PutBucket(s3.Private) 42 err = s.bucket.PutBucket(s3.Private)
42 if s3ErrCode(err) == "BucketAlreadyOwnedByYou" { 43 if s3ErrCode(err) == "BucketAlreadyOwnedByYou" {
dimitern 2013/04/11 18:18:06 put this in a const perhaps?
rog 2013/04/15 23:19:56 i don't think that would be worth it currently. if
43 err = nil 44 err = nil
44 } 45 }
45 if !shouldRetry(err) { 46 if !shouldRetry(err) {
46 break 47 break
47 } 48 }
48 log.Noticef("environs/ec2: transient S3 PutBucket error: %v", er r) 49 log.Noticef("environs/ec2: transient S3 PutBucket error: %v", er r)
fwereade 2013/04/17 13:51:36 Warningf -- if that. Probably Infof or even Debugf
49 } 50 }
50 if err == nil { 51 if err == nil {
51 s.madeBucket = true 52 s.madeBucket = true
52 } 53 }
53 return err 54 return err
54 } 55 }
55 56
56 func (s *storage) Put(file string, r io.ReadSeeker, length int64) error { 57 func (s *storage) Put(file string, r io.ReadSeeker, length int64) error {
57 if err := s.makeBucket(); err != nil { 58 if err := s.makeBucket(); err != nil {
58 return fmt.Errorf("cannot make S3 control bucket: %v", err) 59 return fmt.Errorf("cannot make S3 control bucket: %v", err)
59 } 60 }
60 var err error 61 var err error
62 // TODO(rog) make this simpler when goamz does retries on PutReader,
61 for a := shortAttempt.Start(); a.Next(); { 63 for a := shortAttempt.Start(); a.Next(); {
62 err = s.bucket.PutReader(file, r, length, "binary/octet-stream", s3.Private) 64 err = s.bucket.PutReader(file, r, length, "binary/octet-stream", s3.Private)
63 if !shouldRetry(err) { 65 if !shouldRetry(err) {
64 return err 66 return err
65 } 67 }
66 log.Noticef("environs/ec2: transient S3 Put error: %v", err) 68 log.Noticef("environs/ec2: transient S3 Put error: %v", err)
fwereade 2013/04/17 13:51:36 Warningf at least
67 if _, err := r.Seek(0, 0); err != nil { 69 if _, err := r.Seek(0, 0); err != nil {
68 return err 70 return err
69 } 71 }
70 } 72 }
71 if err != nil { 73 if err != nil {
72 return fmt.Errorf("cannot write file %q to control bucket: %v", file, err) 74 return fmt.Errorf("cannot write file %q to control bucket: %v", file, err)
73 } 75 }
74 return nil 76 return nil
75 } 77 }
76 78
77 func (s *storage) Get(file string) (r io.ReadCloser, err error) { 79 func (s *storage) Get(file string) (r io.ReadCloser, err error) {
78 for a := shortAttempt.Start(); a.Next(); { 80 for a := shortAttempt.Start(); a.Next(); {
79 r, err = s.bucket.GetReader(file) 81 r, err = s.bucket.GetReader(file)
80 if s3ErrorStatusCode(err) != 404 { 82 if s3ErrorStatusCode(err) != 404 {
81 break 83 break
82 } 84 }
83 } 85 }
84 return r, maybeNotFound(err) 86 return r, maybeNotFound(err)
85 } 87 }
86 88
87 func (s *storage) URL(name string) (string, error) { 89 func (s *storage) URL(name string) (string, error) {
88 // 10 years should be good enough. 90 // 10 years should be good enough.
89 return s.bucket.SignedURL(name, time.Now().AddDate(10, 0, 0)), nil 91 return s.bucket.SignedURL(name, time.Now().AddDate(10, 0, 0)), nil
90 } 92 }
91 93
92 // shouldRetry returns whether a error looks transient. 94 // shouldRetry returns whether a error looks transient.
93 // It's an exact copy of the function of the same name in 95 // It's an exact copy of the function of the same name in
94 // goamz/s3. 96 // goamz/s3.
dimitern 2013/04/11 18:18:06 maybe put a TODO to get rid of this once goamz mak
rog 2013/04/15 23:19:56 Done.
97 // TODO(rog) remove when goamz does retries itself.
95 func shouldRetry(err error) bool { 98 func shouldRetry(err error) bool {
96 if err == nil { 99 if err == nil {
97 return false 100 return false
98 } 101 }
99 switch err { 102 switch err {
100 case io.ErrUnexpectedEOF, io.EOF: 103 case io.ErrUnexpectedEOF, io.EOF:
101 return true 104 return true
102 } 105 }
103 switch e := err.(type) { 106 switch e := err.(type) {
104 case *net.DNSError: 107 case *net.DNSError:
105 return true 108 return true
106 case *net.OpError: 109 case *net.OpError:
107 switch e.Op { 110 switch e.Op {
108 case "read", "write": 111 case "read", "write":
109 return true 112 return true
110 } 113 }
111 case *s3.Error: 114 case *s3.Error:
112 switch e.Code { 115 switch e.Code {
113 case "InternalError", "NoSuchUpload", "NoSuchBucket": 116 case "InternalError", "NoSuchUpload", "NoSuchBucket":
dimitern 2013/04/11 18:18:06 these as well
rog 2013/04/15 23:19:56 i'd prefer to keep this function an exact copy of
114 return true 117 return true
115 } 118 }
116 } 119 }
117 return false 120 return false
118 } 121 }
119 122
120 // s3ErrorStatusCode returns the HTTP status of the S3 request error, 123 // s3ErrorStatusCode returns the HTTP status of the S3 request error,
121 // if it is an error from an S3 operation, or 0 if it was not. 124 // if it is an error from an S3 operation, or 0 if it was not.
122 func s3ErrorStatusCode(err error) int { 125 func s3ErrorStatusCode(err error) int {
123 if err, _ := err.(*s3.Error); err != nil { 126 if err, _ := err.(*s3.Error); err != nil {
124 return err.StatusCode 127 return err.StatusCode
125 } 128 }
126 return 0 129 return 0
127 } 130 }
128 131
129 // s3ErrCode returns the text status code of the S3 error code. 132 // s3ErrCode returns the text status code of the S3 error code.
130 func s3ErrCode(err error) string { 133 func s3ErrCode(err error) string {
131 if err, ok := err.(*s3.Error); ok { 134 if err, ok := err.(*s3.Error); ok {
132 return err.Code 135 return err.Code
133 } 136 }
134 return "" 137 return ""
135 } 138 }
136 139
137 func (s *storage) Remove(file string) error { 140 func (s *storage) Remove(file string) error {
138 var err error 141 var err error
142 // TODO(rog) make this simpler when goamz does retries on Del,
139 for a := shortAttempt.Start(); a.Next(); { 143 for a := shortAttempt.Start(); a.Next(); {
140 err = s.bucket.Del(file) 144 err = s.bucket.Del(file)
141 // If we can't delete the object because the bucket doesn't 145 // If we can't delete the object because the bucket doesn't
142 // exist, then we don't care. 146 // exist, then we don't care.
143 if s3ErrorStatusCode(err) == 404 { 147 if s3ErrorStatusCode(err) == 404 {
144 return nil 148 return nil
145 } 149 }
146 if !shouldRetry(err) { 150 if !shouldRetry(err) {
147 return err 151 return err
148 } 152 }
(...skipping 59 matching lines...) Expand 10 before | Expand all | Expand 10 after
208 } 212 }
209 return err 213 return err
210 } 214 }
211 215
212 func maybeNotFound(err error) error { 216 func maybeNotFound(err error) error {
213 if err != nil && s3ErrorStatusCode(err) == 404 { 217 if err != nil && s3ErrorStatusCode(err) == 404 {
214 return &environs.NotFoundError{err} 218 return &environs.NotFoundError{err}
215 } 219 }
216 return err 220 return err
217 } 221 }
LEFTRIGHT

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b