UNOMI-964: Fix flaky GraphQLListIT via live condition-type resolution - #827
Merged
Merged
Conversation
The GraphQL condition factories (Profile/Event/ProfileAlias/Topic) were lazy static singletons whose base ConditionFactory snapshotted getAllConditionTypes() once. Under PaxExam @persuite that snapshot was frozen for the whole container, captured at first get(); if taken before the condition-type cache was warm, getConditionType("profilePropertyCondition") returned null, producing a match-none active-member query for the rest of the run. This is why prior timing-based fixes never worked. - Remove the static singletons; return a fresh factory instance per get(). - Drop the conditionTypesMap snapshot; resolve types live via DefinitionsService.getConditionType(id). Also removes the retained first-request DataFetchingEnvironment/ServiceManager. - Harden AddProfileToListCommand: use (send(event) & PROFILE_UPDATED) == PROFILE_UPDATED instead of exact equality (latent bitmask bug). - Revert the misleading timing workarounds/comment in GraphQLListIT.testCRUD (deferred refresh + tripled retries). - Add unit tests for live condition-type resolution, fresh-per-get factories, and the AddProfileToListCommand bitmask handling.
…ug in RemoveProfileFromListCommand AddProfileToListCommand's PROFILE_UPDATED bitmask fix could silently save a profile even when EventService.send() also reported an ERROR bit from a nested event failure; log a warning in that case, matching the REST layer's existing convention. RemoveProfileFromListCommand had the same pre-existing exact-equality bitmask bug (`== PROFILE_UPDATED`) fixed in the original UNOMI-964 commit but was left untouched; apply the identical fix and add unit test coverage (previously untested). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Skip the unconditional mvn clean so looping a single integration test can reuse the previous reactor install. Default behavior is unchanged.
…clusters Instrument match-none condition collapse, empty list-member queries, and rollover/ILM attachment so CI logs can pin whether flakes are tenant/context, definitions-cache, or index-lifecycle related.
Time getMatchingRules against a large synthetic event-type ruleset instead of full eventService.send in PaxExam, so the optimization signal is stable and no longer fails CI on environmental noise.
…nectionDataFetcher An empty active-members result is a legitimate, common production state (a newly created list, or any list with no currently-active members), so emitting a WARN-level [unomi-diag] line on every such query polluted production logs. Remove the diagnostic, its helper, and the now-unused imports/logger; the es/os-match-none dispatcher diagnostics still cover the genuinely anomalous path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
Fixes UNOMI-964:
GraphQLListIT.testCRUDfails intermittently (locally and in CI) because the GraphQL condition factories are lazy static singletons. The baseConditionFactorysnapshotsdefinitionsService.getAllConditionTypes()once intoconditionTypesMapand never refreshes it.Under PaxExam
@PerSuite, that snapshot is frozen for the whole container, captured at the firstget()call. If that happens before the condition-type cache is warm,getConditionType("profilePropertyCondition")returnsnull, so the active-member query becomes a structuralmatch-nonefor the rest of the run:findLists.totalCount == 1still passes (fresh factory), whileactive.edgesis always empty (stale singleton). This is a structural match-none, not an indexing delay — which is why prior timing-based fixes (deferredrefreshPersistence, tripled retries) never worked.Changes
ProfileConditionFactory,EventConditionFactory,ProfileAliasConditionFactory,TopicConditionFactory; return a fresh instance perget().conditionTypesMapsnapshot inConditionFactory; resolve types live viaDefinitionsService.getConditionType(id). Also removes the retained first-requestDataFetchingEnvironment/ServiceManager.AddProfileToListCommand:(send(event) & PROFILE_UPDATED) == PROFILE_UPDATEDinstead of exact equality (latent bitmask bug on the same code path).GraphQLListIT.testCRUD.get()factories, andAddProfileToListCommandbitmask handling.Performance
Negligible. All condition-type lookups are in-memory cache reads (no ES/IO). Net-positive: removes a synchronized static bottleneck and per-container memory retention. The high-frequency ingestion path (
/context.json,/eventcollector, rule/segment evaluation) does not use these factories and is unaffected.Test plan
mvn -pl graphql/cxs-impl test -Dtest=ConditionFactoryTest,AddProfileToListCommandTest— 10/10 pass.graphql/cxs-implcompiles;iteststest-compiles.GraphQLListIT.testCRUDpasses reliably (e.g. 20 consecutive local runs on the Elasticsearch profile).returning match-none querywarnings during the run.