-
Notifications
You must be signed in to change notification settings - Fork 53
feat: tolerate immediately recoverable stream faults, improve logging #1019
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
Merged
toddbaert
merged 12 commits into
open-feature:main
from
guidobrei:feat/1010-improve-flagd-error-logging
Oct 15, 2024
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
6ac6414
feat(flagd): Log stream errors and metadata errors separately
guidobrei e42f663
feat(flagd): Add reusable backoff strategies
guidobrei 786f489
feat(flagd): Use backoff service in GrpcConnector
guidobrei 18807cf
feat(flagd): Small refactorings
guidobrei 9858567
feat(flagd): Log stream errors and metadata errors separately
guidobrei 923932b
feat(flagd): Add reusable backoff strategies
guidobrei 77760bb
feat(flagd): Use backoff service in GrpcConnector
guidobrei 9c4ab05
feat(flagd): Small refactorings
guidobrei 6d9b7f8
fixup: reconnect tests
toddbaert 05cd1fe
Merge branch 'feat/1010-improve-flagd-error-logging' of https://githu…
guidobrei 396db2b
feat(flagd): Use GrpcStreamConnectorBackoffService in GrpcConnector
guidobrei 6eaa5fc
Merge branch 'main' into feat/1010-improve-flagd-error-logging
guidobrei File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
90 changes: 90 additions & 0 deletions
90
.../java/dev/openfeature/contrib/providers/flagd/resolver/common/backoff/BackoffService.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
import lombok.Getter; | ||
|
||
import java.util.concurrent.ThreadLocalRandom; | ||
|
||
/** | ||
* A service that provides backoff functionality. | ||
*/ | ||
public class BackoffService { | ||
public static final int DEFAULT_MAX_JITTER = 0x1 << 8; // 256; Random likes boundaries that are a power of 2 | ||
guidobrei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@Getter | ||
private final BackoffStrategy strategy; | ||
|
||
@Getter | ||
private final int maxJitter; | ||
|
||
/** | ||
* Creates a new BackoffService with the given strategy and default maximum jitter. | ||
* The default maximum jitter is 256. | ||
* | ||
* @param strategy The backoff strategy to use | ||
*/ | ||
public BackoffService(BackoffStrategy strategy) { | ||
this(strategy, DEFAULT_MAX_JITTER); | ||
} | ||
|
||
/** | ||
* Creates a new BackoffService with the given strategy and maximum jitter. | ||
* | ||
* @param strategy The backoff strategy to use | ||
* @param maxJitter The maximum jitter value | ||
*/ | ||
public BackoffService(BackoffStrategy strategy, int maxJitter) { | ||
this.strategy = strategy; | ||
this.maxJitter = maxJitter; | ||
} | ||
|
||
/** | ||
* Returns the current backoff time in milliseconds. | ||
* This backoff time will be used in waitUntilNextAttempt. | ||
* | ||
* @return the current backoff time in milliseconds | ||
*/ | ||
public long getCurrentBackoffMillis() { | ||
return strategy.getCurrentBackoffMillis(); | ||
} | ||
|
||
/** | ||
* Returns a random jitter value between 0 and maxJitter. | ||
* | ||
* @return a random jitter value | ||
*/ | ||
public long getRandomJitter() { | ||
if (maxJitter == 0) { | ||
return 0; | ||
} | ||
|
||
return ThreadLocalRandom.current().nextInt(maxJitter); | ||
} | ||
|
||
/** | ||
* Resets the backoff strategy to its initial state. | ||
*/ | ||
public void reset() { | ||
strategy.reset(); | ||
} | ||
|
||
/** | ||
* Returns whether the backoff strategy has more attempts left. | ||
* @return true if the backoff strategy has more attempts left, false otherwise | ||
*/ | ||
public boolean shouldRetry() { | ||
return !strategy.isExhausted(); | ||
} | ||
|
||
/** | ||
* Bolocks the current thread until the next attempt should be made. | ||
* The time to wait is determined by the backoff strategy and a random jitter. | ||
* | ||
* @throws InterruptedException if the thread is interrupted while waiting | ||
*/ | ||
public void waitUntilNextAttempt() throws InterruptedException { | ||
long retryDelay = getCurrentBackoffMillis() + getRandomJitter(); | ||
strategy.nextBackoff(); | ||
|
||
Thread.sleep(retryDelay); | ||
} | ||
} |
30 changes: 30 additions & 0 deletions
30
...va/dev/openfeature/contrib/providers/flagd/resolver/common/backoff/BackoffStrategies.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
/** | ||
* A factory class for creating common backoff strategies. | ||
*/ | ||
public class BackoffStrategies { | ||
private BackoffStrategies() { | ||
} | ||
|
||
public static BackoffStrategy exponentialTimeBackoff(long initialBackoffMillis) { | ||
return new ExponentialTimeBackoff(initialBackoffMillis); | ||
} | ||
|
||
public static BackoffStrategy exponentialTimeBackoff(long initialBackoffMillis, long maxBackoffMillis) { | ||
return new ExponentialTimeBackoff(initialBackoffMillis, maxBackoffMillis); | ||
} | ||
|
||
public static BackoffStrategy constantTimeBackoff(long millis) { | ||
return new ConstantTimeBackoff(millis); | ||
} | ||
|
||
public static BackoffStrategy noBackoff() { | ||
return new ConstantTimeBackoff(0L); | ||
} | ||
|
||
public static BackoffStrategy maxRetriesWithExponentialTimeBackoffStrategy(int maxRetries, | ||
long initialBackoffMillis) { | ||
return new NumberOfRetriesBackoff(maxRetries, exponentialTimeBackoff(initialBackoffMillis)); | ||
} | ||
} |
32 changes: 32 additions & 0 deletions
32
...java/dev/openfeature/contrib/providers/flagd/resolver/common/backoff/BackoffStrategy.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
/** | ||
* A strategy interface for determining how long to backoff before retrying a failed operation. | ||
*/ | ||
public interface BackoffStrategy { | ||
|
||
/** | ||
* The current backoff time in milliseconds. | ||
* This value should be used to determine how long to wait before retrying. | ||
* | ||
* @return the current backoff time in milliseconds | ||
*/ | ||
long getCurrentBackoffMillis(); | ||
|
||
/** | ||
* Determines if the backoff strategy has been exhausted. | ||
* | ||
* @return true if the operation should backoff, false otherwise | ||
*/ | ||
boolean isExhausted(); | ||
|
||
/** | ||
* Move to the next backoff time. | ||
*/ | ||
void nextBackoff(); | ||
|
||
/** | ||
* Reset the backoff strategy to its initial state. | ||
*/ | ||
void reset(); | ||
} |
73 changes: 73 additions & 0 deletions
73
...java/dev/openfeature/contrib/providers/flagd/resolver/common/backoff/CombinedBackoff.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
import lombok.Getter; | ||
|
||
/** | ||
guidobrei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* A backoff strategy that combines multiple backoff strategies. | ||
* The strategy starts with the first provided strategy and will switch to the next backoff strategy in the list when | ||
* the current one is exhausted. | ||
*/ | ||
public class CombinedBackoff implements BackoffStrategy { | ||
private final BackoffStrategy[] backoffStrategies; | ||
private int currentStrategyIndex; | ||
|
||
@Getter | ||
private BackoffStrategy currentStrategy; | ||
|
||
/** | ||
* Creates a new combined backoff strategy. | ||
* The strategy starts with the first provided strategy and will switch to the next backoff strategy in the list | ||
* when the current one is exhausted. | ||
* | ||
* @param backoffStrategies the list of backoff strategies to combine | ||
*/ | ||
|
||
public CombinedBackoff(BackoffStrategy[] backoffStrategies) { | ||
this.backoffStrategies = backoffStrategies.clone(); | ||
this.currentStrategyIndex = 0; | ||
this.currentStrategy = this.backoffStrategies[currentStrategyIndex]; | ||
updateCurrentStrategy(); | ||
} | ||
|
||
@Override | ||
public long getCurrentBackoffMillis() { | ||
return currentStrategy.getCurrentBackoffMillis(); | ||
} | ||
|
||
@Override | ||
public boolean isExhausted() { | ||
updateCurrentStrategy(); | ||
return currentStrategy.isExhausted(); | ||
} | ||
|
||
@Override | ||
public void nextBackoff() { | ||
updateCurrentStrategy(); | ||
currentStrategy.nextBackoff(); | ||
} | ||
|
||
/** | ||
* Switches to the next backoff strategy if the current one is exhausted. | ||
*/ | ||
private void updateCurrentStrategy() { | ||
// Move to the next non-exhausted strategy if the current one is exhausted | ||
while (!isLastStrategy() && currentStrategy.isExhausted()) { | ||
currentStrategyIndex++; | ||
currentStrategy = backoffStrategies[currentStrategyIndex]; | ||
} | ||
} | ||
|
||
private boolean isLastStrategy() { | ||
return currentStrategyIndex + 1 >= backoffStrategies.length; | ||
} | ||
|
||
@Override | ||
public void reset() { | ||
for (int i = 0; i <= currentStrategyIndex; i++) { | ||
backoffStrategies[i].reset(); | ||
} | ||
|
||
currentStrategyIndex = 0; | ||
currentStrategy = backoffStrategies[currentStrategyIndex]; | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
.../dev/openfeature/contrib/providers/flagd/resolver/common/backoff/ConstantTimeBackoff.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
/** | ||
* A backoff strategy that always returns the same backoff time. | ||
* This backoff is never exhausted. | ||
*/ | ||
public class ConstantTimeBackoff implements BackoffStrategy { | ||
final long millis; | ||
|
||
public ConstantTimeBackoff(long millis) { | ||
this.millis = millis; | ||
} | ||
|
||
@Override | ||
public long getCurrentBackoffMillis() { | ||
return millis; | ||
} | ||
|
||
@Override | ||
public boolean isExhausted() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public void nextBackoff() { | ||
} | ||
|
||
@Override | ||
public void reset() { | ||
} | ||
} |
57 changes: 57 additions & 0 deletions
57
...v/openfeature/contrib/providers/flagd/resolver/common/backoff/ExponentialTimeBackoff.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
/** | ||
* A backoff strategy that exponentially increases the backoff time. | ||
* This backoff is never exhausted. | ||
*/ | ||
public class ExponentialTimeBackoff implements BackoffStrategy { | ||
public static final long DEFAULT_MAX_BACK_OFF = 120 * 1000; | ||
|
||
private final long initialBackoff; | ||
private final long maxBackoff; | ||
private long currentBackoff; | ||
|
||
/** | ||
* A backoff strategy that exponentially increases the backoff time. | ||
* This backoff will double the backoff time until the DEFAULT_MAX_BACK_OFF is reached. | ||
* | ||
* @param initialBackoffMillis the initial backoff time in milliseconds | ||
*/ | ||
public ExponentialTimeBackoff(long initialBackoffMillis) { | ||
this(initialBackoffMillis, DEFAULT_MAX_BACK_OFF); | ||
} | ||
|
||
/** | ||
* A backoff strategy that exponentially increases the backoff time. | ||
* This backoff will double the backoff time until the maximum backoff time is reached. | ||
* It is never exhausted but will stale at the maximum backoff time. | ||
* | ||
* @param initialBackoffMillis the initial backoff time in milliseconds | ||
* @param maxBackoffMillis the maximum backoff time in milliseconds | ||
*/ | ||
public ExponentialTimeBackoff(long initialBackoffMillis, long maxBackoffMillis) { | ||
this.initialBackoff = initialBackoffMillis; | ||
this.maxBackoff = maxBackoffMillis; | ||
reset(); | ||
} | ||
|
||
@Override | ||
public long getCurrentBackoffMillis() { | ||
return currentBackoff; | ||
} | ||
|
||
@Override | ||
public boolean isExhausted() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public void nextBackoff() { | ||
currentBackoff = Math.min(currentBackoff * 2, maxBackoff); | ||
} | ||
|
||
@Override | ||
public void reset() { | ||
currentBackoff = Math.min(initialBackoff, maxBackoff); | ||
} | ||
} |
40 changes: 40 additions & 0 deletions
40
...re/contrib/providers/flagd/resolver/common/backoff/GrpcStreamConnectorBackoffService.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package dev.openfeature.contrib.providers.flagd.resolver.common.backoff; | ||
|
||
/** | ||
* Backoff service that supports "silent" backoff. | ||
*/ | ||
public class GrpcStreamConnectorBackoffService extends BackoffService { | ||
toddbaert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private final BackoffStrategy silentRecoverBackoff; | ||
|
||
/** | ||
* Create a new backoff service that will not backoff (0ms) on first attempt. | ||
* Subsequent attempts will backoff exponentially. | ||
* | ||
* @param initialBackoffMillis initial backoff time in milliseconds used for exponential error backoff | ||
*/ | ||
public GrpcStreamConnectorBackoffService(long initialBackoffMillis) { | ||
this(BackoffStrategies.exponentialTimeBackoff(initialBackoffMillis)); | ||
} | ||
|
||
/** | ||
* Create a new backoff service that will not backoff (0ms) on first attempt. | ||
* Subsequent attempts will backoff using the provided backoff strategy. | ||
* | ||
* @param errorBackoff backoff strategy to use after the first attempt | ||
*/ | ||
public GrpcStreamConnectorBackoffService(BackoffStrategy errorBackoff) { | ||
this(new NumberOfRetriesBackoff(1, BackoffStrategies.noBackoff()), errorBackoff); | ||
} | ||
|
||
private GrpcStreamConnectorBackoffService(BackoffStrategy silentRecoverBackoff, BackoffStrategy errorBackoff) { | ||
super(new CombinedBackoff(new BackoffStrategy[]{ | ||
silentRecoverBackoff, | ||
errorBackoff | ||
})); | ||
this.silentRecoverBackoff = silentRecoverBackoff; | ||
} | ||
|
||
public boolean shouldRetrySilently() { | ||
return ((CombinedBackoff) getStrategy()).getCurrentStrategy() == silentRecoverBackoff; | ||
} | ||
} |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.