xiangfu0 commented on code in PR #20064:
URL: https://github.com/apache/pinot/pull/20064#discussion_r4395596008


##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerConfigValidationTest.java:
##########
@@ -118,6 +123,44 @@ public void testUpdateInstanceRejectsOnValidation() {
     verify(_helixDataAccessor, never()).setProperty(any(PropertyKey.class), 
any(InstanceConfig.class));
   }
 
+  @Test
+  public void testUpdateInstancePreservesFieldsWrittenAfterInitialRead() {

Review Comment:
   Nice test — it drives the real updater lambda and preservation path. 
Optional follow-ups to lock in behavior: (1) a `!updated` failure case (stub 
`updateProperty` to return `false`, assert the `RuntimeException`); (2) the 
explicit `queriesDisabled=true` legacy path (live false, body true); (3) retry 
idempotency — invoke the captured updater twice against distinct records and 
assert the final result, since Helix may call the `DataUpdater` more than once 
on a CAS conflict. None are blocking.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to