fix(gofeatureflag): issue when using inProcess with high concurency#1781
fix(gofeatureflag): issue when using inProcess with high concurency#1781thomaspoignant wants to merge 8 commits intomainfrom
Conversation
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a WasmEvaluatorPool to handle concurrent flag evaluations and adds a wasmEvaluatorPoolSize configuration option. The review feedback identifies a race condition in the preWarmWasm method and suggests moving the pre-warming logic into the WasmEvaluatorPool constructor to ensure thread safety. Additionally, it is recommended to encapsulate the multiple volatile configuration fields into a single immutable object to prevent inconsistent states during evaluation.
| this.evaluationContextEnrichment = configFlags.getEvaluationContextEnrichment(); | ||
| // We call the WASM engine to avoid a cold start at the 1st evaluation | ||
| this.evaluationEngine.preWarmWasm(); | ||
| this.evaluationPool.preWarmWasm(); |
There was a problem hiding this comment.
If the WasmEvaluatorPool constructor is updated to pre-warm instances during allocation (as suggested in the WasmEvaluatorPool review), this explicit call becomes redundant and can be removed to simplify the initialization sequence.
There was a problem hiding this comment.
Code Review
This pull request introduces a WasmEvaluatorPool to enable concurrent flag evaluations in the InProcessEvaluator and adds a configuration option for the pool size. The changes also include thread-safety improvements using volatile fields and a new concurrency test. Feedback focuses on enhancing the robustness of the pool management, grouping related configuration fields into an immutable object for atomic updates, and ensuring consistent state capture during evaluation.
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a WasmEvaluatorPool to enable concurrent flag evaluations and refactors the InProcessEvaluator to use an immutable EvaluatorState for thread-safe configuration updates. Feedback suggests using Schedulers.io() for the polling interval to prevent blocking computation threads and recommends throwing an exception if the WASM pool fails to initialize properly.
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
1 participant
This PR
Addressing issue when concurrent access on the evaluator.
This issue is adding a pool for the evaluation in order to avoid sharing the instances.
Related Issues
closes #1775 thomaspoignant/go-feature-flag#5135