xiangfu0 commented on code in PR #19817:
URL: https://github.com/apache/pinot/pull/19817#discussion_r4280191657
##########
pinot-core/src/test/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManagerTest.java:
##########
@@ -987,12 +987,29 @@ public void
testDownloadAfterLocalBuildFlagsUpsertRevertWhenConsumingDuringBuild
ParallelSegmentConsumptionPolicy.ALLOW_DURING_BUILD_ONLY,
serverMetrics)) {
Semaphore semaphore =
_partitionGroupIdToConsumerCoordinatorMap.get(PARTITION_GROUP_ID).getSemaphore();
Assert.assertTrue(semaphore.tryAcquire());
+ // The policy would release, but the replace reverts upsert metadata, so
the guard keeps the permit.
segmentDataManager._releaseSemaphoreOnBuild = true;
+
Assert.assertFalse(segmentDataManager.shouldReleaseSemaphoreBeforeBuild());
runGoOnlineForCrcGuard(segmentDataManager, crcMetadata(finalOffsetValue,
12345L), false, finalOffsetValue);
Assert.assertTrue(segmentDataManager._downloadAndReplaceCalled);
- Assert.assertEquals(semaphore.availablePermits(), 1);
- verify(serverMetrics).addMeteredTableValue(anyString(),
- eq(ServerMeter.UPSERT_REVERT_WITH_CONSUMPTION_DURING_DOWNLOAD),
eq(1L));
+ Assert.assertEquals(semaphore.availablePermits(), 0);
+ verify(serverMetrics, never()).addMeteredTableValue(anyString(),
+ eq(ServerMeter.UPSERT_REVERT_WITH_CONSUMPTION_DURING_DOWNLOAD),
anyLong());
Review Comment:
Nit (non-blocking): all four cases now assert `never()` on the meter, so
despite the method name nothing positively verifies the
`UPSERT_REVERT_WITH_CONSUMPTION_DURING_DOWNLOAD` emit path
(`releasedDuringBuild && revertsUpsertMetadata()`) or the new override `warn`.
Consider one case that sets the semaphore already-released under
`ALLOW_DURING_BUILD_ONLY` + revert and asserts the meter fires once, and
asserting the warn in the `ALLOW_DURING_DOWNLOAD_ONLY` case, so those branches
don't regress silently.
--
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]
