Skip to content

HDFS-15987. Improve oiv tool to parse fsimage file in parallel with d… #2918

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

Closed
whbing wants to merge 8 commits into from
Closed

HDFS-15987. Improve oiv tool to parse fsimage file in parallel with d… #2918

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
91e3eb2
Merge pull request #1 from apache/trunk
whbing Apr 23, 2021
97aab8c
Merge pull request #2 from apache/trunk
whbing May 10, 2021
3ddf00a
Merge remote-tracking branch 'apache/trunk' into w_trunk
whbing May 14, 2021
8ff25ea
Merge remote-tracking branch 'apache/trunk' into w_trunk
Aug 27, 2021
66502f9
HDFS-15987. Improve oiv tool to parse fsimage file in parallel with d…
whbing Apr 16, 2021
f99527d
Replace threads with thread pools and some other small fixes.
Mar 15, 2022
0dbfa74
rename threads option.
Mar 16, 2022
362762d
fix checkstyle.
Mar 17, 2022
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
10 changes: 6 additions & 4 deletions ...s/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageViewerPB.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public class OfflineImageViewerPB {
+ " Delimited outputs. If not set, the processor\n"
+ " constructs the namespace in memory \n"
+ " before outputting text.\n"
+ "-threads <args> Use multiple threads to process sub-sections.\n"
+ "-h,--help Display usage information and exit\n";

/**
Expand All @@ -132,6 +133,7 @@ private static Options buildOptions() {
options.addOption("delimiter", true, "");
options.addOption("sp", false, "");
options.addOption("t", "temp", true, "");
options.addOption("threads", true, "");
Copy link
Contributor

@Hexiaoqiao Hexiaoqiao Jun 1, 2021

Choose a reason for hiding this comment

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

I am concerned that parameter -threads will collide with -t, and it could be parsed to -t hreads here. It is safe to changes names to another one to avoid abuse.

Copy link
Contributor Author

@whbing whbing Jun 2, 2021

Choose a reason for hiding this comment

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

Yes, it should be renamed and will be fixed in next commit. By the way, is there a suitable parameter name recommendation ? 😊

Copy link
Contributor

@Hexiaoqiao Hexiaoqiao Mar 16, 2022

Choose a reason for hiding this comment

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

This option name threads has to rename here. Otherwise it will conflict with -t.

Copy link
Contributor Author

@whbing whbing Mar 17, 2022

Choose a reason for hiding this comment

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

This option name threads has to rename here. Otherwise it will conflict with -t.

New option as follow. Point it out If any better option, thanks!

options.addOption("m", "multiThread", true, "");


return options;
}
Expand Down Expand Up @@ -185,6 +187,7 @@ public static int run(String[] args) throws Exception {
String delimiter = cmd.getOptionValue("delimiter",
PBImageTextWriter.DEFAULT_DELIMITER);
String tempPath = cmd.getOptionValue("t", "");
int threads = Integer.parseInt(cmd.getOptionValue("threads", "1"));

Configuration conf = new Configuration();
PrintStream out = null;
Expand Down Expand Up @@ -227,15 +230,14 @@ public static int run(String[] args) throws Exception {
boolean printStoragePolicy = cmd.hasOption("sp");
try (PBImageDelimitedTextWriter writer =
new PBImageDelimitedTextWriter(out, delimiter,
tempPath, printStoragePolicy);
RandomAccessFile r = new RandomAccessFile(inputFile, "r")) {
writer.visit(r);
tempPath, printStoragePolicy, threads, outputFile)) {
writer.visit(inputFile);
}
break;
case "DETECTCORRUPTION":
try (PBImageCorruptionDetector detector =
new PBImageCorruptionDetector(out, delimiter, tempPath)) {
detector.visit(new RandomAccessFile(inputFile, "r"));
detector.visit(inputFile);
}
break;
default:
Expand Down
2 changes: 1 addition & 1 deletion .../main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/PBImageCorruptionDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public void afterOutput() throws IOException {
if (parentId != -1) {
entryBuilder.setParentId(parentId);
}
printIfNotEmpty(entryBuilder.build());
printIfNotEmpty(out, entryBuilder.build());
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion ...main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/PBImageDelimitedTextWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,13 @@ public String build() {
PBImageDelimitedTextWriter(PrintStream out, String delimiter,
String tempPath, boolean printStoragePolicy)
throws IOException {
super(out, delimiter, tempPath);
this(out, delimiter, tempPath, printStoragePolicy, 1, "-");
Copy link
Contributor

@Hexiaoqiao Hexiaoqiao Jun 1, 2021

Choose a reason for hiding this comment

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

I am confused why we use static string "-" rather than null. any other consideration?

Copy link
Contributor Author

@whbing whbing Jun 2, 2021

Choose a reason for hiding this comment

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

Considering the default value of outputFile in OfflineImageViewerPB.java is "-", I continue this style.

    String outputFile = cmd.getOptionValue("o", "-");

}

PBImageDelimitedTextWriter(PrintStream out, String delimiter,
String tempPath, boolean printStoragePolicy, int threads,
String parallelOut) throws IOException {
super(out, delimiter, tempPath, threads, parallelOut);
this.printStoragePolicy = printStoragePolicy;
}

Expand Down
Loading