[RW] Use remoteapi.Write function to sendSamplesWithBackoff#17373
Closed
pipiland2612 wants to merge 11 commits intoprometheus:mainfrom
Closed
[RW] Use remoteapi.Write function to sendSamplesWithBackoff#17373pipiland2612 wants to merge 11 commits intoprometheus:mainfrom
pipiland2612 wants to merge 11 commits intoprometheus:mainfrom
Conversation
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
pipiland2612
requested review from
bwplotka,
cstyan and
tomwilkie
as code owners
Contributor
Author
|
This is just an overall idea of what would look liek if we want to use Write function for sendsSampleWithBackOff. The test I will fix later if the overall method is good. |
Signed-off-by: pipiland2612 <[email protected]>
Signed-off-by: pipiland2612 <[email protected]>
pipiland2612
commented
Oct 24, 2025
| @@ -459,37 +489,42 @@ func TestSampleDeliveryOrder(t *testing.T) { | |||
|
|
|||
| func TestShutdown(t *testing.T) { | |||
Contributor
Author
There was a problem hiding this comment.
This changed version of the test is flaky
bwplotka
reviewed
Oct 24, 2025
| externalLabels: extLabelsSlice, | ||
| relabelConfigs: relabelConfigs, | ||
| storeClient: client, | ||
| remoteAPI: nil, // Will be set below after QueueManager is created |
Member
There was a problem hiding this comment.
Suggested change
| remoteAPI: nil, // Will be set below after QueueManager is created |
| backoffCfg := remoteapi.BackoffConfig{ | ||
| Min: time.Duration(t.cfg.MinBackoff), | ||
| Max: time.Duration(t.cfg.MaxBackoff), | ||
| MaxRetries: 0, |
Member
There was a problem hiding this comment.
this is noop
Suggested change
| MaxRetries: 0, |
| var err error | ||
| t.remoteAPI, err = remoteapi.NewAPI(client.Endpoint(), apiOpts...) | ||
| if err != nil { | ||
| logger.Error("Failed to create remote API client", "endpoint", client.Endpoint(), "err", err) |
| logger.Error("Failed to create remote API client", "endpoint", client.Endpoint(), "err", err) | ||
| } | ||
|
|
||
| return t |
Member
There was a problem hiding this comment.
Suggested change
| return t | |
| return t, err |
|
|
||
| // Initialize remoteAPI now that QueueManager is fully created | ||
| // Configure backoff to match queue manager config | ||
| backoffCfg := remoteapi.BackoffConfig{ |
Member
There was a problem hiding this comment.
Why not just do t.SetClient(....)
| @@ -1048,8 +1073,29 @@ func (t *QueueManager) SeriesReset(index int) { | |||
| // fields are updated to avoid restarting the queue. | |||
| func (t *QueueManager) SetClient(c WriteClient) { | |||
Member
There was a problem hiding this comment.
Suggested change
| func (t *QueueManager) SetClient(c WriteClient) { | |
| func (t *QueueManager) SetClient(c WriteClient) error { |
| if isSampleOld(currentTime, time.Duration(s.qm.cfg.SampleAgeLimit), lowest) { | ||
| // Data became stale - cancel the Write to stop retrying stale data. | ||
| s.qm.logger.Debug("data became stale during Write retries, canceling", | ||
| "refresh_attempt", refreshAttempt, |
Member
There was a problem hiding this comment.
Let's ensure we have a test for refreshed retries.
bwplotka
reviewed
Nov 24, 2025
| return accumulatedStats, err | ||
| s.qm.metrics.sentBatchDuration.Observe(time.Since(begin).Seconds()) | ||
|
|
||
| // Convert client_golang stats to Prometheus stats. |
Member
There was a problem hiding this comment.
Do we need to convert things? I wonder why we still need to use private field here. Is this enough to convince us to make this field public or are we missing helper function?
26 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue(s) does the PR fix:
Does this PR introduce a user-facing change?