Conversation
| wg.Go(func() { | ||
| // Each worker keeps consuming from the channel until it's closed | ||
| for config := range scheduleConfigChan { | ||
| func() { |
There was a problem hiding this comment.
I don't think this needs to be in a lambda, given that it's already in the wg.Go(func() { ... }) scope
There was a problem hiding this comment.
I added the lambda so I could use defer and maybe some variable scoping issues. I'll see if I can remove it
| }(sc.ScheduleID, start) | ||
| } | ||
|
|
||
| <-ticker.C |
There was a problem hiding this comment.
think you could replace all of the ticker business with <-time.After(s.config.OperationInterval) here (or, probably, select over that and ctx.Done)
There was a problem hiding this comment.
I didn't want the duration of he API call to impact the wait time between operations. there's a sizable latency different between v1/v2. though this should have a select.
| defer close(scheduleIDChan) | ||
| listErrChan <- s.listSchedules(ctx, client, scheduleIDChan, logger) |
There was a problem hiding this comment.
IMO, the iterator should be responsible for closing the channel, instead of the caller (and if the caller cancels the channel, the iterator should stop producing).
| if !s.config.SkipDeletion { | ||
| defer func(id string, startTime time.Time) { | ||
| dur := time.Until(startTime.Add(s.config.WaitTimeBeforeCleanup)) | ||
| select { | ||
| case <-time.After(dur): | ||
| if err := s.deleteSchedule(ctx, client, id, logger); err != nil { | ||
| logger.Errorw("Failed to delete schedule", "scheduleID", id, "error", err) | ||
| } | ||
| case <-ctx.Done(): | ||
| logger.Infow("Context canceled") | ||
| } | ||
| }(scheduleID, start) | ||
| } |
There was a problem hiding this comment.
I think this can be factored out to a helper.
| if errors.As(err, ¬FoundErr) { | ||
| // Return nil if schedule is not found (already deleted or never existed) | ||
| logger.Debug("Schedule not found during describe operation", "scheduleID", scheduleID) | ||
| // retryConfig defines retry behavior for schedule operations |
There was a problem hiding this comment.
stale comment? I don't see a retryConfig
| ) | ||
|
|
||
| // retryWithBackoff executes an operation with exponential backoff and jitter on context deadline errors | ||
| func retryWithBackoff(ctx context.Context, operation string, logger *zap.SugaredLogger, fn func() error) error { |
There was a problem hiding this comment.
Why not use the SDK's built in retry mechanism for this?
There was a problem hiding this comment.
I was getting a bunch of context cancellations and I don't have control over the sdk config. don't remember if the SDK was actually retrying or not.
Rework the scheduler_stress scenario for flexible ad-hoc load testing.
Changes:
skip-deletion,skip-creation, andworker-countoptions for controlling load patterns without rebuildingworker-count)executeWithExistingSchedulespath to re-run operations against previously created schedules (skip-creation=true)retryWithBackoffwith exponential backoff and jitter for all schedule operations (create, describe, update, delete)TriggerImmediatelyon schedule creation