Skip to content
Draft
Show file tree
Hide file tree
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
@@ -0,0 +1,314 @@
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 dev.openfeature.sdk.internal.ConfigurableThreadFactory;
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.Executors;
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.
*
* <p>Providers that do not respond before the internal timeout do not fail the evaluation. The
* fallback provider's result is still returned, carrying a {@link ProviderError} for each provider
* that timed out, so that a slow provider under comparison cannot degrade evaluations. Only a
* timeout of the fallback provider itself produces an error result.
*/
public class ComparisonStrategy implements Strategy {

private static final long DEFAULT_TIMEOUT_MS = 30_000;

/**
* Shared pool used when no executor is supplied.
*
* <p>Provider evaluations block, so they are kept off {@link java.util.concurrent.ForkJoinPool
* #commonPool()} to avoid starving unrelated parallel work in the host application. Threads are
* daemon threads, so this pool never prevents JVM shutdown.
*/
private static final ExecutorService DEFAULT_EXECUTOR =
Executors.newCachedThreadPool(new ConfigurableThreadFactory("openfeature-comparison-strategy", true));

@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.
*
* @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.
*
* @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, DEFAULT_EXECUTOR, DEFAULT_TIMEOUT_MS);
}
Comment on lines +79 to +82

Copy link
Copy Markdown

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:

#!/bin/bash
set -euo pipefail

# Inspect the strategy structure and locate default-constructor call sites.
ast-grep outline src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java --items all
rg -nP --type java 'new\s+ComparisonStrategy\s*\(' src

# Inspect declared Java/build toolchain files before selecting an executor lifecycle design.
fd -HI -t f '^(pom\.xml|build\.gradle(\.kts)?|\.tool-versions|\.java-version)$' . \
  -x sh -c 'echo "=== $1 ==="; sed -n "1,220p" "$1"' sh {}

Repository: open-feature/java-sdk

Length of output: 10692


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant ComparisonStrategy implementation and public APIs that document or expose evaluation behavior.
for f in src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java \
         src/main/java/dev/openfeature/sdk/multiprovider/*.java \
         src/main/java/dev/openfeature/sdk/multiprovider/**/*.java; do
  if [ -f "$f" ]; then echo "=== $f ==="; wc -l "$f"; done
done

printf '\n--- ComparisonStrategy.java ---\n'
sed -n '1,260p' src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java

printf '\n--- Strategy.java (if exists) ---\n'
if [ -f src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java ]; then sed -n '1,220p' src/main/java/dev/openfeature/sdk/multiprovider/Strategy.java; fi

printf '\n--- All references to ComparisonStrategy / parallel evaluation words ---\n'
rg -n "ComparisonStrategy|parallel|executor|thread|blocking|runEvaluations|evaluate|fallbackProvider|DEFAULT_TIMEOUT" src/main/java

Repository: open-feature/java-sdk

Length of output: 215


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- ComparisonStrategy.java ---'
sed -n '1,280p' src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java

printf '%s\n' '--- all Multiprovider files under src/main/java/dev/openfeature/sdk/multiprovider ---'
find src/main/java/dev/openfeature/sdk/multiprovider -maxdepth 2 -type f -print | sort -u

printf '%s\n' '--- references to ComparisonStrategy / parallel/executor/thread words ---'
rg -n "ComparisonStrategy|parallel|executor|Executor|thread|Thread|blocking|runEvaluations|evaluate|fallbackProvider|DEFAULT_TIMEOUT|ForkJoinPool" src/main/java

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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java`
around lines 67 - 70, Update the default ComparisonStrategy constructor to use a
dedicated executor instead of ForkJoinPool.commonPool() for provider
evaluations, and ensure that executor has an explicit lifecycle consistent with
the class’s cleanup behavior. Preserve the existing fallback provider, mismatch
handler, and timeout configuration.


/**
* Constructs a comparison strategy with a caller-supplied executor and timeout.
*
* <p>Intentionally not public: the executor and timeout are implementation details, and the
* public surface is kept aligned with the js-sdk reference implementation.
*
* @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
*/
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);
Map<String, ProviderError> timeoutErrors = new ConcurrentHashMap<>(capacity);

Optional<ProviderEvaluation<T>> runFailure =
runEvaluations(providers, providerFunction, successfulResults, providerErrors, timeoutErrors);
if (runFailure.isPresent()) {
return runFailure.get();
}

if (!providerErrors.isEmpty()) {
return errorResult(
"Provider errors during comparison", orderedErrors(providers, providerErrors, timeoutErrors));
}

ProviderEvaluation<T> fallbackResult = successfulResults.get(fallbackProvider);
if (fallbackResult == null) {
return errorResult(
fallbackFailureMessage(timeoutErrors), orderedErrors(providers, providerErrors, timeoutErrors));
}

if (onMismatch != null && !allEvaluationsMatch(successfulResults)) {
onMismatch.accept(key, orderedResults(providers, successfulResults));
}

if (timeoutErrors.isEmpty()) {
return fallbackResult;
}
// A provider under comparison was too slow. Report it, but keep serving the fallback result.
return withProviderErrors(fallbackResult, orderedErrors(providers, providerErrors, timeoutErrors));
}

private String fallbackFailureMessage(Map<String, ProviderError> timeoutErrors) {
if (timeoutErrors.containsKey(fallbackProvider)) {
return "Fallback provider did not respond within " + timeoutMs + "ms: " + fallbackProvider;
}
return "Fallback provider did not return a successful evaluation: " + fallbackProvider;
}

/**
* Evaluates every provider in parallel, recording each outcome into {@code successfulResults},
* {@code providerErrors}, or, for providers that did not finish in time, {@code timeoutErrors}.
*
* @return an error evaluation if the parallel run itself could not complete (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,
Map<String, ProviderError> timeoutErrors) {
try {
List<String> providerNames = new ArrayList<>(providers.keySet());
List<Callable<Void>> tasks = new ArrayList<>(providers.size());
for (String providerName : providerNames) {
FeatureProvider provider = providers.get(providerName);
tasks.add(() -> {
recordEvaluation(providerName, provider, providerFunction, successfulResults, providerErrors);
return null;
});
}
// invokeAll returns futures in task submission order, which matches providerNames.
List<Future<Void>> futures = executorService.invokeAll(tasks, timeoutMs, TimeUnit.MILLISECONDS);
for (int i = 0; i < futures.size(); i++) {
Future<Void> future = futures.get(i);
String providerName = providerNames.get(i);
if (future.isCancelled()) {
timeoutErrors.put(
providerName,
ProviderError.fromResult(
providerName,
ErrorCode.GENERAL,
"Provider did not respond within " + timeoutMs + "ms"));
} else {
future.get();
}
Comment on lines +190 to +203

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Record a ProviderError for every timed-out provider.

Lines 165-170 return on the first cancelled Future. They do not create ProviderError entries for cancelled providers. A timeout result can therefore contain no structured provider errors.

Associate each submitted task with its provider name. Mark every cancelled future as a timeout error before building the aggregate result. Extend shouldReturnTimeoutErrorWhenProvidersExceedTheTimeout to assert the timed-out provider entries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java`
around lines 165 - 170, Update the timeout handling in ComparisonStrategy to
associate each submitted Future with its provider name, iterate all completed
futures, and record a timeout ProviderError for every cancelled future before
constructing the aggregate errorResult. Avoid returning on the first
cancellation, and extend shouldReturnTimeoutErrorWhenProvidersExceedTheTimeout
to verify each timed-out provider entry.

}
return Optional.empty();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return Optional.of(errorResult(
"Comparison strategy interrupted: " + e.getMessage(),
orderedErrors(providers, providerErrors, timeoutErrors)));
} catch (Exception e) {
return Optional.of(errorResult(
"Comparison strategy failed: " + e.getMessage(),
orderedErrors(providers, providerErrors, timeoutErrors)));
}
}

/** 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. */
private <T> ProviderEvaluation<T> errorResult(String baseMessage, List<ProviderError> orderedErrors) {
return MultiProviderEvaluation.<T>builder()
.errorCode(ErrorCode.GENERAL)
.errorMessage(ProviderError.buildAggregateMessage(baseMessage, orderedErrors))
.providerErrors(orderedErrors)
.build();
}

/**
* Merges the recorded errors into a single list ordered by provider registration order, so that
* aggregate messages are stable across runs.
*/
private List<ProviderError> orderedErrors(
Map<String, FeatureProvider> providers,
Map<String, ProviderError> providerErrors,
Map<String, ProviderError> timeoutErrors) {
List<ProviderError> orderedErrors = new ArrayList<>(providerErrors.size() + timeoutErrors.size());
for (String providerName : providers.keySet()) {
ProviderError error = providerErrors.get(providerName);
if (error == null) {
error = timeoutErrors.get(providerName);
}
if (error != null) {
orderedErrors.add(error);
}
}
return orderedErrors;
}

/**
* Returns the successful evaluation as a {@link MultiProviderEvaluation} carrying the given
* per-provider errors, so callers can see which providers were skipped or timed out.
*/
private <T> ProviderEvaluation<T> withProviderErrors(
ProviderEvaluation<T> evaluation, List<ProviderError> orderedErrors) {
return MultiProviderEvaluation.<T>builder()
.value(evaluation.getValue())
.variant(evaluation.getVariant())
.reason(evaluation.getReason())
.flagMetadata(evaluation.getFlagMetadata())
.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;
}
}
Loading
Loading