ai cooking
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

readiness is timing out and not waiting on all locatiopn backends anyways. increase timout and actuually block. Log reiness as operation id (#452)

Co-authored-by: paul miller <paul.miller>

authored by

Paul Miller
paul miller
and committed by
GitHub
677006bf b47485d2

+27 -36
+7 -1
cmd/careme/ready.go
··· 5 5 "fmt" 6 6 "log/slog" 7 7 "net/http" 8 + "sync" 9 + 10 + "careme/internal/logsetup" 8 11 ) 9 12 10 13 type readyOnce struct { 11 14 done bool 12 15 checks []Readyable 16 + mu sync.Mutex 13 17 } 14 18 15 19 func (r *readyOnce) Ready(ctx context.Context) error { 20 + r.mu.Lock() 21 + defer r.mu.Unlock() 16 22 if r.done { 17 23 return nil 18 24 } 25 + ctx = logsetup.WithOperationID(ctx, "readiness_check") 19 26 for _, check := range r.checks { 20 27 if err := check.Ready(ctx); err != nil { 21 28 slog.ErrorContext(ctx, "check failed", "error", err, "check", fmt.Sprintf("%T", check)) 22 29 return err 23 30 } 24 31 } 25 - // not thread safe? only ever set to true 26 32 r.done = true 27 33 return nil 28 34 }
+2
deploy/deploy.yaml
··· 64 64 port: http 65 65 initialDelaySeconds: 1 66 66 periodSeconds: 5 67 + timeoutSeconds: 3 #first call has to get outh token (should we do a startup probe?) 67 68 livenessProbe: 68 69 httpGet: 69 70 path: /ready 70 71 port: http 71 72 initialDelaySeconds: 15 72 73 periodSeconds: 20 74 + timeoutSeconds: 3 #first call has to get outh token 73 75 resources: 74 76 requests: 75 77 cpu: 50m
+2 -1
internal/locations/server.go
··· 141 141 142 142 func (l *locationServer) renderLocationsPage(w http.ResponseWriter, ctx context.Context, zip string, favoriteStore string, serverSignedIn bool) error { 143 143 locs, err := l.storage.GetLocationsByZip(ctx, zip) 144 - if err != nil { 144 + // be very forgiving of errors here. 145 + if len(locs) == 0 && err != nil { 145 146 return fmt.Errorf("failed to get locations for zip %s: %w", zip, err) 146 147 } 147 148
+13 -31
internal/locations/storage.go
··· 7 7 "fmt" 8 8 "log/slog" 9 9 "sort" 10 - "sync" 11 10 "time" 12 11 13 12 "careme/internal/albertsons" ··· 18 17 "careme/internal/kroger" 19 18 "careme/internal/locations/geo" 20 19 "careme/internal/logsetup" 20 + "careme/internal/parallelism" 21 21 "careme/internal/publix" 22 22 "careme/internal/walmart" 23 23 "careme/internal/wegmans" ··· 174 174 } 175 175 176 176 func (l *locationStorage) GetLocationsByZip(ctx context.Context, zipcode string) ([]Location, error) { 177 - results := make(chan []Location, len(l.clients)) 178 - errors := make(chan error, len(l.clients)) 179 - var wg sync.WaitGroup 180 - for _, backend := range l.clients { 181 - wg.Add(1) 182 - go func(backend locationBackend) { 183 - defer wg.Done() 184 - start := time.Now() 185 - locations, err := backend.GetLocationsByZip(ctx, zipcode) 186 - if err != nil { 187 - slog.ErrorContext(ctx, "error fetching locations from backend", "error", err, "backend", fmt.Sprintf("%T", backend), "zip", zipcode) 188 - errors <- err 189 - return 190 - } 191 - slog.InfoContext(ctx, "Got results for backend", "backend", fmt.Sprintf("%T", backend), "zip", zipcode, "count", len(locations), "latencyMS", time.Since(start).Milliseconds()) 192 - results <- locations 193 - }(backend) 194 - } 195 - wg.Wait() 196 - close(results) 197 - close(errors) 198 - if len(errors) == len(l.clients) { 199 - return nil, fmt.Errorf("all backends failed to get locations for zip %s", zipcode) 200 - } 201 - var allLocations []Location 202 - for result := range results { 203 - allLocations = append(allLocations, result...) 204 - } 177 + allLocations, fetcherrors := parallelism.Flatten(l.clients, func(backend locationBackend) ([]Location, error) { 178 + start := time.Now() 179 + locations, err := backend.GetLocationsByZip(ctx, zipcode) 180 + if err != nil { 181 + slog.ErrorContext(ctx, "error fetching locations from backend", "error", err, "backend", fmt.Sprintf("%T", backend), "zip", zipcode) 182 + return nil, err 183 + } 184 + slog.InfoContext(ctx, "Got results for backend", "backend", fmt.Sprintf("%T", backend), "zip", zipcode, "count", len(locations), "latencyMS", time.Since(start).Milliseconds()) 185 + return locations, err 186 + }) 205 187 206 188 for _, loc := range allLocations { 207 189 go func(loc Location) { ··· 215 197 if !hasRequestedCentroid { 216 198 // were missign zip codes. Make this an error later? 217 199 slog.WarnContext(ctx, "requested zip has no centroid; returning unsorted locations without distance filter", "zip", zipcode, "count", len(allLocations)) 218 - return allLocations, nil 200 + return allLocations, fetcherrors 219 201 } 220 202 221 203 filtered := make([]Location, 0, len(allLocations)) ··· 235 217 allLocations = filtered 236 218 sortLocationsByDistanceFromCentroid(allLocations, requestedCentroid, l.zipCentroids) 237 219 238 - return allLocations, nil 220 + return allLocations, fetcherrors 239 221 } 240 222 241 223 func (l *locationStorage) cachedLocationByID(ctx context.Context, locationID string) (Location, bool) {
+3 -3
internal/locations/storage_test.go
··· 409 409 } 410 410 } 411 411 412 - func TestGetLocationsByZipSucceedsWhenAtLeastOneBackendSucceeds(t *testing.T) { 412 + func TestGetLocationsByZipWhenAtLeastOneBackendSucceeds(t *testing.T) { 413 413 fail := newFakeLocationClient() 414 414 fail.err = fmt.Errorf("backend down") 415 415 ··· 422 422 423 423 server := newTestLocationServerWithBackends([]locationBackend{fail, success}) 424 424 locs, err := server.GetLocationsByZip(context.Background(), "00601") 425 - if err != nil { 426 - t.Fatalf("did not expect error when one backend succeeds: %v", err) 425 + if err == nil { 426 + t.Fatalf("expected an error: %v", err) 427 427 } 428 428 if len(locs) != 1 || locs[0].ID != "ok" { 429 429 t.Fatalf("unexpected locations: %+v", locs)