peterxcli commented on code in PR #8915:
URL: https://github.com/apache/ozone/pull/8915#discussion_r2282702023
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -541,6 +541,7 @@ message CopyContainerResponseProto {
required bool eof = 4;
required bytes data = 5;
optional int64 checksum = 6;
+ optional uint64 actualContainerSize = 7;
Review Comment:
and I'm thinking if using `len(data)` is enough to get the actual container
byte size.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/DownloadAndImportReplicator.java:
##########
@@ -71,18 +72,35 @@ public void replicate(ReplicationTask task) {
LOG.info("Starting replication of container {} from {} using {}",
containerID, sourceDatanodes, compression);
HddsVolume targetVolume = null;
+ long spaceReserved = containerImporter.getDefaultReplicationSpace();
try {
targetVolume = containerImporter.chooseNextVolume();
// Wait for the download. This thread pool is limiting the parallel
// downloads, so it's ok to block here and wait for the full download.
- Path tarFilePath =
+ Pair<Path, Long> downloadResult =
downloader.getContainerDataFromReplicas(containerID, sourceDatanodes,
ContainerImporter.getUntarDirectory(targetVolume), compression);
- if (tarFilePath == null) {
+ if (downloadResult == null || downloadResult.getLeft() == null) {
task.setStatus(Status.FAILED);
return;
}
+
+ Path tarFilePath = downloadResult.getLeft();
+ Long actualContainerSize = downloadResult.getRight();
+
+ if (actualContainerSize != null) {
+ long actualSpaceNeeded =
containerImporter.getRequiredReplicationSpace(actualContainerSize);
+ long spaceAdjustment = actualSpaceNeeded - spaceReserved;
+
+ if (spaceAdjustment > 0) {
Review Comment:
Can we release the committed space if the spaceAdjustment < 0?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -76,7 +76,7 @@ public ContainerImporter(@Nonnull ConfigurationSource conf,
this.controller = controller;
this.volumeSet = volumeSet;
this.volumeChoosingPolicy = volumeChoosingPolicy;
- containerSize = (long) conf.getStorageSize(
+ defaultContainerSize = (long) conf.getStorageSize(
Review Comment:
I can't comment at line 145, so i just comment here.
`chooseNextVolume` still use the default container size. It call
`VolumeChoosingPolicy#chooseVolume`, which will call
volumeData.incrCommit(space)`, so then you have to adjust the container size in
both container push/pull handler.
Let's make `chooseNextVolume` to accept the container size, so we can remove
the commit space adjustment 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]
