swamirishi commented on code in PR #8477:
URL: https://github.com/apache/ozone/pull/8477#discussion_r2140175700
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/Archiver.java:
##########
@@ -115,6 +119,33 @@ public static long includeFile(File file, String entryName,
return bytes;
}
+ /**
+ * Creates a hardlink for the given file in a temporary directory, adds it
+ * as an entry in the archive, and includes its contents in the archive
output.
+ * The temporary hardlink is deleted after processing.
+ */
+ public static void linkAndIncludeFile(File file, String entryName,
+ ArchiveOutputStream<TarArchiveEntry> archiveOutput, Path tmpDir) throws
IOException {
+ File link = null;
+ try {
+ Files.createLink(tmpDir.resolve(entryName), file.toPath());
+ link = tmpDir.resolve(file.getName()).toFile();
+ TarArchiveEntry entry = archiveOutput.createArchiveEntry(link,
entryName);
+ archiveOutput.putArchiveEntry(entry);
+ try (InputStream input = Files.newInputStream(link.toPath())) {
+ IOUtils.copyLarge(input, archiveOutput);
+ }
+ archiveOutput.closeArchiveEntry();
+ } catch (IOException ioe) {
Review Comment:
In case of an exception at least return a boolean value for the caller to
know that the write has failed.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -196,20 +202,16 @@ private void
generateSnapshotCheckpoint(HttpServletRequest request,
flush = Boolean.parseBoolean(flushParam);
}
- List<String> receivedSstList = new ArrayList<>();
+ processMetadataSnapshotRequest(request, response, isFormData, checkpoint,
flush);
+ }
+
+ protected void processMetadataSnapshotRequest(HttpServletRequest request,
HttpServletResponse response,
+ boolean isFormData, DBCheckpoint checkpoint, boolean flush) {
Review Comment:
If you are planning to refactor some code can you please first raise a
refactoring PR. It is very tough to follow such refactoring.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -196,20 +202,16 @@ private void
generateSnapshotCheckpoint(HttpServletRequest request,
flush = Boolean.parseBoolean(flushParam);
}
- List<String> receivedSstList = new ArrayList<>();
+ processMetadataSnapshotRequest(request, response, isFormData, checkpoint,
flush);
+ }
+
+ protected void processMetadataSnapshotRequest(HttpServletRequest request,
HttpServletResponse response,
+ boolean isFormData, DBCheckpoint checkpoint, boolean flush) {
Review Comment:
Why pass DBCheckpoint it is always null?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -130,12 +136,12 @@ private boolean hasPermission(UserGroupInformation user) {
}
}
- private static void logSstFileList(List<String>sstList, String msg, int
sampleSize) {
+ protected static void logSstFileList(Collection<String> sstList, String msg,
int sampleSize) {
Review Comment:
nit: why protected this could be package private.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -130,12 +136,12 @@ private boolean hasPermission(UserGroupInformation user) {
}
}
- private static void logSstFileList(List<String>sstList, String msg, int
sampleSize) {
+ protected static void logSstFileList(Collection<String> sstList, String msg,
int sampleSize) {
Review Comment:
We should move the implementation to the right package both on om and scm.
Please create a follow up jira for this.
--
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]
