Repository navigation
feat(multiprovider): add ComparisonStrategy #2003
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
|---|---|---|
| @@ -0,0 +1,254 @@ | ||
| package dev.openfeature.sdk.multiprovider; | ||
|
|
||
| import dev.openfeature.sdk.ErrorCode; | ||
| import dev.openfeature.sdk.EvaluationContext; | ||
| import dev.openfeature.sdk.FeatureProvider; | ||
| import dev.openfeature.sdk.ProviderEvaluation; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.concurrent.Callable; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.ForkJoinPool; | ||
| import java.util.concurrent.Future; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.function.BiConsumer; | ||
| import java.util.function.Function; | ||
| import lombok.Getter; | ||
|
|
||
| /** | ||
| * Comparison strategy. | ||
| * | ||
| * <p>Evaluates all providers in parallel and compares successful results. | ||
| * If all providers agree on the value, the fallback provider's result is returned. | ||
| * If providers disagree, the optional {@code onMismatch} callback is invoked | ||
| * and the fallback provider's result is returned. | ||
| * If any provider returns an error, all errors are collected and a {@link MultiProviderEvaluation} | ||
| * with {@link ErrorCode#GENERAL} and per-provider {@link ProviderError} details is returned. | ||
| */ | ||
| public class ComparisonStrategy implements Strategy { | ||
|
|
||
| private static final long DEFAULT_TIMEOUT_MS = 30_000; | ||
|
|
||
| @Getter | ||
| private final String fallbackProvider; | ||
|
|
||
| private final BiConsumer<String, Map<String, ProviderEvaluation<?>>> onMismatch; | ||
| private final ExecutorService executorService; | ||
| private final long timeoutMs; | ||
|
|
||
| /** | ||
| * Constructs a comparison strategy with a fallback provider. | ||
| * | ||
| * <p>Uses a shared {@link ForkJoinPool#commonPool()} for parallel evaluation. | ||
| * | ||
| * @param fallbackProvider provider name to use as fallback when successful | ||
| * providers disagree | ||
| */ | ||
| public ComparisonStrategy(String fallbackProvider) { | ||
| this(fallbackProvider, null); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs a comparison strategy with fallback provider and mismatch callback. | ||
| * | ||
| * <p>Uses a shared {@link ForkJoinPool#commonPool()} for parallel evaluation. | ||
| * | ||
| * @param fallbackProvider provider name to use as fallback when successful | ||
| * providers disagree | ||
| * @param onMismatch callback invoked with all successful evaluations | ||
| * when they disagree | ||
| */ | ||
| public ComparisonStrategy( | ||
| String fallbackProvider, BiConsumer<String, Map<String, ProviderEvaluation<?>>> onMismatch) { | ||
| this(fallbackProvider, onMismatch, ForkJoinPool.commonPool(), DEFAULT_TIMEOUT_MS); | ||
| } | ||
|
|
||
| /** | ||
| * Constructs a comparison strategy with a caller-supplied executor. | ||
| * | ||
| * @param fallbackProvider provider name to use as fallback when successful | ||
| * providers disagree | ||
| * @param onMismatch callback invoked with all successful evaluations | ||
| * when they disagree (may be {@code null}) | ||
| * @param executorService executor to use for parallel evaluation | ||
| * @param timeoutMs maximum time in milliseconds to wait for all | ||
| * providers to complete | ||
| */ | ||
| public ComparisonStrategy( | ||
| String fallbackProvider, | ||
| BiConsumer<String, Map<String, ProviderEvaluation<?>>> onMismatch, | ||
| ExecutorService executorService, | ||
| long timeoutMs) { | ||
| this.fallbackProvider = Objects.requireNonNull(fallbackProvider, "fallbackProvider must not be null"); | ||
| this.onMismatch = onMismatch; | ||
| this.executorService = Objects.requireNonNull(executorService, "executorService must not be null"); | ||
| this.timeoutMs = timeoutMs; | ||
| } | ||
|
|
||
| @Override | ||
| public <T> ProviderEvaluation<T> evaluate( | ||
| Map<String, FeatureProvider> providers, | ||
| String key, | ||
| T defaultValue, | ||
| EvaluationContext ctx, | ||
| Function<FeatureProvider, ProviderEvaluation<T>> providerFunction) { | ||
| if (providers.isEmpty()) { | ||
| return ProviderEvaluation.<T>builder() | ||
| .errorCode(ErrorCode.GENERAL) | ||
| .errorMessage("No providers configured") | ||
| .build(); | ||
| } | ||
| if (!providers.containsKey(fallbackProvider)) { | ||
| throw new IllegalArgumentException("fallbackProvider not found in providers: " + fallbackProvider); | ||
| } | ||
|
|
||
| int capacity = providers.size() * 4 / 3 + 1; | ||
| Map<String, ProviderEvaluation<T>> successfulResults = new ConcurrentHashMap<>(capacity); | ||
| Map<String, ProviderError> providerErrors = new ConcurrentHashMap<>(capacity); | ||
|
|
||
| Optional<ProviderEvaluation<T>> runFailure = | ||
| runEvaluations(providers, providerFunction, successfulResults, providerErrors); | ||
| if (runFailure.isPresent()) { | ||
| return runFailure.get(); | ||
| } | ||
|
|
||
| if (!providerErrors.isEmpty()) { | ||
| return errorResult("Provider errors during comparison", providers, providerErrors); | ||
| } | ||
|
|
||
| ProviderEvaluation<T> fallbackResult = successfulResults.get(fallbackProvider); | ||
| if (fallbackResult == null) { | ||
| return errorResult( | ||
| "Fallback provider did not return a successful evaluation: " + fallbackProvider, | ||
| providers, | ||
| providerErrors); | ||
| } | ||
|
|
||
| if (allEvaluationsMatch(successfulResults)) { | ||
| return fallbackResult; | ||
| } | ||
|
|
||
| if (onMismatch != null) { | ||
| onMismatch.accept(key, orderedResults(providers, successfulResults)); | ||
| } | ||
| return fallbackResult; | ||
| } | ||
|
|
||
| /** | ||
| * Evaluates every provider in parallel, recording each outcome into {@code successfulResults} or | ||
| * {@code providerErrors}. | ||
| * | ||
| * @return an error evaluation if the parallel run itself could not complete (timeout, | ||
| * interruption, or executor failure), otherwise {@link Optional#empty()} | ||
| */ | ||
| private <T> Optional<ProviderEvaluation<T>> runEvaluations( | ||
| Map<String, FeatureProvider> providers, | ||
| Function<FeatureProvider, ProviderEvaluation<T>> providerFunction, | ||
| Map<String, ProviderEvaluation<T>> successfulResults, | ||
| Map<String, ProviderError> providerErrors) { | ||
| try { | ||
| List<Callable<Void>> tasks = new ArrayList<>(providers.size()); | ||
| for (Map.Entry<String, FeatureProvider> entry : providers.entrySet()) { | ||
| String providerName = entry.getKey(); | ||
| FeatureProvider provider = entry.getValue(); | ||
| tasks.add(() -> { | ||
| recordEvaluation(providerName, provider, providerFunction, successfulResults, providerErrors); | ||
| return null; | ||
| }); | ||
| } | ||
| List<Future<Void>> futures = executorService.invokeAll(tasks, timeoutMs, TimeUnit.MILLISECONDS); | ||
| for (Future<Void> future : futures) { | ||
| if (future.isCancelled()) { | ||
| return Optional.of(errorResult( | ||
| "Comparison strategy timed out after " + timeoutMs + "ms", providers, providerErrors)); | ||
| } | ||
|
Comment on lines
+190
to
+203
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. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Record a Lines 165-170 return on the first cancelled Associate each submitted task with its provider name. Mark every cancelled future as a timeout error before building the aggregate result. Extend 🤖 Prompt for AI Agents |
||
| future.get(); | ||
| } | ||
| return Optional.empty(); | ||
|
Comment on lines
+181
to
+205
|
||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| return Optional.of( | ||
| errorResult("Comparison strategy interrupted: " + e.getMessage(), providers, providerErrors)); | ||
| } catch (Exception e) { | ||
| return Optional.of(errorResult("Comparison strategy failed: " + e.getMessage(), providers, providerErrors)); | ||
| } | ||
| } | ||
|
|
||
| /** Evaluates a single provider, recording either its result or its error. */ | ||
| private <T> void recordEvaluation( | ||
| String providerName, | ||
| FeatureProvider provider, | ||
| Function<FeatureProvider, ProviderEvaluation<T>> providerFunction, | ||
| Map<String, ProviderEvaluation<T>> successfulResults, | ||
| Map<String, ProviderError> providerErrors) { | ||
| try { | ||
| ProviderEvaluation<T> evaluation = providerFunction.apply(provider); | ||
| if (evaluation == null) { | ||
| providerErrors.put( | ||
| providerName, ProviderError.fromResult(providerName, ErrorCode.GENERAL, "null evaluation")); | ||
| } else if (evaluation.getErrorCode() == null) { | ||
| successfulResults.put(providerName, evaluation); | ||
| } else { | ||
| providerErrors.put( | ||
| providerName, | ||
| ProviderError.fromResult( | ||
| providerName, evaluation.getErrorCode(), evaluation.getErrorMessage())); | ||
| } | ||
| } catch (Exception e) { | ||
| providerErrors.put(providerName, ProviderError.fromException(providerName, e)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Builds a {@link MultiProviderEvaluation} carrying per-provider error details, ordered by the | ||
| * provider registration order so the aggregate message is stable across runs. | ||
| */ | ||
| private <T> ProviderEvaluation<T> errorResult( | ||
| String baseMessage, Map<String, FeatureProvider> providers, Map<String, ProviderError> providerErrors) { | ||
| List<ProviderError> orderedErrors = new ArrayList<>(providerErrors.size()); | ||
| for (String providerName : providers.keySet()) { | ||
| ProviderError error = providerErrors.get(providerName); | ||
| if (error != null) { | ||
| orderedErrors.add(error); | ||
| } | ||
| } | ||
| return MultiProviderEvaluation.<T>builder() | ||
| .errorCode(ErrorCode.GENERAL) | ||
| .errorMessage(ProviderError.buildAggregateMessage(baseMessage, orderedErrors)) | ||
| .providerErrors(orderedErrors) | ||
| .build(); | ||
| } | ||
|
|
||
| /** Returns the successful evaluations in provider registration order. */ | ||
| private <T> Map<String, ProviderEvaluation<?>> orderedResults( | ||
| Map<String, FeatureProvider> providers, Map<String, ProviderEvaluation<T>> successfulResults) { | ||
| Map<String, ProviderEvaluation<?>> ordered = new LinkedHashMap<>(); | ||
| for (String providerName : providers.keySet()) { | ||
| ProviderEvaluation<T> evaluation = successfulResults.get(providerName); | ||
| if (evaluation != null) { | ||
| ordered.put(providerName, evaluation); | ||
| } | ||
| } | ||
| return Collections.unmodifiableMap(ordered); | ||
| } | ||
|
|
||
| private <T> boolean allEvaluationsMatch(Map<String, ProviderEvaluation<T>> results) { | ||
| ProviderEvaluation<T> baseline = null; | ||
| for (ProviderEvaluation<T> evaluation : results.values()) { | ||
| if (baseline == null) { | ||
| baseline = evaluation; | ||
| continue; | ||
| } | ||
| if (!Objects.equals(baseline.getValue(), evaluation.getValue())) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
| } | ||
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.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: open-feature/java-sdk
Length of output: 10692
🏁 Script executed:
Repository: open-feature/java-sdk
Length of output: 215
🏁 Script executed:
Repository: open-feature/java-sdk
Length of output: 32431
🌐 Web query:
JDK ForkJoinPool commonPool parallelism default documentation💡 Result:
In the JDK, the ForkJoinPool.commonPool parallelism level defaults to Runtime.availableProcessors minus 1 [1]. If the system has only one processor, the default parallelism is 1 [1]. The parallelism level can be customized or overridden by setting the system property java.util.concurrent.ForkJoinPool.common.parallelism [2][3][4]. The property must be a non-negative integer [2][3][4]. Key details regarding the common pool parallelism include: - The default formula aims to leave at least one processor available for the calling thread, unless only one processor is available [1]. - The setting can be checked programmatically using the static method ForkJoinPool.getCommonPoolParallelism [5]. - While the default is calculated based on available processors, it is specifically designed to support scenarios like parallel streams [1]. If the property is set to 0, the pool will effectively run tasks on the calling thread [1].
Citations:
Use a dedicated executor for default provider evaluation.
This strategy submits provider evaluations to
ForkJoinPool.commonPool(). Provider evaluations can run longer tasks, and the common pool may serialize them when its parallelism is constrained. This violates the documented parallel evaluation behavior and can starve unrelated common-pool work.Use a dedicated executor with an explicit lifecycle in the default constructor, or do not run evaluations on
ForkJoinPool.commonPool().🤖 Prompt for AI Agents