Skip to content

HDDS-7625. Do not compress OM/SCM checkpoints #4130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jojochuang merged 4 commits into from
Jan 22, 2023
Merged

HDDS-7625. Do not compress OM/SCM checkpoints #4130

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c04ebd
HDDS-7625. Do not compress OM checkpoint
chungen0126 Dec 27, 2022
59c8e92
Merge branch 'master' into HDDS-7625
chungen0126 Jan 18, 2023
fa89868
change .tar.gz & .tgz to .tar
chungen0126 Jan 18, 2023
f8ba421
remove DBCheckPointServlet.writeDBCheckpointToStream()
chungen0126 Jan 21, 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
2 changes: 1 addition & 1 deletion ...ce/src/main/java/org/apache/hadoop/ozone/container/replication/GrpcReplicationClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public CompletableFuture<Path> download(long containerId) {
CompletableFuture<Path> response = new CompletableFuture<>();

Path destinationPath =
getWorkingDirectory().resolve("container-" + containerId + ".tar.gz");
getWorkingDirectory().resolve("container-" + containerId + ".tar");

client.download(request,
new StreamDownloader(containerId, response, destinationPath));
Expand Down
6 changes: 3 additions & 3 deletions ...rvice/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public void testEmptyContainerImportExport() throws Exception {
checkContainerFilesPresent(data, 0);

//destination path
File exportTar = folder.newFile("exported.tar.gz");
File exportTar = folder.newFile("exported.tar");
TarContainerPacker packer = new TarContainerPacker();
//export the container
try (FileOutputStream fos = new FileOutputStream(exportTar)) {
Expand All @@ -220,7 +220,7 @@ public void testContainerImportExport() throws Exception {
populate(numberOfKeysToWrite);

//destination path
File folderToExport = folder.newFile("exported.tar.gz");
File folderToExport = folder.newFile("exported.tar");
for (Map.Entry<CopyContainerCompression, String> entry :
CopyContainerCompression.getCompressionMapping().entrySet()) {
TarContainerPacker packer = new TarContainerPacker(entry.getValue());
Expand Down Expand Up @@ -368,7 +368,7 @@ public void concurrentExport() throws Exception {
List<Thread> threads = IntStream.range(0, 20)
.mapToObj(i -> new Thread(() -> {
try {
File file = folder.newFile("concurrent" + i + ".tar.gz");
File file = folder.newFile("concurrent" + i + ".tar");
try (OutputStream out = new FileOutputStream(file)) {
keyValueContainer.exportContainerData(out, packer);
}
Expand Down
4 changes: 2 additions & 2 deletions ...vice/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public void pack() throws IOException, CompressorException {
//sample container descriptor file
writeDescriptor(sourceContainer);

Path targetFile = TEMP_DIR.resolve("container.tar.gz");
Path targetFile = TEMP_DIR.resolve("container.tar");

//WHEN: pack it
try (FileOutputStream output = new FileOutputStream(targetFile.toFile())) {
Expand Down Expand Up @@ -361,7 +361,7 @@ private File writeSingleFile(Path parentPath, String fileName,

private File packContainerWithSingleFile(File file, String entryName)
throws Exception {
File targetFile = TEMP_DIR.resolve("container.tar.gz").toFile();
File targetFile = TEMP_DIR.resolve("container.tar").toFile();
try (FileOutputStream output = new FileOutputStream(targetFile);
OutputStream compressed = packer.compress(output);
ArchiveOutputStream archive = new TarArchiveOutputStream(compressed)) {
Expand Down
36 changes: 12 additions & 24 deletions hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@
import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.ArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.compressors.CompressorException;
import org.apache.commons.compress.compressors.CompressorOutputStream;
import org.apache.commons.compress.compressors.CompressorStreamFactory;
import org.apache.commons.compress.utils.IOUtils;
import org.apache.commons.lang3.StringUtils;

Expand Down Expand Up @@ -171,7 +168,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) {
response.setContentType("application/x-tgz");

@adoroszlai adoroszlai Jan 3, 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.

Suggested change
response.setContentType("application/x-tgz");
response.setContentType("application/x-tar");

Also update related tests:

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServlet.java
182:    doNothing().when(responseMock).setContentType("application/x-tgz");

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSCMDbCheckpointServlet.java
129:      doNothing().when(responseMock).setContentType("application/x-tgz");

response.setHeader("Content-Disposition",
"attachment; filename=\"" +
file.toString() + ".tgz\"");
file + ".tar\"");

Instant start = Instant.now();
writeDBCheckpointToStream(checkpoint,
Expand Down Expand Up @@ -211,30 +208,21 @@ public static void writeDBCheckpointToStream(DBCheckpoint checkpoint,
OutputStream destination)

@jojochuang jojochuang Jan 20, 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.

Let's remove this method, and replace the usage of DBCheckpointServlet.writeDBCheckpointToStream() with HddsServerUtil.writeDBCheckpointToStream(). (There are three usages)

throws IOException {

try (CompressorOutputStream gzippedOut = new CompressorStreamFactory()
.createCompressorOutputStream(CompressorStreamFactory.GZIP,
destination)) {

try (ArchiveOutputStream archiveOutputStream =
new TarArchiveOutputStream(gzippedOut)) {

Path checkpointPath = checkpoint.getCheckpointLocation();
try (Stream<Path> files = Files.list(checkpointPath)) {
for (Path path : files.collect(Collectors.toList())) {
if (path != null) {
Path fileName = path.getFileName();
if (fileName != null) {
includeFile(path.toFile(), fileName.toString(),
archiveOutputStream);
}
try (ArchiveOutputStream archiveOutputStream =
new TarArchiveOutputStream(destination)) {

Path checkpointPath = checkpoint.getCheckpointLocation();
try (Stream<Path> files = Files.list(checkpointPath)) {
for (Path path : files.collect(Collectors.toList())) {
if (path != null) {
Path fileName = path.getFileName();
if (fileName != null) {
includeFile(path.toFile(), fileName.toString(),
archiveOutputStream);

@adoroszlai adoroszlai Jan 3, 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.

This method and its helper includeFile are duplicated between HddsServerUtil and DBCheckpointServlet. Can you please remove one of them?

Also, please update the method comment (as a compressed file (tgz)) and class comment ((tar.gz)).

}
}
}
}
} catch (CompressorException e) {
throw new IOException(
"Can't compress the checkpoint: " +
checkpoint.getCheckpointLocation(), e);
}
}

Expand Down
14 changes: 2 additions & 12 deletions hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@
import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.ArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.compressors.CompressorException;
import org.apache.commons.compress.compressors.CompressorOutputStream;
import org.apache.commons.compress.compressors.CompressorStreamFactory;
import org.apache.commons.compress.utils.IOUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hdds.DFSConfigKeysLegacy;
Expand Down Expand Up @@ -535,11 +532,8 @@ public static MetricsSystem initializeMetrics(
public static void writeDBCheckpointToStream(DBCheckpoint checkpoint,
OutputStream destination)
throws IOException {
try (CompressorOutputStream gzippedOut = new CompressorStreamFactory()
.createCompressorOutputStream(CompressorStreamFactory.GZIP,
destination);
ArchiveOutputStream archiveOutputStream =
new TarArchiveOutputStream(gzippedOut);
try (ArchiveOutputStream archiveOutputStream =
new TarArchiveOutputStream(destination);
Stream<Path> files =
Files.list(checkpoint.getCheckpointLocation())) {
for (Path path : files.collect(Collectors.toList())) {
Expand All @@ -551,10 +545,6 @@ public static void writeDBCheckpointToStream(DBCheckpoint checkpoint,
}
}
}
} catch (CompressorException e) {
throw new IOException(
"Can't compress the checkpoint: " +
checkpoint.getCheckpointLocation(), e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMSnapshotProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public DBCheckpoint getSCMDBSnapshot(String leaderSCMNodeID)
String snapshotFilePath =
Paths.get(scmSnapshotDir.getAbsolutePath(), snapshotFileName).toFile()
.getAbsolutePath();
File targetFile = new File(snapshotFilePath + ".tar.gz");
File targetFile = new File(snapshotFilePath + ".tar");


// the downloadClient instance will be created as and when install snapshot
Expand Down
2 changes: 1 addition & 1 deletion ...integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSCMDbCheckpointServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void testDoGet() throws ServletException, IOException {
Matchers.anyString());

tempFile = File.createTempFile("testDoGet_" + System
.currentTimeMillis(), ".tar.gz");
.currentTimeMillis(), ".tar");

FileOutputStream fileOutputStream = new FileOutputStream(tempFile);
when(responseMock.getOutputStream()).thenReturn(
Expand Down
54 changes: 23 additions & 31 deletions .../integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
Expand All @@ -38,7 +37,6 @@

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.utils.db.DBCheckpoint;
import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.ozone.MiniOzoneCluster;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.security.UserGroupInformation;
Expand Down Expand Up @@ -101,7 +99,7 @@ public void init() throws Exception {
conf = new OzoneConfiguration();

tempFile = File.createTempFile("testDoGet_" + System
.currentTimeMillis(), ".tar.gz");
.currentTimeMillis(), ".tar");

FileOutputStream fileOutputStream = new FileOutputStream(tempFile);

Expand Down Expand Up @@ -261,34 +259,28 @@ public void testSpnegoEnabled() throws Exception {
@Test
public void testWriteCheckpointToOutputStream() throws Exception {

FileInputStream fis = null;
FileOutputStream fos = null;

try {
String testDirName = folder.newFolder().getAbsolutePath();
File file = new File(testDirName + "/temp1.txt");
OutputStreamWriter writer = new OutputStreamWriter(
new FileOutputStream(file), StandardCharsets.UTF_8);
writer.write("Test data 1");
writer.close();

file = new File(testDirName + "/temp2.txt");
writer = new OutputStreamWriter(
new FileOutputStream(file), StandardCharsets.UTF_8);
writer.write("Test data 2");
writer.close();

File outputFile =
new File(Paths.get(testDirName, "output_file.tgz").toString());
TestDBCheckpoint dbCheckpoint = new TestDBCheckpoint(
Paths.get(testDirName));
writeDBCheckpointToStream(dbCheckpoint,
new FileOutputStream(outputFile));
assertNotNull(outputFile);
} finally {
IOUtils.closeStream(fis);
IOUtils.closeStream(fos);
}
String testDirName = folder.newFolder().getAbsolutePath();
File checkpoint = new File(testDirName, "checkpoint");
checkpoint.mkdir();
File file = new File(checkpoint, "temp1.txt");
OutputStreamWriter writer = new OutputStreamWriter(
new FileOutputStream(file), StandardCharsets.UTF_8);
writer.write("Test data 1");
writer.close();

file = new File(checkpoint, "/temp2.txt");
writer = new OutputStreamWriter(
new FileOutputStream(file), StandardCharsets.UTF_8);
writer.write("Test data 2");
writer.close();

File outputFile =
new File(Paths.get(testDirName, "output_file.tar").toString());
TestDBCheckpoint dbCheckpoint = new TestDBCheckpoint(
checkpoint.toPath());
writeDBCheckpointToStream(dbCheckpoint,
new FileOutputStream(outputFile));
assertNotNull(outputFile);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...nager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OzoneManagerSnapshotProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public DBCheckpoint getOzoneManagerDBSnapshot(String leaderOMNodeID)
+ "-" + snapshotTime;
String snapshotFilePath = Paths.get(omSnapshotDir.getAbsolutePath(),
snapshotFileName).toFile().getAbsolutePath();
File targetFile = new File(snapshotFilePath + ".tar.gz");
File targetFile = new File(snapshotFilePath + ".tar");

String omCheckpointUrl = peerNodesMap.get(leaderOMNodeID)
.getOMDBCheckpointEnpointUrl(httpPolicy.isHttpEnabled());
Expand Down
20 changes: 4 additions & 16 deletions hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.nio.file.Paths;
import java.security.KeyPair;
import java.sql.Timestamp;
import java.util.zip.GZIPOutputStream;

import com.google.inject.Singleton;
import org.apache.hadoop.hdds.HddsConfigKeys;
Expand All @@ -46,7 +45,6 @@
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
import static org.apache.hadoop.hdds.server.ServerUtils.getDirectoryFromConfig;
import static org.apache.hadoop.hdds.server.ServerUtils.getOzoneMetaDirPath;
import static org.apache.hadoop.ozone.recon.ReconServerConfigKeys.OZONE_RECON_SCM_DB_DIR;
Expand Down Expand Up @@ -107,20 +105,17 @@ public File getReconDbDir(ConfigurationSource conf, String dirConfigKey) {
* Given a source directory, create a tar.gz file from it.

@adoroszlai adoroszlai Jan 3, 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.

Suggested change
* Given a source directory, create a tar.gz file from it.
* Given a source directory, create a tar file from it.

*
* @param sourcePath the path to the directory to be archived.
* @return tar.gz file
* @return tar file
* @throws IOException
*/
public static File createTarFile(Path sourcePath) throws IOException {
TarArchiveOutputStream tarOs = null;
FileOutputStream fileOutputStream = null;
GZIPOutputStream gzipOutputStream = null;
try {
String sourceDir = sourcePath.toString();
String fileName = sourceDir.concat(".tar.gz");
String fileName = sourceDir.concat(".tar");
fileOutputStream = new FileOutputStream(fileName);
gzipOutputStream =
new GZIPOutputStream(new BufferedOutputStream(fileOutputStream));
tarOs = new TarArchiveOutputStream(gzipOutputStream);
tarOs = new TarArchiveOutputStream(fileOutputStream);
File folder = new File(sourceDir);
File[] filesInDir = folder.listFiles();
if (filesInDir != null) {
Expand All @@ -133,7 +128,6 @@ public static File createTarFile(Path sourcePath) throws IOException {
try {
org.apache.hadoop.io.IOUtils.closeStream(tarOs);
org.apache.hadoop.io.IOUtils.closeStream(fileOutputStream);
org.apache.hadoop.io.IOUtils.closeStream(gzipOutputStream);
} catch (Exception e) {
LOG.error("Exception encountered when closing " +
"TAR file output stream: " + e);
Expand Down Expand Up @@ -177,12 +171,8 @@ public void untarCheckpointFile(File tarFile, Path destPath)
throws IOException {

FileInputStream fileInputStream = null;
BufferedInputStream buffIn = null;
GzipCompressorInputStream gzIn = null;
try {
fileInputStream = new FileInputStream(tarFile);
buffIn = new BufferedInputStream(fileInputStream);
gzIn = new GzipCompressorInputStream(buffIn);

//Create Destination directory if it does not exist.
if (!destPath.toFile().exists()) {
Expand All @@ -193,7 +183,7 @@ public void untarCheckpointFile(File tarFile, Path destPath)
}

try (TarArchiveInputStream tarInStream =
new TarArchiveInputStream(gzIn)) {
new TarArchiveInputStream(fileInputStream)) {
TarArchiveEntry entry;

while ((entry = (TarArchiveEntry) tarInStream.getNextEntry()) != null) {
Expand Down Expand Up @@ -223,8 +213,6 @@ public void untarCheckpointFile(File tarFile, Path destPath)
}
}
} finally {
IOUtils.closeStream(gzIn);
IOUtils.closeStream(buffIn);
IOUtils.closeStream(fileInputStream);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ DBCheckpoint getOzoneManagerDBSnapshot() {
String snapshotFileName = RECON_OM_SNAPSHOT_DB + "_" +
System.currentTimeMillis();
File targetFile = new File(omSnapshotDBParentDir, snapshotFileName +
".tar.gz");
".tar");
try {
SecurityUtil.doAsLoginUser(() -> {
try (InputStream inputStream = reconUtils.makeHttpCall(
Expand Down
2 changes: 1 addition & 1 deletion ...main/java/org/apache/hadoop/ozone/recon/spi/impl/StorageContainerServiceProviderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public DBCheckpoint getSCMDBSnapshot() {
String snapshotFileName = RECON_SCM_SNAPSHOT_DB + "_" +
System.currentTimeMillis();
File targetFile = new File(scmSnapshotDBParentDir, snapshotFileName +
".tar.gz");
".tar");

try {
if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
Expand Down