perf(decision): add evaluator p99 gate and free the connection during evaluation - #187
Merged
Merged
Conversation
… evaluation Add an in-process latency gate asserting p99 under 10ms for evaluating a 100-condition rule (warm-up then nearest-rank p99 over the timed evaluate calls), so a future evaluator regression fails on CI. Stop holding a JDBC connection across the bounded evaluation: evaluate is no longer transactional and the audit-log write owns its own short transaction, so the up-to-timeout evaluation no longer pins a pooled connection. Reads stay simple finds on immutable data and fail-closed is preserved (the log is written only after a successful evaluation). A reflection test pins the moved boundary. Closes #176
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Two performance concerns for the decision engine (#176):
(A) Evaluator p99 latency gate. A pure, in-process JUnit gate (
EvaluatorLatencyGateTest) parses one 100-conditionallrule via the publicRuleParser, warms up the JIT, then timesRuleEvaluator.evaluateover 10k iterations (nanoTime around the call only) and asserts nearest-rank p99 < 10ms. Deterministic, no I/O; a regression ceiling on the hot path. Stable across repeated local runs with ~100x headroom.(B) Free the JDBC connection during evaluation.
EvaluationServiceImpl.evaluatewas@Transactional, so it held a pooled connection across the up-to-200ms bounded evaluation (open-in-viewis alreadyfalse). The orchestrator is now non-transactional andDecisionLogWriter.writeowns its own short transaction (a separate bean, so the proxy boundary is honored - no self-invocation). The bounded evaluation now runs with no connection bound to the thread; a connection is taken only for the brief audit-log write.Correctness
decision_logsrow. Covered by the existing unit test (verify(exactly = 0)) and end-to-end byDecisionEvaluationApiIT(one row on success, zero on timeout).RuleVersionEntity.dsl/id,DecisionRuleEntity.activeVersionId); no lazy association is traversed outside a transaction.EvaluationTransactionBoundaryTest) pins the moved boundary so the connection-hold cannot silently regress.Tests
4 new tests (2 perf + 2 structural); existing
EvaluationServiceImplTest(4) unchanged and green.DecisionEvaluationApiITis the CI verifier for (B).Gates
critic GO (no must-fixes; verified eager columns,
allno short-circuit, separate-bean proxy) -> code-reviewer APPROVE-WITH-NITS (member-reference reflection, trimmed KDoc, shouldBeLessThan - all folded) -> security-auditor PASS (fail-closed impossible-to-write-on-failure, no regression, connection released, boundary/secrets clean) -> evaluator 0.93 PASS.Note: the same read-only connection-hold exists in
ReplayServiceImpl(@Transactional(readOnly), writes nothing); deferred as read-only and out of scope for #176.Closes #176