Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 merged PR #1: URL: https://github.com/apache/ozone/pull/1 -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3328959533
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -158,52 +177,36 @@ public PendingContainerTracker(long maxContainerSize,
long rollIntervalMs, SCMNo
}
/**
- * Whether the datanode can fit another container of {@link
#maxContainerSize} after accounting for
- * SCM pending allocations for {@code node} (this tracker) and usable space
across volumes on
- * {@code datanodeInfo}. Pending bytes are count × {@code maxContainerSize};
- * effective allocatable space sums full-container slots per storage report.
+ * Atomically checks if the datanode has space for a new container and
records the allocation
+ * if space is available. The check-and-add atomicity is enforced inside
+ * {@link TwoWindowBucket#checkSpaceAndAdd}.
*
- * @param datanodeInfo storage reports for the datanode
+ * @param datanodeInfo datanode whose storage reports and pending bucket
+ * @param containerID the container being allocated
+ * @return true if space was available and the allocation was recorded,
false otherwise
*/
- public boolean hasEffectiveAllocatableSpaceForNewContainer(DatanodeInfo
datanodeInfo) {
+ public boolean checkSpaceAndRecordAllocation(DatanodeInfo datanodeInfo,
ContainerID containerID) {
Objects.requireNonNull(datanodeInfo, "datanodeInfo == null");
+Objects.requireNonNull(containerID, "containerID == null");
-long pendingAllocationSize =
datanodeInfo.getPendingContainerAllocations().getCount() * maxContainerSize;
List storageReports = datanodeInfo.getStorageReports();
Objects.requireNonNull(storageReports, "storageReports == null");
if (storageReports.isEmpty()) {
return false;
}
-long effectiveAllocatableSpace = 0L;
-for (StorageReportProto report : storageReports) {
- long usableSpace = VolumeUsage.getUsableSpace(report);
- long containersOnThisDisk = usableSpace / maxContainerSize;
- effectiveAllocatableSpace += containersOnThisDisk * maxContainerSize;
- if (effectiveAllocatableSpace - pendingAllocationSize >=
maxContainerSize) {
-return true;
- }
-}
-if (metrics != null) {
- metrics.incNumSkippedFullNodeContainerAllocation();
-}
-return false;
- }
- /**
- * Record a pending container allocation for a single DataNode.
- * Container is added to the current window.
- *
- * @param containerID The container being allocated/replicated
- */
- public void recordPendingAllocationForDatanode(DatanodeInfo datanodeInfo,
ContainerID containerID) {
-Objects.requireNonNull(containerID, "containerID == null");
-if (datanodeInfo == null) {
- return;
-}
-final boolean added =
datanodeInfo.getPendingContainerAllocations().add(containerID);
-if (added && metrics != null) {
- metrics.incNumPendingContainersAdded();
+boolean added = datanodeInfo.getPendingContainerAllocations()
+.checkSpaceAndAdd(storageReports, maxContainerSize, containerID);
+if (added) {
+ if (metrics != null) {
+metrics.incNumPendingContainersAdded();
+ }
+} else {
+ if (metrics != null) {
+metrics.incNumSkippedFullNodeContainerAllocation();
+ }
}
Review Comment:
Check null first:
```java
if (metrics != null) {
if (added) {
metrics.incNumPendingContainersAdded();
} else {
metrics.incNumSkippedFullNodeContainerAllocation();
}
}
```
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java:
##
@@ -634,22 +636,23 @@ private boolean isOpenWithUnregisteredNodes(Pipeline
pipeline) {
}
@Override
- public boolean hasEnoughSpace(Pipeline pipeline) {
-for (DatanodeDetails node : pipeline.getNodes()) {
- if (!nodeManager.hasSpaceForNewContainerAllocation(node.getID())) {
+ public boolean checkSpaceAndRecordAllocation(Pipeline pipeline, ContainerID
containerID) {
+List successfulNodes = new ArrayList<>();
+for (DatanodeDetails dn : pipeline.getNodes()) {
Review Comment:
We should getDatanodeInfo first:
```java
public boolean checkSpaceAndRecordAllocation(Pipeline pipeline,
ContainerID containerID) {
final Set datanodeDetails = pipeline.getNodeSet();
final List datanodeInfos = new
ArrayList<>(datanodeDetails.size());
for (DatanodeDetails dn : datanodeDetails) {
final DatanodeInfo info = nodeManager.getDatanodeInfo(dn);
if (info == null) {
LOG.warn("DatanodeInfo not found for {}", dn.getID());
return false;
}
datanodeInfos.add(info);
}
final List successfulNodes = new
ArrayList<>(datanodeInfos.size());
for (DatanodeInfo dn : datanodeInfos) {
if (!nodeMa
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3328941800
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java:
##
@@ -634,22 +636,23 @@ private boolean isOpenWithUnregisteredNodes(Pipeline
pipeline) {
}
@Override
- public boolean hasEnoughSpace(Pipeline pipeline) {
-for (DatanodeDetails node : pipeline.getNodes()) {
- if (!nodeManager.hasSpaceForNewContainerAllocation(node.getID())) {
+ public boolean checkSpaceAndRecordAllocation(Pipeline pipeline, ContainerID
containerID) {
+List successfulNodes = new ArrayList<>();
+for (DatanodeDetails dn : pipeline.getNodes()) {
Review Comment:
We should getDatanodeInfo first:
```java
public boolean checkSpaceAndRecordAllocation(Pipeline pipeline,
ContainerID containerID) {
final Set datanodeDetails = pipeline.getNodeSet();
final List datanodeInfos = new
ArrayList<>(datanodeDetails.size());
for (DatanodeDetails dn : datanodeDetails) {
final DatanodeInfo info = nodeManager.getDatanodeInfo(dn);
if (info == null) {
LOG.warn("DatanodeInfo not found for {}", dn.getID());
return false;
}
datanodeInfos.add(info);
}
final List successfulNodes = new
ArrayList<>(datanodeInfos.size());
for (DatanodeInfo dn : datanodeInfos) {
if (!nodeManager.checkSpaceAndRecordAllocation(dn, containerID)) {
for (DatanodeInfo rollbackNode : successfulNodes) {
nodeManager.removePendingAllocationForDatanode(rollbackNode,
containerID);
}
return false;
}
successfulNodes.add(dn);
}
return true;
}
```
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3328941800
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java:
##
@@ -634,22 +636,23 @@ private boolean isOpenWithUnregisteredNodes(Pipeline
pipeline) {
}
@Override
- public boolean hasEnoughSpace(Pipeline pipeline) {
-for (DatanodeDetails node : pipeline.getNodes()) {
- if (!nodeManager.hasSpaceForNewContainerAllocation(node.getID())) {
+ public boolean checkSpaceAndRecordAllocation(Pipeline pipeline, ContainerID
containerID) {
+List successfulNodes = new ArrayList<>();
+for (DatanodeDetails dn : pipeline.getNodes()) {
Review Comment:
We should getDatanodeInfo first:
```java
public boolean checkSpaceAndRecordAllocation(Pipeline pipeline,
ContainerID containerID) {
final Set datanodeDetails = pipeline.getNodeSet();
final List datanodeInfos = new
ArrayList<>(datanodeDetails.size());
for (DatanodeDetails dn : datanodeDetails) {
final DatanodeInfo info = nodeManager.getDatanodeInfo(dn);
if (info == null) {
LOG.warn("DatanodeInfo not found for {}", dn.getID());
return false;
}
datanodeInfos.add(info);
}
final List successfulNodes = new
ArrayList<>(datanodeInfos.size());
for (DatanodeInfo dn : datanodeInfos) {
if (!nodeManager.checkSpaceAndRecordAllocation(dn, containerID)) {
for (DatanodeInfo rollbackNode : successfulNodes) {
nodeManager.removePendingAllocationForDatanode(rollbackNode,
containerID);
}
return false;
}
successfulNodes.add(dn);
}
return true;
}
```
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4567385707 > @ashishkumar50 , sorry for the late reply. Will review this soon. > > BTW, could you resolve the conflict? @szetszwo, conflicts are resolved. Can you please take a look. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4511657887 @ashishkumar50 , sorry for the late reply. Will review this soon. BTW, could you resolve the conflict? -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4427568023 @szetszwo Can you please take a look, all the comments are addressed. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4378389820 > > ... hasEnoughSpace and recordPendingAllocation methods as we need this during replication which is used by placement policy for space check. ... > > @ashishkumar50 , hasEnoughSpace is unusable since it only checks the space but not updates the value. It will have synchronization issue similar to [#1 (comment)](https://github.com/apache/ozone/pull/1#discussion_r3162988799) I have removed for `hasEnoughSpace,` from this PR but as i said we need it for placement policy which is used by replication. I wanted to allow placement policy to use `hasEnoughSpace` and RM to use `checkSpaceAndAdd`. Because placement policy doesn't require container for checking space availability for a DN. We can discuss more about this in [HDDS-14924](https://issues.apache.org/jira/browse/HDDS-14924). -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3187594757
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -189,6 +190,47 @@ public boolean
hasEffectiveAllocatableSpaceForNewContainer(DatanodeInfo datanode
return false;
}
+ /**
+ * Atomically checks if the datanode has space for a new container and
records the allocation
+ * if space is available. This prevents race conditions where multiple
threads check space
+ * concurrently and over-allocate.
+ *
+ * @param datanodeInfo storage reports for the datanode
+ * @param containerID The container being allocated
+ * @return true if space was available and allocation was recorded, false
otherwise
+ */
+ public boolean checkSpaceAndRecordAllocation(DatanodeInfo datanodeInfo,
ContainerID containerID) {
+Objects.requireNonNull(datanodeInfo, "datanodeInfo == null");
+Objects.requireNonNull(containerID, "containerID == null");
+
+TwoWindowBucket bucket = datanodeInfo.getPendingContainerAllocations();
+synchronized (bucket) {
Review Comment:
Moved to TwoWindowBucket.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3186869795
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -189,6 +190,47 @@ public boolean
hasEffectiveAllocatableSpaceForNewContainer(DatanodeInfo datanode
return false;
}
+ /**
+ * Atomically checks if the datanode has space for a new container and
records the allocation
+ * if space is available. This prevents race conditions where multiple
threads check space
+ * concurrently and over-allocate.
+ *
+ * @param datanodeInfo storage reports for the datanode
+ * @param containerID The container being allocated
+ * @return true if space was available and allocation was recorded, false
otherwise
+ */
+ public boolean checkSpaceAndRecordAllocation(DatanodeInfo datanodeInfo,
ContainerID containerID) {
+Objects.requireNonNull(datanodeInfo, "datanodeInfo == null");
+Objects.requireNonNull(containerID, "containerID == null");
+
+TwoWindowBucket bucket = datanodeInfo.getPendingContainerAllocations();
+synchronized (bucket) {
Review Comment:
We should not synchronized local variables; see
https://github.com/user-attachments/assets/6b481856-fdeb-406d-b2e8-a68e5f44f83a";
/>
Move the code to TwoWindowBucket.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4376658856 @szetszwo Thanks for the review, as i mentioned in https://github.com/apache/ozone/pull/1#discussion_r3172549299, we need hasEnoughSpace and recordPendingAllocation methods so i haven't removed it. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3183672411
##
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java:
##
Review Comment:
recordPendingAllocation(..) is not used anywhere. Please remove it.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java:
##
@@ -232,6 +232,17 @@ void reinitialize(Table
pipelineStore)
*/
boolean hasEnoughSpace(Pipeline pipeline);
Review Comment:
hasEnoughSpace(..) is now only used in tests. Please remove it.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
Review Comment:
hasSpaceForNewContainerAllocation and
hasEffectiveAllocatableSpaceForNewContainer are also become unused.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java:
##
@@ -153,6 +154,7 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
containerReport.getReportsList();
final Set expectedContainersInDatanode =
getNodeManager().getContainers(datanodeDetails);
+DatanodeInfo datanodeInfo =
getNodeManager().getDatanodeInfo(datanodeDetails);
Review Comment:
datanodeDetails and datanodeInfo are the same object; see HDDS-15146.
##
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java:
##
@@ -335,7 +335,12 @@ public boolean isPipelineCreationFrozen() {
@Override
public boolean hasEnoughSpace(Pipeline pipeline) {
-return false;
+for (DatanodeDetails node : pipeline.getNodes()) {
+ if (!nodeManager.hasSpaceForNewContainerAllocation(node.getID())) {
Review Comment:
After hasEnoughSpace(..) is removed, hasSpaceForNewContainerAllocation(..)
is also only used in tests. Please remove it.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3182504580
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
@@ -1103,6 +1103,27 @@ public void
recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerI
pendingContainerTracker.recordPendingAllocationForDatanode(datanodeInfo,
containerID);
}
+ @Override
+ public boolean checkSpaceAndRecordAllocation(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", datanodeID);
+ return false;
+}
+return pendingContainerTracker.checkSpaceAndRecordAllocation(datanodeInfo,
containerID);
+ }
+
+ @Override
+ public void removePendingAllocationForDatanode(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
Review Comment:
Moved
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3175500259
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
@@ -1103,6 +1103,27 @@ public void
recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerI
pendingContainerTracker.recordPendingAllocationForDatanode(datanodeInfo,
containerID);
}
+ @Override
+ public boolean checkSpaceAndRecordAllocation(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", datanodeID);
+ return false;
+}
+return pendingContainerTracker.checkSpaceAndRecordAllocation(datanodeInfo,
containerID);
+ }
+
+ @Override
+ public void removePendingAllocationForDatanode(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
Review Comment:
@ashishkumar50 , we should get datanodeInfo outside the loop instead looking
the same datanodeInfo up for each container.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3175500259
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
@@ -1103,6 +1103,27 @@ public void
recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerI
pendingContainerTracker.recordPendingAllocationForDatanode(datanodeInfo,
containerID);
}
+ @Override
+ public boolean checkSpaceAndRecordAllocation(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", datanodeID);
+ return false;
+}
+return pendingContainerTracker.checkSpaceAndRecordAllocation(datanodeInfo,
containerID);
+ }
+
+ @Override
+ public void removePendingAllocationForDatanode(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
Review Comment:
@ashishkumar50 , we should get datanodeInfo outside the loop instead get it
for each element.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
@@ -1103,6 +1103,27 @@ public void
recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerI
pendingContainerTracker.recordPendingAllocationForDatanode(datanodeInfo,
containerID);
}
+ @Override
+ public boolean checkSpaceAndRecordAllocation(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", datanodeID);
+ return false;
+}
+return pendingContainerTracker.checkSpaceAndRecordAllocation(datanodeInfo,
containerID);
+ }
+
+ @Override
+ public void removePendingAllocationForDatanode(DatanodeID datanodeID,
ContainerID containerID) {
+DatanodeInfo datanodeInfo = getNode(datanodeID);
Review Comment:
@ashishkumar50 , we should get datanodeInfo outside the loop instead get it
for each container.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1: URL: https://github.com/apache/ozone/pull/1#discussion_r3172549299 ## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java: ## @@ -269,6 +269,7 @@ private ContainerInfo allocateContainer(final Pipeline pipeline, } containerStateManager.addContainer(containerInfoBuilder.build()); +pipelineManager.recordPendingAllocation(pipeline, containerID); Review Comment: I have made `hasEnoughSpace` and `recordPendingAllocation` atomic and will be used during new container allocation. But still i have kept `hasEnoughSpace` and `recordPendingAllocation` methods as we need this during replication which is used by placement policy for space check. In placement policy there is no container id so it can just check for space and later record by replication manager. There is already subtask for it HDDS-14924. If there are some over allocation of container during replication it should be accepted by datanode soft and hard limit check which is being handled in https://github.com/apache/ozone/pull/10054. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3162988799
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -269,6 +269,7 @@ private ContainerInfo allocateContainer(final Pipeline
pipeline,
}
containerStateManager.addContainer(containerInfoBuilder.build());
+pipelineManager.recordPendingAllocation(pipeline, containerID);
Review Comment:
hasEnoughSpace and recordPendingAllocation should be atomic.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java:
##
@@ -175,6 +177,13 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
if (!alreadyInDn) {
// This is a new Container not in the nodeManager -> dn map yet
getNodeManager().addContainer(datanodeDetails, cid);
+// Remove from pending tracker when container is added to DN
+// This container was just confirmed for the first time on this DN
+PendingContainerTracker tracker =
getNodeManager().getPendingContainerTracker();
+DatanodeInfo datanodeInfo =
getNodeManager().getDatanodeInfo(datanodeDetails);
Review Comment:
We should get tracker and datanodeInfo outside the loop.
BTW, the datanodeDetails object actually is already a DatanodeInfo since
SCMNodeManager.getNode(..) returns DatanodeInfo; filed HDDS-15146.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##
@@ -103,6 +105,11 @@ protected void
processICR(IncrementalContainerReportFromDatanode report,
}
if (ContainerReportValidator.validate(container, dd, replicaProto)) {
processContainerReplica(dd, container, replicaProto, publisher,
detailsForLogging);
+PendingContainerTracker tracker =
getNodeManager().getPendingContainerTracker();
+DatanodeInfo datanodeInfo = getNodeManager().getDatanodeInfo(dd);
Review Comment:
Similarly, we should get tracker and datanodeInfo outside the loop.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4343026573 @szetszwo updated this PR and fixed comments. Can you please take a look, thanks. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4316003123 > @ashishkumar50 , thanks for the update! > > Let's have another subtask to do some refactoring: > > * Remove DatanodeBuckets map. Add TwoWindowBucket to DatanodeInfo > * Remove containerSize parameter from pipelineManager.hasEnoughSpace(pipeline, containerSize)). > * Remove the DatanodeDetails parameter from hasEffectiveAllocatableSpaceForNewContainer(..) since datanodeInfo already has all the information. > * PendingContainerTracker should use only DatanodeID and DatanodeInfo but never DatanodeDetails. @szetszwo raised PR https://github.com/apache/ozone/pull/10124 for refactor code -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
szetszwo commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3132909987
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##
@@ -225,6 +241,40 @@ private void unregisterMXBean() {
}
}
+ @Override
+ public PendingContainerTracker getPendingContainerTracker() {
+return pendingContainerTracker;
+ }
+
+ /**
+ * Effective space check aligned with container allocation: per-disk slot
model minus
+ * SCM pending allocations.
+ */
+ @Override
+ public boolean hasSpaceForNewContainerAllocation(DatanodeDetails node) {
+Objects.requireNonNull(node, "node==null");
+try {
+ DatanodeInfo datanodeInfo = getDatanodeInfo(node);
+ if (datanodeInfo == null) {
+LOG.warn("DatanodeInfo not found for node {}", node.getUuidString());
+return false;
+ }
+ return
pendingContainerTracker.hasEffectiveAllocatableSpaceForNewContainer(
+ node, datanodeInfo);
Review Comment:
We should remove the DatanodeDetails parameter from
hasEffectiveAllocatableSpaceForNewContainer(..) since datanodeInfo already has
all the information.
##
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##
@@ -138,10 +138,31 @@ public class ScmConfig extends ReconfigurableConfig {
)
private int transactionToDNsCommitMapLimit = 500;
+ @Config(key = "hdds.scm.container.pending.allocation.roll.interval",
Review Comment:
The other `ozone.scm.container.*` confs are in ScmConfigKeys. Let's move
this new conf to ScmConfigKeys.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java:
##
@@ -175,6 +176,17 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
if (!alreadyInDn) {
// This is a new Container not in the nodeManager -> dn map yet
getNodeManager().addContainer(datanodeDetails, cid);
+
+// Remove from pending tracker when container is added to DN
+// This container was just confirmed for the first time on this DN
+// No need to remove on subsequent reports (it's already been
removed)
+if (container != null) {
+ PendingContainerTracker tracker =
+ getNodeManager().getPendingContainerTracker();
+ if (tracker != null) {
Review Comment:
tracker seems never null.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4280063160 @szetszwo @rakeshadr Updated this PR to use pending container tracker during container creation and remove container tracking during ICR/FCR processing. Can you please take a look. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4234240298 > @ashishkumar50 , thanks for working on this! > > This change is quite big and complicated. Let's split it into multiple subtasks. The first one could be adding the new `PendingContainerTracker` class and the related test. > > (Sorry for reviewing this late.) @szetszwo Here is the first split PR https://github.com/apache/ozone/pull/10073 -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3050030672
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = new HashSet<>();
+private Set previousWindow = new HashSet<>();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1: URL: https://github.com/apache/ozone/pull/1#discussion_r3050056386 ## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java: ## Review Comment: @aswinshakil Some allocation may happen extra but that will be taken care by [PR](https://github.com/apache/ozone/pull/10054) as DN has more buffer to handle of these extra allocation. In my opinion synchronizing completely will be too costly here. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3050030672
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = new HashSet<>();
+private Set previousWindow = new HashSet<>();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on PR #1: URL: https://github.com/apache/ozone/pull/1#issuecomment-4204786858 @rakeshadr @aswinshakil Fixed review comments. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3050013136
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = new HashSet<>();
+private Set previousWindow = new HashSet<>();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
aswinshakil commented on code in PR #1: URL: https://github.com/apache/ozone/pull/1#discussion_r3048384046 ## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java: ## Review Comment: @rakeshadr @ashishkr200 `pipelineManager.hasEnoughSpace` is not a synchronized operation on the datanodes. We take lock on the pipeline `synchronized (pipeline.getId())`. Still two different pipelines can still allocate container on the same datanode. We record the pending allocation on only Line 289. Until the code reaches that point multiple pipeline will be allocated to the same space on the datanode. Let's say we only have 6GB space available in d1. Pipeline 1: d1, d2, d3 Pipeline 2: d1, d4, d5 When we allocate container on both the pipeline, will get past `pipelineManager.hasEnoughSpace` and both will be allocated container 5GB *2 = 10GB. But we only have 6GB on d1 -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
aswinshakil commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3048152699
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java:
##
@@ -227,4 +228,11 @@ void reinitialize(Table
pipelineStore)
* Get the pipeline metrics.
*/
SCMPipelineMetrics getMetrics();
+
+ /**
+ * Get DatanodeInfo for a specific DataNode which includes per-volume
storage reports.
+ * @param datanodeDetails The datanode to get info for
+ * @return DatanodeInfo containing detailed node information including
per-disk stats, or null if not available
+ */
+ DatanodeInfo getDatanodeInfo(DatanodeDetails datanodeDetails);
Review Comment:
Looks like this is not used, It can be removed.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
rakeshadr commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3045242417
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+private Set previousWindow = ConcurrentHashMap.newKeySet();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now - lastRollTime >= rollIntervalMs) {
+// Shift: current becomes previous
+previousWindow = currentWindow;
+// Reset: new empty current window
+currentWindow = ConcurrentHashMap.newKeySet();
+lastRollTime = now;
+LOG.debug("Rolled window. Previous window size: {}, Current window
reset to empty", previousWindow.size());
+ }
+}
+
+/**
+ * Get union of both windows (all pending containers).
+ */
+synchronized Set getAll
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
rakeshadr commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3044878303
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = new HashSet<>();
+private Set previousWindow = new HashSet<>();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
rakeshadr commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3044295093
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.node;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import
org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.ozone.container.common.volume.VolumeUsage;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks per-datanode pending container allocations at SCM using a Two Window
Tumbling Bucket
+ * pattern (similar to HDFS HADOOP-3707).
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 5 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:02 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:05 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:07 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:08 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:10 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ * Thread-safety: all mutations and reads for a given datanode run under
+ * {@code synchronized (bucket)} on that datanode's {@link TwoWindowBucket},
including
+ * map insert/remove, so concurrent record/remove/report paths cannot
interleave or orphan
+ * a bucket after it was dropped from the map.
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending.allocation.roll.interval.
+ * Default: 5 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers (same instance as {@link
SCMNodeManager}'s node metrics).
+ */
+ private final SCMNodeMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = new HashSet<>();
+private Set previousWindow = new HashSet<>();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1: URL: https://github.com/apache/ozone/pull/1#discussion_r3043314221 ## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java: ## Review Comment: Checked other places already have safeguard. [SCMClientProtocolServer.java#L258](https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java#L258) is used only for test or tools. Added a safeguard here. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
rakeshadr commented on code in PR #1: URL: https://github.com/apache/ozone/pull/1#discussion_r3038092049 ## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java: ## Review Comment: Since this PR is adding more stricter/defensive "two-window tumbling bucket" logic, there is a high chance to hit the `return null` code path flow. Please double check all the callers of API [ContainerManagerImpl#allocateContainer()](https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java#L245) then safeguard with null check, otw it would result in NPE. For example, [SCMClientProtocolServer.java#L258](https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java#L258), this would hit NPE. -- 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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3036001082
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+private Set previousWindow = ConcurrentHashMap.newKeySet();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
Review Comment:
Roll will be done when node report comes which is every minute, so old
entries will get clear.
--
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: issues-unsubsc
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035999302
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -242,12 +262,75 @@ private ContainerInfo createContainer(Pipeline pipeline,
String owner)
return containerInfo;
}
+ /**
+ * Check if a pipeline has sufficient space after considering pending
allocations.
+ * Tracks containers scheduled but not yet written to DataNodes, preventing
over-allocation.
+ *
+ * @param pipeline The pipeline to check
+ * @return true if sufficient space is available, false otherwise
+ */
+ private boolean hasSpaceAfterPendingAllocations(Pipeline pipeline) {
+try {
+ for (DatanodeDetails node : pipeline.getNodes()) {
+// Get DN's storage statistics
+DatanodeInfo datanodeInfo = pipelineManager.getDatanodeInfo(node);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", node.getUuidString());
+ return false;
+}
+
+List storageReports =
datanodeInfo.getStorageReports();
+if (storageReports == null || storageReports.isEmpty()) {
+ LOG.warn("No storage reports for node {}", node.getUuidString());
+ return false;
+}
+
+// Calculate total capacity and effective allocatable space
+// For each disk, calculate how many containers can actually fit,
+// since containers are written to individual disks, not spread across
them.
+// Example: disk1=9GB, disk2=14GB with 5GB containers
+// (1*5GB) + (2*5GB) = 15GB → actually 3 containers
+long totalCapacity = 0L;
+long effectiveAllocatableSpace = 0L;
+for (StorageReportProto report : storageReports) {
+ totalCapacity += report.getCapacity();
+ long usableSpace = VolumeUsage.getUsableSpace(report);
+ // Calculate how many containers can fit on this disk
+ long containersOnThisDisk = usableSpace / maxContainerSize;
+ // Add effective space (containers that fit * container size)
+ effectiveAllocatableSpace += containersOnThisDisk * maxContainerSize;
+}
+
+// Get pending allocations from tracker
+long pendingAllocations =
pendingContainerTracker.getPendingAllocationSize(node);
+
+// Calculate effective remaining space after pending allocations
+long effectiveRemaining = effectiveAllocatableSpace -
pendingAllocations;
+
+// Check if there's enough space for a new container
+if (effectiveRemaining < maxContainerSize) {
Review Comment:
No need of extra buffer here, as we are anyway going to give buffer in DN by
considering soft and hard limit. So in case of some overflow DN will accept it
until hard limit.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035996607
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java:
##
@@ -175,6 +175,15 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
if (!alreadyInDn) {
// This is a new Container not in the nodeManager -> dn map yet
getNodeManager().addContainer(datanodeDetails, cid);
+
+// Remove from pending tracker when container is added to DN
+// This container was just confirmed for the first time on this DN
+// No need to remove on subsequent reports (it's already been
removed)
+if (container != null && getContainerManager() instanceof
ContainerManagerImpl) {
Review Comment:
Same now
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3036009750
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -242,12 +262,75 @@ private ContainerInfo createContainer(Pipeline pipeline,
String owner)
return containerInfo;
}
+ /**
+ * Check if a pipeline has sufficient space after considering pending
allocations.
+ * Tracks containers scheduled but not yet written to DataNodes, preventing
over-allocation.
+ *
+ * @param pipeline The pipeline to check
+ * @return true if sufficient space is available, false otherwise
+ */
+ private boolean hasSpaceAfterPendingAllocations(Pipeline pipeline) {
+try {
+ for (DatanodeDetails node : pipeline.getNodes()) {
+// Get DN's storage statistics
+DatanodeInfo datanodeInfo = pipelineManager.getDatanodeInfo(node);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", node.getUuidString());
+ return false;
+}
+
+List storageReports =
datanodeInfo.getStorageReports();
+if (storageReports == null || storageReports.isEmpty()) {
+ LOG.warn("No storage reports for node {}", node.getUuidString());
+ return false;
+}
+
+// Calculate total capacity and effective allocatable space
+// For each disk, calculate how many containers can actually fit,
+// since containers are written to individual disks, not spread across
them.
+// Example: disk1=9GB, disk2=14GB with 5GB containers
+// (1*5GB) + (2*5GB) = 15GB → actually 3 containers
+long totalCapacity = 0L;
+long effectiveAllocatableSpace = 0L;
+for (StorageReportProto report : storageReports) {
+ totalCapacity += report.getCapacity();
+ long usableSpace = VolumeUsage.getUsableSpace(report);
+ // Calculate how many containers can fit on this disk
+ long containersOnThisDisk = usableSpace / maxContainerSize;
+ // Add effective space (containers that fit * container size)
+ effectiveAllocatableSpace += containersOnThisDisk * maxContainerSize;
+}
+
+// Get pending allocations from tracker
+long pendingAllocations =
pendingContainerTracker.getPendingAllocationSize(node);
+
+// Calculate effective remaining space after pending allocations
+long effectiveRemaining = effectiveAllocatableSpace -
pendingAllocations;
+
+// Check if there's enough space for a new container
+if (effectiveRemaining < maxContainerSize) {
+ LOG.info("Node {} insufficient space: capacity={}, effective
allocatable={}, " +
+ "pending allocations={}, effective remaining={},
required={}",
+ node.getUuidString(), totalCapacity, effectiveAllocatableSpace,
+ pendingAllocations, effectiveRemaining, maxContainerSize);
+ return false;
+}
+ }
+
+ return true;
+} catch (Exception e) {
+ LOG.warn("Error checking space for pipeline {}", pipeline.getId(), e);
+ return true;
Review Comment:
Moved the code, there is no exception here.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035996309
##
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##
@@ -138,10 +138,31 @@ public class ScmConfig extends ReconfigurableConfig {
)
private int transactionToDNsCommitMapLimit = 500;
+ @Config(key = "hdds.scm.container.pending-allocation.roll-interval",
+ defaultValue = "10m",
+ type = ConfigType.TIME,
+ tags = { ConfigTag.SCM, ConfigTag.CONTAINER },
+ description =
+ "Time interval for rolling the pending container allocation window.
" +
+ "Pending container allocations are tracked in a two-window
tumbling bucket " +
+ "pattern. Each window has this duration. " +
+ "After 2x this interval, allocations that haven't been confirmed
via " +
+ "container reports will automatically age out. Default is 10
minutes."
+ )
+ private Duration pendingContainerAllocationRollInterval =
Duration.ofMinutes(10);
Review Comment:
5 minute
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035997148
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java:
##
@@ -175,6 +175,15 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
if (!alreadyInDn) {
// This is a new Container not in the nodeManager -> dn map yet
getNodeManager().addContainer(datanodeDetails, cid);
+
+// Remove from pending tracker when container is added to DN
+// This container was just confirmed for the first time on this DN
+// No need to remove on subsequent reports (it's already been
removed)
+if (container != null && getContainerManager() instanceof
ContainerManagerImpl) {
Review Comment:
Moved to node package, so not required these conversions now.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035994340
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##
@@ -103,6 +112,13 @@ protected void
processICR(IncrementalContainerReportFromDatanode report,
}
if (ContainerReportValidator.validate(container, dd, replicaProto)) {
processContainerReplica(dd, container, replicaProto, publisher,
detailsForLogging);
+
+// Remove from pending tracker when container is added to DN
+if (!alreadyOnDn && getContainerManager() instanceof
ContainerManagerImpl) {
+ ((ContainerManagerImpl) getContainerManager())
+ .getPendingContainerTracker()
+ .removePendingAllocation(dd, id);
Review Comment:
Added roll in every node report which is per minute from DN.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035995487
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+private Set previousWindow = ConcurrentHashMap.newKeySet();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now - lastRollTime >= rollIntervalMs) {
+// Shift: current becomes previous
+previousWindow = currentWindow;
+// Reset: new empty current window
+currentWindow = ConcurrentHashMap.newKeySet();
+lastRollTime = now;
+LOG.debug("Rolled window. Previous window size: {}, Current window
reset to empty", previousWindow.size());
+ }
+}
+
+/**
+ * Get union of both windows (all pending containers).
+ */
+synchronized Set ge
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035995302
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+private Set previousWindow = ConcurrentHashMap.newKeySet();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now - lastRollTime >= rollIntervalMs) {
+// Shift: current becomes previous
+previousWindow = currentWindow;
+// Reset: new empty current window
+currentWindow = ConcurrentHashMap.newKeySet();
+lastRollTime = now;
+LOG.debug("Rolled window. Previous window size: {}, Current window
reset to empty", previousWindow.size());
+ }
+}
+
+/**
+ * Get union of both windows (all pending containers).
+ */
+synchronized Set ge
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3035994917
##
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestPendingContainerTracker.java:
##
@@ -0,0 +1,375 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.IOException;
+import java.util.Set;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
+import org.apache.hadoop.hdds.scm.pipeline.MockPipeline;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for PendingContainerTracker.
+ */
+public class TestPendingContainerTracker {
+
+ private static final long MAX_CONTAINER_SIZE = 5L * 1024 * 1024 * 1024; //
5GB
+
+ private PendingContainerTracker tracker;
+ private Pipeline pipeline;
+ private DatanodeDetails dn1;
+ private DatanodeDetails dn2;
+ private DatanodeDetails dn3;
+ private ContainerID container1;
+ private ContainerID container2;
+ private ContainerID container3;
+
+ @BeforeEach
+ public void setUp() throws IOException {
+tracker = new PendingContainerTracker(MAX_CONTAINER_SIZE);
+
+// Create a 3-node Ratis pipeline
+pipeline = MockPipeline.createPipeline(3);
+dn1 = pipeline.getNodes().get(0);
+dn2 = pipeline.getNodes().get(1);
+dn3 = pipeline.getNodes().get(2);
+
+container1 = ContainerID.valueOf(1L);
+container2 = ContainerID.valueOf(2L);
+container3 = ContainerID.valueOf(3L);
+ }
+
+ @Test
+ public void testRecordPendingAllocation() {
+// Initially no pending containers
+assertEquals(0, tracker.getPendingContainers(dn1).size());
+assertEquals(0, tracker.getPendingAllocationSize(dn1));
+
+// Record a pending allocation
+tracker.recordPendingAllocation(pipeline, container1);
+
+// All 3 DNs should have the container pending
+assertEquals(1, tracker.getPendingContainers(dn1).size());
+assertEquals(1, tracker.getPendingContainers(dn2).size());
+assertEquals(1, tracker.getPendingContainers(dn3).size());
+
+// Size should be MAX_CONTAINER_SIZE for each DN
+assertEquals(MAX_CONTAINER_SIZE, tracker.getPendingAllocationSize(dn1));
+assertEquals(MAX_CONTAINER_SIZE, tracker.getPendingAllocationSize(dn2));
+assertEquals(MAX_CONTAINER_SIZE, tracker.getPendingAllocationSize(dn3));
+ }
+
+ @Test
+ public void testRecordMultiplePendingAllocations() {
+tracker.recordPendingAllocation(pipeline, container1);
+tracker.recordPendingAllocation(pipeline, container2);
+tracker.recordPendingAllocation(pipeline, container3);
+
+// Each DN should have 3 pending containers
+assertEquals(3, tracker.getPendingContainers(dn1).size());
+assertEquals(3, tracker.getPendingContainers(dn2).size());
+assertEquals(3, tracker.getPendingContainers(dn3).size());
+
+// Size should be 3 × MAX_CONTAINER_SIZE
+assertEquals(3 * MAX_CONTAINER_SIZE,
tracker.getPendingAllocationSize(dn1));
+ }
+
+ @Test
+ public void testIdempotentRecording() {
+tracker.recordPendingAllocation(pipeline, container1);
+tracker.recordPendingAllocation(pipeline, container1); // Duplicate
+
+// Should still be 1 container (Set deduplication)
+assertEquals(1, tracker.getPendingContainers(dn1).size());
+assertEquals(MAX_CONTAINER_SIZE, tracker.getPendingAllocationSize(dn1));
+ }
+
+ @Test
+ public void testRemovePendingAllocation() {
+tracker.recordPendingAllocation(pipeline, container1);
+tracker.recordPendingAllocation(pipeline, container2);
+
+assertEquals(2, tracker.getPendingContainers(dn1).size());
+
+// Remove one container from DN1
+tracker.removePendingAllocation(dn1, container1);
+
+assertEquals(1, tracker.getPendingContainers(dn1).size());
+assertEquals(MAX_CONTAINER_SIZE, tracker.getPending
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
ashishkumar50 commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3036008474
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -242,12 +262,75 @@ private ContainerInfo createContainer(Pipeline pipeline,
String owner)
return containerInfo;
}
+ /**
+ * Check if a pipeline has sufficient space after considering pending
allocations.
+ * Tracks containers scheduled but not yet written to DataNodes, preventing
over-allocation.
+ *
+ * @param pipeline The pipeline to check
+ * @return true if sufficient space is available, false otherwise
+ */
+ private boolean hasSpaceAfterPendingAllocations(Pipeline pipeline) {
+try {
+ for (DatanodeDetails node : pipeline.getNodes()) {
+// Get DN's storage statistics
+DatanodeInfo datanodeInfo = pipelineManager.getDatanodeInfo(node);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", node.getUuidString());
+ return false;
+}
+
+List storageReports =
datanodeInfo.getStorageReports();
+if (storageReports == null || storageReports.isEmpty()) {
+ LOG.warn("No storage reports for node {}", node.getUuidString());
+ return false;
+}
+
+// Calculate total capacity and effective allocatable space
+// For each disk, calculate how many containers can actually fit,
+// since containers are written to individual disks, not spread across
them.
+// Example: disk1=9GB, disk2=14GB with 5GB containers
+// (1*5GB) + (2*5GB) = 15GB → actually 3 containers
+long totalCapacity = 0L;
+long effectiveAllocatableSpace = 0L;
+for (StorageReportProto report : storageReports) {
Review Comment:
Updated the logic to break when enough space is available on any volume
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
aswinshakil commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3030706480
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##
@@ -89,6 +89,15 @@ protected void
processICR(IncrementalContainerReportFromDatanode report,
ContainerID id = ContainerID.valueOf(replicaProto.getContainerID());
final ContainerInfo container;
try {
+ // Check if container is already known to this DN before adding
+ boolean alreadyOnDn = false;
Review Comment:
Just checked, This inherently does a copy of the containers with a
TreeSet`getExisting(id).copyContainers();`, We should avoid this at all cost.
--
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]
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
aswinshakil commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3030331856
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java:
##
@@ -242,12 +262,75 @@ private ContainerInfo createContainer(Pipeline pipeline,
String owner)
return containerInfo;
}
+ /**
+ * Check if a pipeline has sufficient space after considering pending
allocations.
+ * Tracks containers scheduled but not yet written to DataNodes, preventing
over-allocation.
+ *
+ * @param pipeline The pipeline to check
+ * @return true if sufficient space is available, false otherwise
+ */
+ private boolean hasSpaceAfterPendingAllocations(Pipeline pipeline) {
+try {
+ for (DatanodeDetails node : pipeline.getNodes()) {
+// Get DN's storage statistics
+DatanodeInfo datanodeInfo = pipelineManager.getDatanodeInfo(node);
+if (datanodeInfo == null) {
+ LOG.warn("DatanodeInfo not found for node {}", node.getUuidString());
+ return false;
+}
+
+List storageReports =
datanodeInfo.getStorageReports();
+if (storageReports == null || storageReports.isEmpty()) {
+ LOG.warn("No storage reports for node {}", node.getUuidString());
+ return false;
+}
+
+// Calculate total capacity and effective allocatable space
+// For each disk, calculate how many containers can actually fit,
+// since containers are written to individual disks, not spread across
them.
+// Example: disk1=9GB, disk2=14GB with 5GB containers
+// (1*5GB) + (2*5GB) = 15GB → actually 3 containers
+long totalCapacity = 0L;
+long effectiveAllocatableSpace = 0L;
+for (StorageReportProto report : storageReports) {
+ totalCapacity += report.getCapacity();
+ long usableSpace = VolumeUsage.getUsableSpace(report);
+ // Calculate how many containers can fit on this disk
+ long containersOnThisDisk = usableSpace / maxContainerSize;
+ // Add effective space (containers that fit * container size)
+ effectiveAllocatableSpace += containersOnThisDisk * maxContainerSize;
+}
+
+// Get pending allocations from tracker
+long pendingAllocations =
pendingContainerTracker.getPendingAllocationSize(node);
+
+// Calculate effective remaining space after pending allocations
+long effectiveRemaining = effectiveAllocatableSpace -
pendingAllocations;
+
+// Check if there's enough space for a new container
+if (effectiveRemaining < maxContainerSize) {
+ LOG.info("Node {} insufficient space: capacity={}, effective
allocatable={}, " +
+ "pending allocations={}, effective remaining={},
required={}",
+ node.getUuidString(), totalCapacity, effectiveAllocatableSpace,
+ pendingAllocations, effectiveRemaining, maxContainerSize);
+ return false;
+}
+ }
+
+ return true;
+} catch (Exception e) {
+ LOG.warn("Error checking space for pipeline {}", pipeline.getId(), e);
+ return true;
Review Comment:
If we are not sure if we can create container here, Should we still choose
this pipeline? Instead of making it generic, we can specify what to do for each
exception we might see.
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a T
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
sumitagrawl commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3021666985
##
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##
@@ -138,10 +138,31 @@ public class ScmConfig extends ReconfigurableConfig {
)
private int transactionToDNsCommitMapLimit = 500;
+ @Config(key = "hdds.scm.container.pending-allocation.roll-interval",
Review Comment:
we can have config as hdds.scm.container.pending.allocation.roll.interval
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##
@@ -89,6 +89,15 @@ protected void
processICR(IncrementalContainerReportFromDatanode report,
ContainerID id = ContainerID.valueOf(replicaProto.getContainerID());
final ContainerInfo container;
try {
+ // Check if container is already known to this DN before adding
+ boolean alreadyOnDn = false;
Review Comment:
Do we really need check if container exist? or just remove if exist as
single call ?
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+priva
Re: [PR] HDDS-14921. Improve space accounting in SCM with In-Flight container allocation tracking. [ozone]
rakeshadr commented on code in PR #1:
URL: https://github.com/apache/ozone/pull/1#discussion_r3021573986
##
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/PendingContainerTracker.java:
##
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.container.metrics.SCMContainerManagerMetrics;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tracks pending container allocations using a Two Window Tumbling Bucket
pattern.
+ * Similar like HDFS HADOOP-3707.
+ *
+ * Two Window Tumbling Bucket for automatic aging and cleanup.
+ *
+ * How It Works:
+ * Each DataNode has two sets: currentWindow and
previousWindow
+ * New allocations go into currentWindow
+ * Every ROLL_INTERVAL (default 10 minutes):
+ *
+ * previousWindow = currentWindow (shift)
+ * currentWindow = new empty set (reset)
+ * Old previousWindow is discarded (automatic aging)
+ *
+ *
+ * When checking pending: return union of currentWindow +
previousWindow
+ *
+ *
+ * Example Timeline:
+ *
+ * Time | Action| CurrentWindow | PreviousWindow | Total
Pending
+ *
--+---+---++--
+ * 00:00 | Allocate Container-1 | {C1} | {} | {C1}
+ * 00:05 | Allocate Container-2 | {C1, C2} | {} | {C1,
C2}
+ * 00:10 | [ROLL] Window tumbles | {}| {C1, C2} | {C1,
C2}
+ * 00:12 | Allocate Container-3 | {C3} | {C1, C2} | {C1,
C2, C3}
+ * 00:15 | Report confirms C1| {C3} | {C2} | {C2,
C3}
+ * 00:20 | [ROLL] Window tumbles | {}| {C3} | {C3}
+ * | (C2 aged out if not reported)
+ *
+ *
+ */
+public class PendingContainerTracker {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(PendingContainerTracker.class);
+
+ /**
+ * Roll interval in milliseconds.
+ * Configurable via hdds.scm.container.pending-allocation.roll-interval.
+ * Default: 10 minutes.
+ * Containers automatically age out after 2 × rollIntervalMs.
+ */
+ private final long rollIntervalMs;
+
+ /**
+ * Map of DataNode UUID to TwoWindowBucket.
+ */
+ private final ConcurrentHashMap datanodeBuckets;
+
+ /**
+ * Maximum container size in bytes.
+ */
+ private final long maxContainerSize;
+
+ /**
+ * Metrics for tracking pending containers.
+ */
+ private final SCMContainerManagerMetrics metrics;
+
+ /**
+ * Two-window bucket for a single DataNode.
+ * Contains current and previous window sets, plus last roll timestamp.
+ */
+ private static class TwoWindowBucket {
+private Set currentWindow = ConcurrentHashMap.newKeySet();
+private Set previousWindow = ConcurrentHashMap.newKeySet();
+private long lastRollTime = Time.monotonicNow();
+private final long rollIntervalMs;
+
+TwoWindowBucket(long rollIntervalMs) {
+ this.rollIntervalMs = rollIntervalMs;
+}
+
+/**
+ * Roll the windows: previous = current, current = empty.
+ * Called when current time exceeds lastRollTime + rollIntervalMs.
+ */
+synchronized void rollIfNeeded() {
+ long now = Time.monotonicNow();
+ if (now - lastRollTime >= rollIntervalMs) {
+// Shift: current becomes previous
+previousWindow = currentWindow;
+// Reset: new empty current window
+currentWindow = ConcurrentHashMap.newKeySet();
+lastRollTime = now;
+LOG.debug("Rolled window. Previous window size: {}, Current window
reset to empty", previousWindow.size());
+ }
+}
+
+/**
+ * Get union of both windows (all pending containers).
+ */
+synchronized Set g
