adoroszlai commented on code in PR #3242:
URL: https://github.com/apache/ozone/pull/3242#discussion_r852651775
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsGenerator.java:
##########
@@ -28,6 +28,7 @@
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import com.codahale.metrics.Timer;
+import org.apache.hadoop.ozone.freon.ContentGenerator.Builder;
Review Comment:
```suggestion
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsGenerator.java:
##########
@@ -89,7 +90,11 @@ public Void call() throws Exception {
}
contentGenerator =
- new ContentGenerator(fileSize, bufferSize, copyBufferSize);
+ new Builder()
Review Comment:
Nit: I think it's more readable if builder is qualified where it is being
used (and also more consistent with usage in other generator classes).
```suggestion
new ContentGenerator.Builder()
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ContentGenerator.java:
##########
@@ -48,38 +49,101 @@ public class ContentGenerator {
private final byte[] buffer;
- ContentGenerator(long keySize, int bufferSize) {
- this(keySize, bufferSize, bufferSize);
- }
+ /**
+ * Issue Hsync after every write ( Cannot be used with Hflush ).
+ */
+ private final boolean hSync;
+
+ /**
+ * Issue Hflush after every write ( Cannot be used with Hsync ).
+ */
+ private final boolean hFlush;
- ContentGenerator(long keySize, int bufferSize, int copyBufferSize) {
- this.keySize = keySize;
- this.bufferSize = bufferSize;
- this.copyBufferSize = copyBufferSize;
+ ContentGenerator(Builder objectBuild) {
+ this.keySize = objectBuild.keySize;
+ this.bufferSize = objectBuild.bufferSize;
+ this.copyBufferSize = objectBuild.copyBufferSize;
+ this.hSync = objectBuild.hSync;
+ this.hFlush = objectBuild.hFlush;
buffer = RandomStringUtils.randomAscii(bufferSize)
.getBytes(StandardCharsets.UTF_8);
}
- /**
- * Write the required bytes to the output stream.
- */
+
public void write(OutputStream outputStream) throws IOException {
- for (long nrRemaining = keySize;
- nrRemaining > 0; nrRemaining -= bufferSize) {
+ for (long nrRemaining = keySize; nrRemaining > 0;
+ nrRemaining -= bufferSize) {
int curSize = (int) Math.min(bufferSize, nrRemaining);
if (copyBufferSize == 1) {
for (int i = 0; i < curSize; i++) {
outputStream.write(buffer[i]);
+ flushOrSync(outputStream);
}
} else {
+ /**
+ * Write the required bytes to the output stream.
+ */
for (int i = 0; i < curSize; i += copyBufferSize) {
- outputStream.write(buffer, i,
- Math.min(copyBufferSize, curSize - i));
+ outputStream.write(buffer, i, Math.min(copyBufferSize, curSize - i));
+ flushOrSync(outputStream);
}
}
}
}
+ private void flushOrSync(OutputStream outputStream) throws IOException {
+ if (outputStream instanceof FSDataOutputStream) {
+ if (hSync) {
+ ((FSDataOutputStream) outputStream).hsync();
+ } else if (hFlush) {
+ ((FSDataOutputStream) outputStream).hflush();
+ }
+ }
+ }
+
+ /**
+ * Builder Class for Content generator.
+ */
+ public static class Builder {
+
+ private long keySize = 1024;
+ private int bufferSize = 1024;
+ private int copyBufferSize = 1024;
+ private boolean hSync = false;
+ private boolean hFlush = false;
+
+ public Builder seyKeySize(long keysize) {
+ this.keySize = keysize;
+ return this;
+ }
+
+ public Builder setBufferSize(int buffersize) {
+ this.bufferSize = buffersize;
+ return this;
+ }
+
+ public Builder setCopyBufferSize(int copybuffersize) {
+ this.copyBufferSize = copybuffersize;
+ return this;
+ }
+
+ public Builder setHsyncData(boolean hsync) {
+ this.hSync = hsync;
+ return this;
+ }
+
+ public Builder setHflushData(boolean hflush) {
+ this.hFlush = hflush;
+ return this;
+ }
+
+ //Return the final constructed builder object
+ public ContentGenerator build() {
+ ContentGenerator contentgenerator = new ContentGenerator(this);
+ return contentgenerator;
Review Comment:
Builder's `build` method should verify that the object being constructed is
valid. Here `hflush` and `hsync` are not applicable at the same time. Please
add a check for that.
Nit: no need for local variable, return directly.
```suggestion
return new ContentGenerator(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]
