jojochuang commented on PR #10696: URL: https://github.com/apache/ozone/pull/10696#issuecomment-5005944948
Sample repro test for the Bugbot finding about **leader change mid chunked transfer** (`ReconRDBSnapshotProvider` re-resolves `leaderInfoSupplier.get()` on every chunk while `checkLeaderConsistency` runs only once). Pushed to my fork: https://github.com/jojochuang/ozone/commit/fa988a0df78fe19fd208684efc9c37aeec80beb1 The test simulates a two-chunk transfer where the supplier returns `om-leader-a` for chunk 1 and `om-leader-b` for chunk 2. It asserts the **safe** behavior (pin original leader for all chunks **or** reset the candidate dir). On current code it **fails**, which demonstrates the bug: ``` leadersUsedPerChunk=[om-leader-a, om-leader-b], initCountBefore=1, initCountAfter=1 ``` Run locally: ```bash mvn -pl :ozone-recon test \ -Dtest=TestReconRDBSnapshotProvider#testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate \ -DskipShade -DskipRecon -DskipDocs ``` Core test code (`TestReconRDBSnapshotProvider.java`): ```java @Test public void testLeaderChangeMidTransferUsesNewLeaderWithoutResettingCandidate( @TempDir File snapshotDir) throws IOException { ServiceInfo leaderA = mock(ServiceInfo.class); when(leaderA.getHostname()).thenReturn("om-leader-a"); when(leaderA.getPort(any())).thenReturn(9874); ServiceInfo leaderB = mock(ServiceInfo.class); when(leaderB.getHostname()).thenReturn("om-leader-b"); when(leaderB.getPort(any())).thenReturn(9874); AtomicInteger supplierCalls = new AtomicInteger(); Supplier<ServiceInfo> leaderSupplier = () -> supplierCalls.getAndIncrement() == 0 ? leaderA : leaderB; List<String> leadersUsedPerChunk = new ArrayList<>(); ReconRDBSnapshotProvider provider = new ReconRDBSnapshotProvider(snapshotDir, null, false, HttpConfig.Policy.HTTP_ONLY, false, true, leaderSupplier) { @Override public void downloadSnapshot(String leaderNodeID, File targetFile) throws IOException { ServiceInfo leader = leaderSupplier.get(); leadersUsedPerChunk.add(leader.getHostname()); if (leadersUsedPerChunk.size() == 1) { Map<String, String> chunkOne = new HashMap<>(); chunkOne.put("fromLeaderA.sst", "partial-a"); chunkOne.put("CURRENT", "MANIFEST"); writeTar(targetFile, chunkOne); } else { Map<String, String> chunkTwo = new HashMap<>(); chunkTwo.put(HddsServerUtil.OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME, ""); writeTar(targetFile, chunkTwo); } } }; long initCountBefore = provider.getInitCount(); provider.downloadDBSnapshotFromLeader("leader-a-node-id"); boolean pinnedLeader = leadersUsedPerChunk.stream() .allMatch("om-leader-a"::equals); boolean resetCandidate = provider.getInitCount() > initCountBefore; assertTrue(pinnedLeader || resetCandidate, "When the resolved OM leader changes mid-transfer, Recon must either " + "pin the original leader for all chunks or reset the candidate " + "dir before continuing; leadersUsedPerChunk=" + leadersUsedPerChunk + ", initCountBefore=" + initCountBefore + ", initCountAfter=" + provider.getInitCount()); } ``` This is a repro-only sketch for discussion — not intended to land as-is on the PR branch until the fix is in place. -- 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]
