anton-vinogradov commented on PR #13265:
URL: https://github.com/apache/ignite/pull/13265#issuecomment-4802140972

   LGTM — root cause and fix are both correct.
   
   A couple of notes for the record:
   
   **Why only the server variant flaked:** both `@Test`s share `check()`, but 
starting a *second server* triggers rebalancing/PME while starting a *client* 
moves no data. So the initial `ScanQuery` racing an in-progress PME cleanly 
explains why `testRemoteFilterFactoryServer` misses pre-loaded entries and the 
client variant doesn't. `awaitPartitionMapExchange()` blocks on a deterministic 
condition (no `MOVING` partitions), so this is a real fix rather than a 
sleep-style band-aid, and it matches the idiom already used by ~10 other tests 
in this package. Keeping the wait in the shared `check()` also hardens the 
client variant at no cost. 👍
   
   **Latch math checks out:** 5 initial (even keys 0,2,4,6,8) + 10 updates 
(keys 10–19; `DummyEventFilterFactory` accepts all) = 15.
   
   **Minor:** the numbers in the explanation don't quite reconcile — a 2-entry 
initial scan leaves the latch at 3 (15−2−10), but the message was 
`AssertionError: 4`, which implies either a 1-entry initial scan or that one 
*update* event was also missed in that run. `awaitPartitionMapExchange()` 
covers both, just worth confirming the missed-event path is the initial scan 
only and not also CQ registration.
   
   **Residual:** the 5s `latch.await` for async CQ delivery is the only 
remaining timing dependency — standard and fine, flagging for completeness.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to