-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Small cleanup in mercury v0.3 request code #11442
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package v03 | |
import ( | ||
"context" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"io" | ||
"net/http" | ||
|
@@ -62,42 +61,34 @@ func NewClient(mercuryConfig mercury.MercuryConfigProvider, httpClient mercury.H | |
} | ||
|
||
func (c *client) DoRequest(ctx context.Context, streamsLookup *mercury.StreamsLookup, pluginRetryKey string) (mercury.MercuryUpkeepState, mercury.MercuryUpkeepFailureReason, [][]byte, bool, time.Duration, error) { | ||
resultLen := len(streamsLookup.Feeds) | ||
ch := make(chan mercury.MercuryData, resultLen) | ||
if len(streamsLookup.Feeds) == 0 { | ||
return mercury.NoPipelineError, mercury.MercuryUpkeepFailureReasonInvalidRevertDataInput, [][]byte{}, false, 0 * time.Second, fmt.Errorf("invalid revert data input: feed param key %s, time param key %s, feeds %s", streamsLookup.FeedParamKey, streamsLookup.TimeParamKey, streamsLookup.Feeds) | ||
} | ||
resultLen = 1 | ||
resultLen := 1 // Only 1 multi-feed request is made for all feeds | ||
ch := make(chan mercury.MercuryData, resultLen) | ||
c.threadCtrl.Go(func(ctx context.Context) { | ||
c.multiFeedsRequest(ctx, ch, streamsLookup) | ||
}) | ||
|
||
var reqErr error | ||
var retryInterval time.Duration | ||
results := make([][]byte, len(streamsLookup.Feeds)) | ||
retryable := true | ||
allSuccess := true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no longer need to have logic for allSuccess since we don't have a loop and its based on single result |
||
retryable := false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no longer need to start with true and keep doing && within loop If the single request fails then we'll set the retryable value, else it's false by default |
||
state := mercury.NoPipelineError | ||
|
||
for i := 0; i < resultLen; i++ { | ||
m := <-ch | ||
if m.Error != nil { | ||
reqErr = errors.Join(reqErr, m.Error) | ||
retryable = retryable && m.Retryable | ||
allSuccess = false | ||
if m.State != mercury.NoPipelineError { | ||
state = m.State | ||
} | ||
continue | ||
m := <-ch | ||
if m.Error != nil { | ||
reqErr = m.Error | ||
retryable = m.Retryable | ||
state = m.State | ||
if retryable { | ||
retryInterval = mercury.CalculateRetryConfigFn(pluginRetryKey, c.mercuryConfig) | ||
} | ||
} else { | ||
results = m.Bytes | ||
} | ||
// only retry when not all successful AND none are not retryable | ||
if retryable && !allSuccess { | ||
retryInterval = mercury.CalculateRetryConfigFn(pluginRetryKey, c.mercuryConfig) | ||
} | ||
// only retry when not all successful AND none are not retryable | ||
return state, mercury.MercuryUpkeepFailureReasonNone, results, retryable && !allSuccess, retryInterval, reqErr | ||
|
||
return state, mercury.MercuryUpkeepFailureReasonNone, results, retryable, retryInterval, reqErr | ||
} | ||
|
||
func (c *client) multiFeedsRequest(ctx context.Context, ch chan<- mercury.MercuryData, sl *mercury.StreamsLookup) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously a larger size channel was being made than was necessary (only 1 value was written to it). Reducing this shouldn't have any effect