xiangfu0 commented on code in PR #19817:
URL: https://github.com/apache/pinot/pull/19817#discussion_r4290445818
##########
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());
+ }
+
+ // ALLOW_DURING_DOWNLOAD_ONLY: releasedDuringBuild is never true here, so
only the override path can report it.
Review Comment:
These cases all exercise the upsert-revert branch (the helper stubs
`shouldRevertMetadataOnInconsistency`), but the dedup branch of
`holdsConsumerSemaphoreForMetadata()` is never covered. Since dedup is half the
point of this change, a case with `_partitionDedupMetadataManager != null` (and
no upsert manager) would guard the hold/release decision for dedup, which
otherwise diverges from upsert under the build-only policy.
--
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]
