Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
db0cf77
HDDS-7083. Spread container-copy directories
symious Aug 3, 2022
ea07251
HDDS-7083. Add config in ozone-default.xml
symious Aug 3, 2022
e5c0a34
HDDS-7083. Enable spread volume feature by default
symious Aug 9, 2022
75e9f8d
HDDS-7083. Deprecate configuration and use tmp/container-copy directory
symious Aug 16, 2022
1cc345d
HDDS-7083. Fix config test
symious Aug 16, 2022
14d3e00
HDDS-7083. Add deprecated key
symious Aug 16, 2022
615070d
HDDS-7083. Download, untar, move, then load container
symious Aug 25, 2022
417fd4b
HDDS-7083. Unit test to be fixed
symious Aug 26, 2022
1ef25a4
HDDS-7083. parameter not to create dir when initializing
symious Sep 2, 2022
4bfa4df
HDDS-7083. Fix unit test
symious Sep 2, 2022
f590482
trigger new CI check
symious Sep 2, 2022
8cb4030
trigger new CI check
symious Sep 3, 2022
dfd161d
trigger new CI check
symious Sep 3, 2022
e551a67
Merge branch 'master' into HDDS-7083
symious Dec 10, 2022
205d6a3
HDDS-7083. Remove unused import
symious Dec 10, 2022
fc6bee4
Merge branch 'master' into HDDS-7083
symious Dec 14, 2022
cc5dcdf
Merge branch 'master' into HDDS-7083
symious Dec 18, 2022
4ecff14
HDDS-7083. Fix according to comments
symious Dec 18, 2022
2207a50
Merge branch 'master' into HDDS-7083
symious Jan 8, 2023
a5c34fe
HDDS-7083. Remove duplicated import
symious Jan 8, 2023
299d0f6
HDDS-7083. Delete tmp direcotry
symious Jan 9, 2023
c62d37f
HDDS-7083. Delete temporary files
symious Jan 10, 2023
e8f036e
HDDS-7083. Throw proper exception
symious Jan 10, 2023
5806f6c
HDDS-7083. Clean tmp data
symious Jan 11, 2023
0f1780d
HDDS-7083. Remove unused import
symious Jan 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.apache.ratis.server.RaftServerConfigKeys;

import static org.apache.hadoop.hdds.ratis.RatisHelper.HDDS_DATANODE_RATIS_PREFIX_KEY;
import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CONTAINER_COPY_WORKDIR;

/**
* Configuration for ozone.
Expand Down Expand Up @@ -308,7 +309,9 @@ private static void addDeprecatedKeys() {
new DeprecationDelta("dfs.datanode.keytab.file",
DFSConfigKeysLegacy.DFS_DATANODE_KERBEROS_KEYTAB_FILE_KEY),
new DeprecationDelta("ozone.scm.chunk.layout",
ScmConfigKeys.OZONE_SCM_CONTAINER_LAYOUT_KEY)
ScmConfigKeys.OZONE_SCM_CONTAINER_LAYOUT_KEY),
new DeprecationDelta("hdds.datanode.replication.work.dir",
OZONE_CONTAINER_COPY_WORKDIR)
});
}
}
6 changes: 3 additions & 3 deletions hadoop-hdds/common/src/main/resources/ozone-default.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1554,9 +1554,9 @@
<property>
<name>hdds.datanode.replication.work.dir</name>
<tag>DATANODE</tag>
<description>Temporary which is used during the container replication
betweeen datanodes. Should have enough space to store multiple container
(in compressed format), but doesn't require fast io access such as SSD.
<description>This configuration is deprecated. Default directory which is used

@ChenSammi ChenSammi Aug 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This configuration is deprecated. Temporary sub directory under each hdds.datanode.dir will be used during the container replication between datanodes to save the downloaded container(in compressed format).

during the container replication betweeen datanodes. Replication work will
be spread to different volumes.
</description>
</property>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails,
ContainerReplicator replicator =
new DownloadAndImportReplicator(container.getContainerSet(),
container.getController(),
new SimpleContainerDownloader(conf, dnCertClient),
new SimpleContainerDownloader(conf, dnCertClient,
container.getVolumeSet()),
new TarContainerPacker());

replicatorMetrics = new MeasuredReplicator(replicator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,24 @@
import java.util.concurrent.ExecutionException;

import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.StorageUnit;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.security.x509.SecurityConfig;
import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
import org.apache.hadoop.ozone.OzoneConfigKeys;

import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.ozone.container.common.interfaces.VolumeChoosingPolicy;
import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil;
import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
import org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingPolicy;
import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_CHOOSING_POLICY;

/**
* Simple ContainerDownloaderImplementation to download the missing container
* from the first available datanode.
Expand All @@ -50,24 +58,33 @@ public class SimpleContainerDownloader implements ContainerDownloader {
private static final Logger LOG =
LoggerFactory.getLogger(SimpleContainerDownloader.class);

private final Path workingDirectory;
public static final String CONTAINER_COPY_DIR = "container-copy";

@ChenSammi ChenSammi Dec 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two fields are not used anymore.


private ConfigurationSource conf;

@ChenSammi ChenSammi Jan 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this conf is not used anymore after initialized.

private final SecurityConfig securityConfig;
private final CertificateClient certClient;
private final VolumeSet volumeSet;
private VolumeChoosingPolicy volumeChoosingPolicy = null;
private long containerSize;

public SimpleContainerDownloader(
ConfigurationSource conf, CertificateClient certClient) {
ConfigurationSource conf, CertificateClient certClient,
VolumeSet volumeSet) {

String workDirString =
conf.get(OzoneConfigKeys.OZONE_CONTAINER_COPY_WORKDIR);

if (workDirString == null) {
workingDirectory = Paths.get(System.getProperty("java.io.tmpdir"))
.resolve("container-copy");
} else {
workingDirectory = Paths.get(workDirString);
}
this.conf = conf;
securityConfig = new SecurityConfig(conf);
this.certClient = certClient;
this.volumeSet = volumeSet;

@ChenSammi ChenSammi Aug 16, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@symious , the new proposed flow should be,

  1. choose a volume and download the container tar into the temp directory.
  2. untar the container into temp directory.
  3. move the container directory to destination directory and finish the container import so that if import failed, there will be no container residual in the data volume.

So not only SimpleContainerDownloader, but also the container import flow should be updated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, should we apply the new proposal in a new ticket or the current one?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can implement the proposal in more than one tickets. If so, please change the JIRA to a feature JIRA and create sub JIRAs for different tasks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted with thanks.

try {
this.volumeChoosingPolicy = conf.getClass(
HDDS_DATANODE_VOLUME_CHOOSING_POLICY, RoundRobinVolumeChoosingPolicy
.class, VolumeChoosingPolicy.class).newInstance();
} catch (Exception e) {
throw new RuntimeException(e);
}
this.containerSize = (long) conf.getStorageSize(
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES);
}

@Override
Expand Down Expand Up @@ -118,7 +135,7 @@ protected CompletableFuture<Path> downloadContainer(
GrpcReplicationClient grpcReplicationClient =
new GrpcReplicationClient(datanode.getIpAddress(),
datanode.getPort(Name.REPLICATION).getValue(),
workingDirectory, securityConfig, certClient);
getWorkingDirectory(), securityConfig, certClient);
result = grpcReplicationClient.download(containerId)
.whenComplete((r, ex) -> {
try {
Expand All @@ -135,4 +152,21 @@ protected CompletableFuture<Path> downloadContainer(
public void close() {
// noop
}

public Path getWorkingDirectory() {
Path defaultWorkingDirectory =
Paths.get(System.getProperty("java.io.tmpdir"));

@ChenSammi ChenSammi Aug 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Since data volume must be configured, then we don't fallback to this "java.io.tmpdir".
  2. move this volume choose logic to outside DownloadAndImportReplicator, so we can use this choose volume later the container import process.
  3. define a constant field for "tmp" directory.

try {
// Use containerSize * 2 to store source and dest file
HddsVolume volume = volumeChoosingPolicy.chooseVolume(
StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()),
containerSize * 2);
return Paths.get(volume.getStorageDir().getParent()).resolve("tmp")
.resolve(CONTAINER_COPY_DIR);
} catch (IOException e) {
LOG.error("Exception when spreading copy directory, using default " +
"working directory {}", defaultWorkingDirectory.toAbsolutePath(), e);
}
return defaultWorkingDirectory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@

package org.apache.hadoop.ozone.container.replication;

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;

Expand All @@ -31,14 +34,23 @@
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;

import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet;
import org.apache.hadoop.ozone.container.common.volume.StorageVolume;
import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

/**
* Test SimpleContainerDownloader.
*/
public class TestSimpleContainerDownloader {

@Rule
public final TemporaryFolder tempDir = new TemporaryFolder();

private static final String SUCCESS_PATH = "downloaded";

@Test
Expand Down Expand Up @@ -106,7 +118,7 @@ public void testRandomSelection()
final List<DatanodeDetails> datanodes = createDatanodes();

SimpleContainerDownloader downloader =
new SimpleContainerDownloader(new OzoneConfiguration(), null) {
new SimpleContainerDownloader(new OzoneConfiguration(), null, null) {

@Override
protected CompletableFuture<Path> downloadContainer(
Expand All @@ -133,6 +145,29 @@ protected CompletableFuture<Path> downloadContainer(
+ "used.");
}

@Test
public void testGetWorkingDirectory()
throws Exception {

//GIVEN
List<DatanodeDetails> datanodes = createDatanodes();

// Spread container copy directory
OzoneConfiguration conf = new OzoneConfiguration();
VolumeSet volumeSet = getVolumeSet(datanodes.get(0), conf);

SimpleContainerDownloader downloader =
new SimpleContainerDownloader(conf, null, volumeSet);

Path firstDi = downloader.getWorkingDirectory();
Path secondDi = downloader.getWorkingDirectory();

Assert.assertNotEquals(firstDi,
Paths.get(System.getProperty("java.io.tmpdir"))
.resolve(SimpleContainerDownloader.CONTAINER_COPY_DIR));
Assert.assertNotEquals(firstDi, secondDi);
}

/**
* Creates downloader which fails with datanodes in the arguments.
*
Expand All @@ -149,7 +184,7 @@ private SimpleContainerDownloader createDownloaderWithPredefinedFailures(
final List<DatanodeDetails> datanodes =
Arrays.asList(failedDatanodes);

return new SimpleContainerDownloader(conf, null) {
return new SimpleContainerDownloader(conf, null, null) {

//for retry testing we use predictable list of datanodes.
@Override
Expand Down Expand Up @@ -192,4 +227,21 @@ private List<DatanodeDetails> createDatanodes() {
datanodes.add(MockDatanodeDetails.randomDatanodeDetails());
return datanodes;
}

private VolumeSet getVolumeSet(DatanodeDetails datanodeDetails,
OzoneConfiguration conf) throws IOException {
String clusterId = UUID.randomUUID().toString();
int volumeNum = 3;
File[] hddsVolumeDirs = new File[volumeNum];
StringBuilder hddsDirs = new StringBuilder();
for (int i = 0; i < volumeNum; i++) {
hddsVolumeDirs[i] = tempDir.newFolder();
hddsDirs.append(hddsVolumeDirs[i]).append(",");
}
conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_KEY, hddsDirs.toString());
VolumeSet hddsVolumeSet = new MutableVolumeSet(
datanodeDetails.getUuidString(), clusterId, conf, null,
StorageVolume.VolumeType.DATA_VOLUME, null);
return hddsVolumeSet;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ public void importContainer(long containerID, File source) throws Exception {
DownloadAndImportReplicator replicator =
new DownloadAndImportReplicator(dsm.getContainer().getContainerSet(),
dsm.getContainer().getController(),
new SimpleContainerDownloader(conf, null),
new SimpleContainerDownloader(conf, null, null),
new TarContainerPacker());

File tempFile = tempFolder.newFile();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,6 @@ protected List<HddsDatanodeService> createHddsDatanodes(
reservedSpaceString);
dnConf.set(OzoneConfigKeys.DFS_CONTAINER_RATIS_DATANODE_STORAGE_DIR,
ratisDir.toString());
dnConf.set(OzoneConfigKeys.OZONE_CONTAINER_COPY_WORKDIR,
workDir.toString());
if (reconServer != null) {
OzoneStorageContainerManager reconScm =
reconServer.getReconStorageContainerManager();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private void initializeReplicationSupervisor(ConfigurationSource conf)
ContainerReplicator replicator =
new DownloadAndImportReplicator(containerSet,
controller,
new SimpleContainerDownloader(conf, null),
new SimpleContainerDownloader(conf, null, null),
new TarContainerPacker());

supervisor = new ReplicationSupervisor(containerSet, replicator, 10);
Expand Down