Skip to content
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

Merged
merged 1 commit into from
Dec 1, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package v03
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

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

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor Author

@infiloop2 infiloop2 Dec 1, 2023

Choose a reason for hiding this comment

The 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) {
Expand Down
Loading