sreejasahithi commented on code in PR #10105:
URL: https://github.com/apache/ozone/pull/10105#discussion_r3128362876
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -996,4 +997,95 @@ private void
mockInadequateReplicaUnhealthyContainerInfo(long containerID,
ContainerID.valueOf(containerID)))
.thenReturn(replicaSet);
}
+
+ /**
+ * Verifies that for EC containers the delete transaction carries the correct
+ * fields for the DN-side fix:
+ * <ul>
+ * <li>{@code totalBlockSize} = logical (unreplicated) block size – SCM's
+ * own reporting must remain accurate.</li>
+ * <li>{@code totalBlockReplicatedSize} = total physical size across all
+ * k+m shards.</li>
+ * <li>{@code requiredNodes} = k+m (9 for EC(6,3)) – the DN divides
+ * {@code totalBlockReplicatedSize} by this value to get its per-shard
+ * contribution.</li>
+ * </ul>
+ *
+ * <p>The DN then computes its pending-deletion bytes as
+ * {@code totalBlockReplicatedSize / requiredNodes}, so that Recon's cluster
+ * total ({@code sum across all DNs}) equals {@code totalBlockReplicatedSize}
+ * rather than {@code requiredNodes × logicalSize}.
+ */
+ @Test
+ public void testECContainerDeleteTransactionFieldsForDnFix() throws
Exception {
+ // EC(6,3) container: 9 required nodes.
+ final int dataBlocks = 6;
+ final int parityBlocks = 3;
+ final int chunkSize = 1024 * 1024; // 1 MB
+ ECReplicationConfig ecConfig = new ECReplicationConfig(dataBlocks,
parityBlocks,
+ ECReplicationConfig.EcCodec.RS, chunkSize);
+
+ final long containerID = 9999L;
+ ContainerInfo ecContainerInfo = new ContainerInfo.Builder()
+ .setContainerID(containerID)
+ .setReplicationConfig(ecConfig)
+ .setState(HddsProtos.LifeCycleState.CLOSED)
+ .setOwner("TestDeletedBlockLog")
+ .setPipelineID(PipelineID.randomId())
+ .build();
+ // Register the container in both the in-memory map (used by the
+ // updateDeleteTransactionId mock) and the DB table (used by getContainer).
+ ContainerID ecContainerCID = ecContainerInfo.containerID();
+ containers.put(ecContainerCID, ecContainerInfo);
+ containerTable.put(ecContainerCID, ecContainerInfo);
+ final Set<ContainerReplica> ecReplicas = dnList.stream()
+ .map(dd -> ContainerReplica.newBuilder()
+ .setContainerID(ecContainerCID)
+ .setContainerState(ContainerReplicaProto.State.CLOSED)
+ .setDatanodeDetails(dd)
+ .build())
+ .collect(Collectors.toSet());
+ replicas.put(ecContainerCID, ecReplicas);
+
+ // Simulate deletion of a single logical EC block of 64 MB.
+ long logicalSize = 64L * 1024 * 1024;
+ // Compute expected replicatedSize using the same formula as QuotaUtil.
+ long dataStripeSize = (long) dataBlocks * chunkSize;
+ long fullStripes = logicalSize / dataStripeSize;
+ long partialFirstChunk = Math.min(chunkSize, logicalSize % dataStripeSize);
+ long parityOverhead = fullStripes * parityBlocks * chunkSize
+ + partialFirstChunk * parityBlocks;
+ long expectedReplicatedSize = logicalSize + parityOverhead;
+ int requiredNodes = ecConfig.getRequiredNodes(); // = 9
+ long reminder = expectedReplicatedSize % requiredNodes;
+ List<DeletedBlock> blocks = Collections.singletonList(
+ new DeletedBlock(new BlockID(containerID, 1L), logicalSize,
expectedReplicatedSize));
+ Map<Long, List<DeletedBlock>> blockMap = new HashMap<>();
+ blockMap.put(containerID, blocks);
+
SCMDeletedBlockTransactionStatusManager.setDisableDataDistributionForTest(false);
+ deletedBlockLog.addTransactions(blockMap);
+ scmHADBTransactionBuffer.flush();
+ try (Table.KeyValueIterator<Long, DeletedBlocksTransaction> iter =
+
deletedBlockLog.getDeletedBlockLogStateManager().getReadOnlyIterator()) {
+ boolean found = false;
+ while (iter.hasNext()) {
+ DeletedBlocksTransaction tx = iter.next().getValue();
+ if (tx.getContainerID() == containerID) {
+ found = true;
+ // SCM reporting must still show the logical (unreplicated) size.
+ assertEquals(logicalSize, tx.getTotalBlockSize());
+ // Full physical cost across all k+m shards is preserved.
+ assertEquals(expectedReplicatedSize,
tx.getTotalBlockReplicatedSize());
+ // requiredNodes lets each DN compute its per-shard contribution:
+ // pendingBytes = totalBlockReplicatedSize / requiredNodes
+ assertTrue(tx.hasRequiredNodes());
+ assertEquals(requiredNodes, tx.getRequiredNodes());
+ // The per-DN contribution sums correctly to replicatedSize.
+ long perDnBytes = tx.getTotalBlockReplicatedSize() /
tx.getRequiredNodes();
+ assertEquals(expectedReplicatedSize, perDnBytes * requiredNodes +
reminder);
+ }
Review Comment:
My previous review comment still exists here
from the following two assertions
```
assertEquals(expectedReplicatedSize, tx.getTotalBlockReplicatedSize());
assertEquals(requiredNodes, tx.getRequiredNodes());
```
we know that
```
tx.getTotalBlockReplicatedSize() == expectedReplicatedSize and
tx.getRequiredNodes() == requiredNodes.
```
So by substitution, the last assertion becomes:
```
perDnBytes = expectedReplicatedSize / requiredNodes
reminder = expectedReplicatedSize % requiredNodes
assertEquals(expectedReplicatedSize,
(expectedReplicatedSize / requiredNodes) * requiredNodes
+ (expectedReplicatedSize % requiredNodes))
```
it will always hold and may not be validating additional behavior. We could
consider removing it or replacing it with an assertion that verifies the
expected perDN byte value.
--
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]
