You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
When the publishEnvelope function encounters the error "not enough peers to publish," it's advisable not to increment the envelope.attempts counter within the EnvelopesMonitor.handleEnvelopeFailure method.
Summary of Changes
New Fields in EnvelopesMonitor:
Added retryQueue field of type []*monitoredEnvelope to manage envelopes that need to be retried.
New Methods:
backoffDuration(attempts int) time.Duration: Calculates the backoff duration based on the number of attempts.
retryLoop(): Handles the retry logic in a loop, triggered by a ticker every 500 milliseconds.
retryOnce(): Retries sending an envelope once, adhering to backoff logic.
removeFromRetryQueue(envelopeID types.Hash): Removes the specified envelope from the retry queue.
Modified Methods:
Start(): Added a new goroutine to handle the retry logic using retryLoop().
Stop(): Now waits for both goroutines to finish using WaitGroup.
Add(): Now sets lastAttemptTime for new envelopes.
handleEnvelopeFailure(hash types.Hash, err error): Envelopes that need to be retried are now added to the retryQueue instead of being retried immediately.
Miscellaneous:
Renamed some fields for better understanding
Set the lastAttemptTime field in monitoredEnvelope during creation and update.
Updated various logging statements to include more context.
These changes add a retry mechanism with exponential backoff to the EnvelopesMonitor, ensuring that envelopes are retried a limited number of times before being marked as expired. This should improve the reliability and robustness of the envelope handling process.
Hi @richard-ramos, appreciate your feedback. The initial motivation behind creating this PR was to address the potential issue of encountering the error "not enough peers to publish" multiple times in quick succession. This could lead to a situation where handleEnvelopeFailure retries immediately after the error occurs, while the underlying problem of "not enough peers to publish" may not be resolved promptly. Consequently, this could result in envelope.attempts reaching maxAttempts too quickly.
With your insights, things are clearer now. I believe the best approach to prevent envelope.attempts from reaching maxAttempts too rapidly is to implement a backoff mechanism linked to envelope.attempts. I'll incorporate this into my PR tomorrow. Thanks again for your input. WDYT? @richard-ramos cc @chaitanyaprem
Hi @richard-ramos, appreciate your feedback. The initial motivation behind creating this PR was to address the potential issue of encountering the error "not enough peers to publish" multiple times in quick succession. This could lead to a situation where handleEnvelopeFailure retries immediately after the error occurs, while the underlying problem of "not enough peers to publish" may not be resolved promptly. Consequently, this could result in envelope.attempts reaching maxAttempts too quickly.
With your insights, things are clearer now. I believe the best approach to prevent envelope.attempts from reaching maxAttempts too rapidly is to implement a backoff mechanism linked to envelope.attempts. I'll incorporate this into my PR tomorrow. Thanks again for your input. WDYT? @richard-ramos cc @chaitanyaprem
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.
qfrank
force-pushed
the
chore/optimise_waku_broadcast
branch
from
When the
publishEnvelopefunction encounters the error "not enough peers to publish," it's advisable not to increment theenvelope.attemptscounter within theEnvelopesMonitor.handleEnvelopeFailuremethod.Summary of Changes
New Fields in
EnvelopesMonitor:retryQueuefield of type[]*monitoredEnvelopeto manage envelopes that need to be retried.New Methods:
backoffDuration(attempts int) time.Duration: Calculates the backoff duration based on the number of attempts.retryLoop(): Handles the retry logic in a loop, triggered by a ticker every 500 milliseconds.retryOnce(): Retries sending an envelope once, adhering to backoff logic.removeFromRetryQueue(envelopeID types.Hash): Removes the specified envelope from the retry queue.Modified Methods:
Start(): Added a new goroutine to handle the retry logic usingretryLoop().Stop(): Now waits for both goroutines to finish usingWaitGroup.Add(): Now setslastAttemptTimefor new envelopes.handleEnvelopeFailure(hash types.Hash, err error): Envelopes that need to be retried are now added to theretryQueueinstead of being retried immediately.Miscellaneous:
lastAttemptTimefield inmonitoredEnvelopeduring creation and update.These changes add a retry mechanism with exponential backoff to the
EnvelopesMonitor, ensuring that envelopes are retried a limited number of times before being marked as expired. This should improve the reliability and robustness of the envelope handling process.