From aba8e92c70195623b800c88789227c3aea3f3cf7 Mon Sep 17 00:00:00 2001
From: ermahesh <mahi.ramuk@gmail.com>
Date: Tue, 8 Sep 2026 14:18:49 -0500
Subject: [PATCH 1/4] HDDS-16354. Make the dfsrw read/write ratio configurable

The dfsrw workload paired one read with every write, pinning the mix at
1:1.  Real workloads are rarely balanced, so --read-write-ratio now sets
how many reads a write is followed by: 4 reads back each write four
times, 0.25 reads back every fourth write, and the default of 1 leaves
the workload as it was.

Reads stay inside the thread that wrote the file, so a read still
validates the CRC32 of the latest write of the path it reads, and an
overwritten path returning older bytes is still detected.  A ratio that
is not a whole number is accumulated per thread rather than rounded on
every write, otherwise every ratio below 0.5 would read back nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../freon/HadoopFsReadWriteValidator.java     |  47 ++-
 .../freon/TestHadoopFsReadWriteRatio.java     | 274 ++++++++++++++++++
 2 files changed, 320 insertions(+), 1 deletion(-)
 create mode 100644 hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java

diff --git a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
index 86ab8192d6b9..4cc324240e1f 100644
--- a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
+++ b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
@@ -49,6 +49,10 @@
  * concurrent load, including in time-based (--duration) runs where paths are
  * reused.
  * <p>
+ * --read-write-ratio tunes the mix: a task still writes a single file, but
+ * reads back as many as the ratio asks for, so a run can be made read-heavy or
+ * write-heavy without changing what any one read validates.
+ * <p>
  * CRC32 keeps the validation off the critical path of the measured throughput.
  * Successive writes of a path differ in the marker only, and markers less than
  * 2^32 apart never share a CRC32, so a stale read is always detected.
@@ -89,6 +93,14 @@ public class HadoopFsReadWriteValidator extends HadoopBaseFreonGenerator
       defaultValue = "10000")
   private int maxFilesPerThread;
 
+  @Option(names = {"--read-write-ratio"},
+      description = "Number of reads issued per write. 1 pairs one read with every write, 4 makes the run "
+          + "read-heavy with four read-backs of each write, and 0.25 makes it write-heavy with one read-back "
+          + "every fourth write. A ratio that is not a whole number is spread over the writes of a thread "
+          + "rather than rounded on every one of them.",
+      defaultValue = "1.0")
+  private double readWriteRatio;
+
   private ContentGenerator contentGenerator;
 
   private Timer writeTimer;
@@ -113,6 +125,12 @@ public Void call() throws Exception {
       throw new IllegalArgumentException(
           "--max-files-per-thread must be positive");
     }
+    // NaN fails the first test, and an infinite ratio would make a single task
+    // read forever
+    if (!(readWriteRatio > 0) || Double.isInfinite(readWriteRatio)) {
+      throw new IllegalArgumentException(
+          "--read-write-ratio must be a positive finite number");
+    }
 
     super.init();
 
@@ -155,6 +173,17 @@ private void writeAndValidate(long counter) throws Exception {
     }
     history.record(fileId, checksum);
 
+    for (int reads = history.readsDue(readWriteRatio); reads > 0; reads--) {
+      validateRandomFile(history);
+    }
+  }
+
+  /**
+   * Read back one of the files this thread wrote, picked at random, and verify
+   * that its content still matches the checksum of the latest write of that
+   * path.
+   */
+  private void validateRandomFile(ThreadHistory history) throws Exception {
     long readId = history.randomFileId();
     Path target = objectPath(readId);
     long expected = history.checksumOf(readId);
@@ -215,13 +244,15 @@ private long readChecksum(Path file) throws IOException {
    * is keyed by file id (an overwrite updates the checksum), so it holds one
    * entry per file the thread wrote and never more than
    * --max-files-per-thread. The id is kept rather than the {@link Path} it maps
-   * to, which {@link #objectPath} rebuilds on demand.
+   * to, which {@link #objectPath} rebuilds on demand. It also carries the
+   * thread's share of the read/write ratio, see {@link #readsDue(double)}.
    */
   private static final class ThreadHistory {
     private final long markerBase;
     private int markerSeq;
     private final Map<Long, Long> checksums = new HashMap<>();
     private final List<Long> fileIds = new ArrayList<>();
+    private double readCredit;
 
     private ThreadHistory(long threadSequenceId) {
       this.markerBase = threadSequenceId << Integer.SIZE;
@@ -259,5 +290,19 @@ private long randomFileId() {
     private long checksumOf(long fileId) {
       return checksums.get(fileId);
     }
+
+    /**
+     * How many files to read back after the write that just completed. The
+     * fraction a ratio like 2.5 leaves over is carried to the next write
+     * instead of being rounded away, so the thread issues the requested number
+     * of reads per write on average, and a ratio below 1 reads back every
+     * n-th write rather than never reading at all.
+     */
+    private int readsDue(double readsPerWrite) {
+      readCredit += readsPerWrite;
+      int reads = (int) readCredit;
+      readCredit -= reads;
+      return reads;
+    }
   }
 }
diff --git a/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
new file mode 100644
index 000000000000..9fd7aedc9539
--- /dev/null
+++ b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
@@ -0,0 +1,274 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.freon;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.LocalFileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.PositionedReadable;
+import org.apache.hadoop.fs.Seekable;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import picocli.CommandLine;
+
+/**
+ * Verifies that {@code --read-write-ratio} of {@link HadoopFsReadWriteValidator}
+ * (dfsrw) controls how many reads the run issues per write. The workload only
+ * needs a Hadoop {@code FileSystem}, so these run against the local one instead
+ * of a cluster.
+ */
+public class TestHadoopFsReadWriteRatio {
+
+  private static final int WRITES = 8;
+
+  @TempDir
+  private java.nio.file.Path tempDir;
+
+  private String rootPath;
+
+  @BeforeEach
+  void setUp() {
+    // toUri() rather than the path itself: on Windows the latter is not a valid
+    // URI, it has backslashes and a drive letter where the authority goes
+    String uri = tempDir.toUri().toString();
+    rootPath = uri.endsWith("/") ? uri.substring(0, uri.length() - 1) : uri;
+    CorruptingLocalFileSystem.corruptFromRead(Integer.MAX_VALUE);
+  }
+
+  /**
+   * One read per write is the default, and it stays that way when the ratio is
+   * given explicitly.
+   */
+  @Test
+  void pairsOneReadWithEveryWriteByDefault() {
+    CommandLine cmd = runValidator(2);
+
+    assertEquals(WRITES, writeCount(cmd));
+    assertEquals(WRITES, readCount(cmd));
+  }
+
+  /**
+   * A whole number of reads per write holds however the writes are spread over
+   * the threads, so this can afford more than one of them.
+   */
+  @ParameterizedTest
+  @ValueSource(ints = {2, 3, 5})
+  void issuesRequestedReadsPerWrite(int ratio) {
+    CommandLine cmd =
+        runValidator(2, "--read-write-ratio", String.valueOf(ratio));
+
+    assertEquals(WRITES, writeCount(cmd));
+    assertEquals((long) WRITES * ratio, readCount(cmd));
+  }
+
+  /**
+   * A ratio below 1 reads back only every n-th write, and the fraction it
+   * leaves over is carried between writes rather than rounded away on each of
+   * them, which would read back nothing at all. The leftover is per thread, so
+   * only a single thread gives an exact count.
+   */
+  @Test
+  void spreadsFractionalRatioOverWrites() {
+    CommandLine cmd = runValidator(1, "--read-write-ratio", "0.25");
+
+    assertEquals(WRITES, writeCount(cmd));
+    assertEquals(WRITES / 4, readCount(cmd));
+  }
+
+  /** A ratio with a whole and a fractional part mixes the two behaviours. */
+  @Test
+  void spreadsMixedRatioOverWrites() {
+    CommandLine cmd = runValidator(1, "--read-write-ratio", "1.5");
+
+    assertEquals(WRITES, writeCount(cmd));
+    assertEquals(WRITES * 3 / 2, readCount(cmd));
+  }
+
+  /**
+   * Every read of a task validates, not only the first one: a single write read
+   * back three times fails the run when the content is corrupted before the
+   * last of the three reads.
+   */
+  @Test
+  void everyReadOfATaskValidatesContent() {
+    CorruptingLocalFileSystem.corruptFromRead(3);
+
+    int exitCode = new Freon().getCmd().execute(
+        "-D", "fs.file.impl=" + CorruptingLocalFileSystem.class.getName(),
+        "dfsrw",
+        "-r", rootPath,
+        "-p", "dfsrw-corrupt",
+        "-n", "1",
+        "-t", "1",
+        "-s", "1KB",
+        "--buffer", "1024",
+        "--copy-buffer", "1024",
+        "--read-write-ratio", "3");
+
+    assertNotEquals(0, exitCode, "Corrupted content was not detected");
+  }
+
+  @Test
+  void rejectsNonPositiveRatio() {
+    assertThat(execute(new Freon().getCmd(), 1, "--read-write-ratio", "0"))
+        .isNotZero();
+    assertThat(execute(new Freon().getCmd(), 1, "--read-write-ratio", "-1"))
+        .isNotZero();
+  }
+
+  private CommandLine runValidator(int threads, String... args) {
+    CommandLine cmd = new Freon().getCmd();
+    assertEquals(0, execute(cmd, threads, args), "Freon dfsrw command failed");
+    return cmd;
+  }
+
+  private int execute(CommandLine cmd, int threads, String... args) {
+    String[] fixed = {
+        "dfsrw",
+        "-r", rootPath,
+        "-p", "dfsrw",
+        "-n", String.valueOf(WRITES),
+        "-t", String.valueOf(threads),
+        "-s", "1KB",
+        "--buffer", "1024",
+        "--copy-buffer", "1024"};
+    String[] argv = new String[fixed.length + args.length];
+    System.arraycopy(fixed, 0, argv, 0, fixed.length);
+    System.arraycopy(args, 0, argv, fixed.length, args.length);
+    return cmd.execute(argv);
+  }
+
+  private static long writeCount(CommandLine cmd) {
+    return timerCount(cmd, "file-write");
+  }
+
+  private static long readCount(CommandLine cmd) {
+    return timerCount(cmd, "file-read-validate");
+  }
+
+  private static long timerCount(CommandLine cmd, String name) {
+    BaseFreonGenerator subject = (BaseFreonGenerator)
+        cmd.getParseResult().subcommand().commandSpec().userObject();
+    return subject.getMetrics().timer(name).getCount();
+  }
+
+  /**
+   * A {@link LocalFileSystem} that alters the content it hands out from the
+   * n-th {@code open()} on. The bytes are changed above the checksum
+   * verification of {@link org.apache.hadoop.fs.ChecksumFileSystem}, so it is
+   * the workload, not the file system, that has to notice.
+   */
+  public static final class CorruptingLocalFileSystem extends LocalFileSystem {
+
+    private static final AtomicInteger READS = new AtomicInteger();
+    private static int corruptFrom = Integer.MAX_VALUE;
+
+    static void corruptFromRead(int read) {
+      READS.set(0);
+      corruptFrom = read;
+    }
+
+    @Override
+    public FSDataInputStream open(Path f, int bufferSize) throws IOException {
+      FSDataInputStream input = super.open(f, bufferSize);
+      // the hidden .crc companions are the checksum bookkeeping of the file
+      // system itself, only the reads of the workload are to be counted
+      if (f.getName().startsWith(".") || READS.incrementAndGet() < corruptFrom) {
+        return input;
+      }
+      return new FSDataInputStream(new FlippingInputStream(input));
+    }
+  }
+
+  /**
+   * Passes the wrapped stream through, with the first byte of the file flipped.
+   */
+  private static final class FlippingInputStream extends InputStream
+      implements Seekable, PositionedReadable {
+
+    private final FSDataInputStream input;
+
+    private FlippingInputStream(FSDataInputStream input) {
+      this.input = input;
+    }
+
+    @Override
+    public int read(byte[] b, int off, int len) throws IOException {
+      long pos = input.getPos();
+      int read = input.read(b, off, len);
+      if (read > 0 && pos == 0) {
+        b[off] ^= 0xff;
+      }
+      return read;
+    }
+
+    @Override
+    public int read() throws IOException {
+      long pos = input.getPos();
+      int b = input.read();
+      return b >= 0 && pos == 0 ? b ^ 0xff : b;
+    }
+
+    @Override
+    public void close() throws IOException {
+      input.close();
+    }
+
+    @Override
+    public void seek(long pos) throws IOException {
+      input.seek(pos);
+    }
+
+    @Override
+    public long getPos() throws IOException {
+      return input.getPos();
+    }
+
+    @Override
+    public boolean seekToNewSource(long targetPos) throws IOException {
+      return input.seekToNewSource(targetPos);
+    }
+
+    @Override
+    public int read(long position, byte[] buffer, int offset, int length)
+        throws IOException {
+      return input.read(position, buffer, offset, length);
+    }
+
+    @Override
+    public void readFully(long position, byte[] buffer, int offset, int length)
+        throws IOException {
+      input.readFully(position, buffer, offset, length);
+    }
+
+    @Override
+    public void readFully(long position, byte[] buffer) throws IOException {
+      input.readFully(position, buffer);
+    }
+  }
+}

From 3900ef9858e22d79552011b1027d4ff6f24f6a31 Mon Sep 17 00:00:00 2001
From: ermahesh <mahi.ramuk@gmail.com>
Date: Thu, 10 Sep 2026 14:28:47 -0500
Subject: [PATCH 2/4] HDDS-16354. Address review: rename the option and carry
 the ratio exactly

- Rename --read-write-ratio to --reads-per-write, and describe the reads as
  validation reads that each pick a file the thread wrote at random, rather
  than as read-backs of the write just made.
- Carry the ratio in whole millionths of a read instead of accumulating it as
  a double, which drifted: adding 0.1 ten times gives 0.9999999999999999, so
  the read the tenth write was due slipped to an eleventh. The option is now
  a BigDecimal, which also removes the NaN and infinity checks.
- Parameterize the corruption point of everyReadOfATaskValidatesContent over
  the first, second and third read, so it no longer covers only the last.
- Extend FlippingInputStream from FSInputStream, which supplies the
  positioned reads and drops three forwarding methods.

Adds carriesFractionalRatioWithoutDrift, which fails on the old accumulator.
---
 .../freon/HadoopFsReadWriteValidator.java     | 69 +++++++++++-----
 .../freon/TestHadoopFsReadWriteRatio.java     | 80 ++++++++++---------
 2 files changed, 90 insertions(+), 59 deletions(-)

diff --git a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
index 4cc324240e1f..7cac085fe98d 100644
--- a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
+++ b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
@@ -19,6 +19,8 @@
 
 import com.codahale.metrics.Timer;
 import java.io.IOException;
+import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -49,8 +51,8 @@
  * concurrent load, including in time-based (--duration) runs where paths are
  * reused.
  * <p>
- * --read-write-ratio tunes the mix: a task still writes a single file, but
- * reads back as many as the ratio asks for, so a run can be made read-heavy or
+ * --reads-per-write tunes the mix: a task still writes a single file, but reads
+ * back as many as the ratio asks for, so a run can be made read-heavy or
  * write-heavy without changing what any one read validates.
  * <p>
  * CRC32 keeps the validation off the critical path of the measured throughput.
@@ -68,6 +70,21 @@
 public class HadoopFsReadWriteValidator extends HadoopBaseFreonGenerator
     implements Callable<Void> {
 
+  /**
+   * One read, in the fixed-point units the read credit of a thread is counted
+   * in. The ratio is converted to whole units once, so the fraction it leaves
+   * over on every write is carried exactly. Accumulating the ratio itself as a
+   * double drifts: adding 0.1 ten times gives 0.9999999999999999, which delays
+   * the read the tenth write is due to an eleventh write.
+   */
+  private static final long CREDIT_UNIT = 1_000_000L;
+
+  /**
+   * Beyond this a run reads back more files per write than it could complete,
+   * and the bound is what keeps the credit of a thread well inside a long.
+   */
+  private static final BigDecimal MAX_READS_PER_WRITE = new BigDecimal("1000000");
+
   @Option(names = {"-s", "--size"},
       description = "Size of the generated files. " +
           StorageSizeConverter.STORAGE_SIZE_DESCRIPTION,
@@ -93,13 +110,17 @@ public class HadoopFsReadWriteValidator extends HadoopBaseFreonGenerator
       defaultValue = "10000")
   private int maxFilesPerThread;
 
-  @Option(names = {"--read-write-ratio"},
-      description = "Number of reads issued per write. 1 pairs one read with every write, 4 makes the run "
-          + "read-heavy with four read-backs of each write, and 0.25 makes it write-heavy with one read-back "
-          + "every fourth write. A ratio that is not a whole number is spread over the writes of a thread "
-          + "rather than rounded on every one of them.",
+  @Option(names = {"--reads-per-write"},
+      description = "Number of validation reads issued per write. 1 pairs one read with every write, 4 makes the "
+          + "run read-heavy with four validation reads per write, and 0.25 makes it write-heavy with one read "
+          + "every fourth write. Every read picks a file the thread wrote at random, so this changes how many "
+          + "files a task validates, not which write it validates. A value that is not a whole number is spread "
+          + "over the writes of a thread rather than rounded on every one of them.",
       defaultValue = "1.0")
-  private double readWriteRatio;
+  private BigDecimal readsPerWrite;
+
+  /** {@link #readsPerWrite} in the units of {@link ThreadHistory#readCredit}. */
+  private long creditPerWrite;
 
   private ContentGenerator contentGenerator;
 
@@ -125,12 +146,16 @@ public Void call() throws Exception {
       throw new IllegalArgumentException(
           "--max-files-per-thread must be positive");
     }
-    // NaN fails the first test, and an infinite ratio would make a single task
-    // read forever
-    if (!(readWriteRatio > 0) || Double.isInfinite(readWriteRatio)) {
-      throw new IllegalArgumentException(
-          "--read-write-ratio must be a positive finite number");
+    if (readsPerWrite.signum() <= 0
+        || readsPerWrite.compareTo(MAX_READS_PER_WRITE) > 0) {
+      throw new IllegalArgumentException("--reads-per-write must be positive "
+          + "and at most " + MAX_READS_PER_WRITE.toPlainString());
     }
+    // Rounded away from zero, so a ratio finer than a credit unit still reads
+    // back now and then instead of silently never reading at all.
+    creditPerWrite = readsPerWrite.multiply(BigDecimal.valueOf(CREDIT_UNIT))
+        .setScale(0, RoundingMode.UP)
+        .longValueExact();
 
     super.init();
 
@@ -173,7 +198,7 @@ private void writeAndValidate(long counter) throws Exception {
     }
     history.record(fileId, checksum);
 
-    for (int reads = history.readsDue(readWriteRatio); reads > 0; reads--) {
+    for (int reads = history.readsDue(creditPerWrite); reads > 0; reads--) {
       validateRandomFile(history);
     }
   }
@@ -245,14 +270,15 @@ private long readChecksum(Path file) throws IOException {
    * entry per file the thread wrote and never more than
    * --max-files-per-thread. The id is kept rather than the {@link Path} it maps
    * to, which {@link #objectPath} rebuilds on demand. It also carries the
-   * thread's share of the read/write ratio, see {@link #readsDue(double)}.
+   * thread's share of the read/write ratio, see {@link #readsDue(long)}.
    */
   private static final class ThreadHistory {
     private final long markerBase;
     private int markerSeq;
     private final Map<Long, Long> checksums = new HashMap<>();
     private final List<Long> fileIds = new ArrayList<>();
-    private double readCredit;
+    /** Owed reads, in {@link #CREDIT_UNIT}s, left over by earlier writes. */
+    private long readCredit;
 
     private ThreadHistory(long threadSequenceId) {
       this.markerBase = threadSequenceId << Integer.SIZE;
@@ -296,12 +322,13 @@ private long checksumOf(long fileId) {
      * fraction a ratio like 2.5 leaves over is carried to the next write
      * instead of being rounded away, so the thread issues the requested number
      * of reads per write on average, and a ratio below 1 reads back every
-     * n-th write rather than never reading at all.
+     * n-th write rather than never reading at all. The carry is exact: it is
+     * counted in whole {@link #CREDIT_UNIT}s, not accumulated as a double.
      */
-    private int readsDue(double readsPerWrite) {
-      readCredit += readsPerWrite;
-      int reads = (int) readCredit;
-      readCredit -= reads;
+    private int readsDue(long creditPerWrite) {
+      readCredit += creditPerWrite;
+      int reads = (int) (readCredit / CREDIT_UNIT);
+      readCredit %= CREDIT_UNIT;
       return reads;
     }
   }
diff --git a/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
index 9fd7aedc9539..26fcb8937249 100644
--- a/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
+++ b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
@@ -22,13 +22,11 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSInputStream;
 import org.apache.hadoop.fs.LocalFileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.PositionedReadable;
-import org.apache.hadoop.fs.Seekable;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -37,7 +35,7 @@
 import picocli.CommandLine;
 
 /**
- * Verifies that {@code --read-write-ratio} of {@link HadoopFsReadWriteValidator}
+ * Verifies that {@code --reads-per-write} of {@link HadoopFsReadWriteValidator}
  * (dfsrw) controls how many reads the run issues per write. The workload only
  * needs a Hadoop {@code FileSystem}, so these run against the local one instead
  * of a cluster.
@@ -80,7 +78,7 @@ void pairsOneReadWithEveryWriteByDefault() {
   @ValueSource(ints = {2, 3, 5})
   void issuesRequestedReadsPerWrite(int ratio) {
     CommandLine cmd =
-        runValidator(2, "--read-write-ratio", String.valueOf(ratio));
+        runValidator(2, "--reads-per-write", String.valueOf(ratio));
 
     assertEquals(WRITES, writeCount(cmd));
     assertEquals((long) WRITES * ratio, readCount(cmd));
@@ -94,7 +92,7 @@ void issuesRequestedReadsPerWrite(int ratio) {
    */
   @Test
   void spreadsFractionalRatioOverWrites() {
-    CommandLine cmd = runValidator(1, "--read-write-ratio", "0.25");
+    CommandLine cmd = runValidator(1, "--reads-per-write", "0.25");
 
     assertEquals(WRITES, writeCount(cmd));
     assertEquals(WRITES / 4, readCount(cmd));
@@ -103,20 +101,35 @@ void spreadsFractionalRatioOverWrites() {
   /** A ratio with a whole and a fractional part mixes the two behaviours. */
   @Test
   void spreadsMixedRatioOverWrites() {
-    CommandLine cmd = runValidator(1, "--read-write-ratio", "1.5");
+    CommandLine cmd = runValidator(1, "--reads-per-write", "1.5");
 
     assertEquals(WRITES, writeCount(cmd));
     assertEquals(WRITES * 3 / 2, readCount(cmd));
   }
 
   /**
-   * Every read of a task validates, not only the first one: a single write read
-   * back three times fails the run when the content is corrupted before the
-   * last of the three reads.
+   * The leftover of a fractional ratio is carried exactly, so the tenth write
+   * of a run at 0.1 is read back. A ratio accumulated as a double drifts —
+   * adding 0.1 ten times gives 0.9999999999999999 — which would put that read
+   * off to an eleventh write a ten-write run never makes.
    */
   @Test
-  void everyReadOfATaskValidatesContent() {
-    CorruptingLocalFileSystem.corruptFromRead(3);
+  void carriesFractionalRatioWithoutDrift() {
+    CommandLine cmd = runValidator(1, 10, "--reads-per-write", "0.1");
+
+    assertEquals(10, writeCount(cmd));
+    assertEquals(1, readCount(cmd));
+  }
+
+  /**
+   * Every read of a task validates, not only the first or the last one: a
+   * single write read back three times fails the run wherever among the three
+   * the content is corrupted.
+   */
+  @ParameterizedTest
+  @ValueSource(ints = {1, 2, 3})
+  void everyReadOfATaskValidatesContent(int corruptedRead) {
+    CorruptingLocalFileSystem.corruptFromRead(corruptedRead);
 
     int exitCode = new Freon().getCmd().execute(
         "-D", "fs.file.impl=" + CorruptingLocalFileSystem.class.getName(),
@@ -128,31 +141,38 @@ void everyReadOfATaskValidatesContent() {
         "-s", "1KB",
         "--buffer", "1024",
         "--copy-buffer", "1024",
-        "--read-write-ratio", "3");
+        "--reads-per-write", "3");
 
-    assertNotEquals(0, exitCode, "Corrupted content was not detected");
+    assertNotEquals(0, exitCode,
+        "Corrupted content of read " + corruptedRead + " was not detected");
   }
 
   @Test
   void rejectsNonPositiveRatio() {
-    assertThat(execute(new Freon().getCmd(), 1, "--read-write-ratio", "0"))
+    assertThat(execute(new Freon().getCmd(), 1, WRITES, "--reads-per-write", "0"))
         .isNotZero();
-    assertThat(execute(new Freon().getCmd(), 1, "--read-write-ratio", "-1"))
+    assertThat(execute(new Freon().getCmd(), 1, WRITES, "--reads-per-write", "-1"))
         .isNotZero();
   }
 
   private CommandLine runValidator(int threads, String... args) {
+    return runValidator(threads, WRITES, args);
+  }
+
+  private CommandLine runValidator(int threads, int writes, String... args) {
     CommandLine cmd = new Freon().getCmd();
-    assertEquals(0, execute(cmd, threads, args), "Freon dfsrw command failed");
+    assertEquals(0, execute(cmd, threads, writes, args),
+        "Freon dfsrw command failed");
     return cmd;
   }
 
-  private int execute(CommandLine cmd, int threads, String... args) {
+  private int execute(CommandLine cmd, int threads, int writes,
+      String... args) {
     String[] fixed = {
         "dfsrw",
         "-r", rootPath,
         "-p", "dfsrw",
-        "-n", String.valueOf(WRITES),
+        "-n", String.valueOf(writes),
         "-t", String.valueOf(threads),
         "-s", "1KB",
         "--buffer", "1024",
@@ -207,9 +227,10 @@ public FSDataInputStream open(Path f, int bufferSize) throws IOException {
 
   /**
    * Passes the wrapped stream through, with the first byte of the file flipped.
+   * {@link FSInputStream} supplies the positioned reads on top of seek and
+   * read, so only those two and the plain reads are forwarded here.
    */
-  private static final class FlippingInputStream extends InputStream
-      implements Seekable, PositionedReadable {
+  private static final class FlippingInputStream extends FSInputStream {
 
     private final FSDataInputStream input;
 
@@ -253,22 +274,5 @@ public long getPos() throws IOException {
     public boolean seekToNewSource(long targetPos) throws IOException {
       return input.seekToNewSource(targetPos);
     }
-
-    @Override
-    public int read(long position, byte[] buffer, int offset, int length)
-        throws IOException {
-      return input.read(position, buffer, offset, length);
-    }
-
-    @Override
-    public void readFully(long position, byte[] buffer, int offset, int length)
-        throws IOException {
-      input.readFully(position, buffer, offset, length);
-    }
-
-    @Override
-    public void readFully(long position, byte[] buffer) throws IOException {
-      input.readFully(position, buffer);
-    }
   }
 }

From a38f87273e04a0c2dc758f98dc5246e546b8056e Mon Sep 17 00:00:00 2001
From: ermahesh <mahi.ramuk@gmail.com>
Date: Fri, 11 Sep 2026 15:38:51 -0500
Subject: [PATCH 3/4] HDDS-16354. Replace --reads-per-write with --read-percent
 in dfsrw

Review feedback: express the read/write mix as a percentage and draw
per operation instead of issuing a fixed number of reads after every
write.

Every operation of the run now independently draws whether it writes a
file or reads one back, so -n counts operations of both kinds and the
split holds on average rather than exactly. This removes the fixed-point
carry machinery that was needed to spread a fractional reads-per-write
ratio across writes.

Two invariants the ratio model got for free need holding explicitly: a
thread whose history is still empty has nothing to read and writes
instead, and the file a write goes to is now named after the write
sequence in the thread's own marker rather than the task counter, which
no longer counts that thread's writes. That keeps a thread within
--max-files-per-thread however its reads and writes happen to fall.
---
 .../freon/HadoopFsReadWriteValidator.java     | 120 ++++++++--------
 ...atio.java => TestHadoopFsReadPercent.java} | 130 +++++++++---------
 2 files changed, 120 insertions(+), 130 deletions(-)
 rename hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/{TestHadoopFsReadWriteRatio.java => TestHadoopFsReadPercent.java} (64%)

diff --git a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
index 7cac085fe98d..6a2310063ed7 100644
--- a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
+++ b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
@@ -19,8 +19,6 @@
 
 import com.codahale.metrics.Timer;
 import java.io.IOException;
-import java.math.BigDecimal;
-import java.math.RoundingMode;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -51,9 +49,11 @@
  * concurrent load, including in time-based (--duration) runs where paths are
  * reused.
  * <p>
- * --reads-per-write tunes the mix: a task still writes a single file, but reads
- * back as many as the ratio asks for, so a run can be made read-heavy or
- * write-heavy without changing what any one read validates.
+ * --read-percent tunes the mix: every operation of the run independently draws
+ * whether to write a file or to read one back, so a run can be made read-heavy
+ * or write-heavy without changing what any one read validates. -n therefore
+ * counts operations of both kinds rather than writes alone, and the split holds
+ * on average rather than exactly.
  * <p>
  * CRC32 keeps the validation off the critical path of the measured throughput.
  * Successive writes of a path differ in the marker only, and markers less than
@@ -70,21 +70,6 @@
 public class HadoopFsReadWriteValidator extends HadoopBaseFreonGenerator
     implements Callable<Void> {
 
-  /**
-   * One read, in the fixed-point units the read credit of a thread is counted
-   * in. The ratio is converted to whole units once, so the fraction it leaves
-   * over on every write is carried exactly. Accumulating the ratio itself as a
-   * double drifts: adding 0.1 ten times gives 0.9999999999999999, which delays
-   * the read the tenth write is due to an eleventh write.
-   */
-  private static final long CREDIT_UNIT = 1_000_000L;
-
-  /**
-   * Beyond this a run reads back more files per write than it could complete,
-   * and the bound is what keeps the credit of a thread well inside a long.
-   */
-  private static final BigDecimal MAX_READS_PER_WRITE = new BigDecimal("1000000");
-
   @Option(names = {"-s", "--size"},
       description = "Size of the generated files. " +
           StorageSizeConverter.STORAGE_SIZE_DESCRIPTION,
@@ -110,17 +95,14 @@ public class HadoopFsReadWriteValidator extends HadoopBaseFreonGenerator
       defaultValue = "10000")
   private int maxFilesPerThread;
 
-  @Option(names = {"--reads-per-write"},
-      description = "Number of validation reads issued per write. 1 pairs one read with every write, 4 makes the "
-          + "run read-heavy with four validation reads per write, and 0.25 makes it write-heavy with one read "
-          + "every fourth write. Every read picks a file the thread wrote at random, so this changes how many "
-          + "files a task validates, not which write it validates. A value that is not a whole number is spread "
-          + "over the writes of a thread rather than rounded on every one of them.",
-      defaultValue = "1.0")
-  private BigDecimal readsPerWrite;
-
-  /** {@link #readsPerWrite} in the units of {@link ThreadHistory#readCredit}. */
-  private long creditPerWrite;
+  @Option(names = {"--read-percent"},
+      description = "Percentage of the operations that read a file back and validate it instead of writing one. "
+          + "0 only writes, 50 pairs a read with every write on average, and 90 makes the run read-heavy. Every "
+          + "operation draws on its own, so the split holds over a run rather than on any particular pair of "
+          + "operations. A read validates a file the thread wrote, picked at random; a thread that has written "
+          + "nothing yet has nothing to read and writes instead.",
+      defaultValue = "50")
+  private double readPercent;
 
   private ContentGenerator contentGenerator;
 
@@ -146,16 +128,11 @@ public Void call() throws Exception {
       throw new IllegalArgumentException(
           "--max-files-per-thread must be positive");
     }
-    if (readsPerWrite.signum() <= 0
-        || readsPerWrite.compareTo(MAX_READS_PER_WRITE) > 0) {
-      throw new IllegalArgumentException("--reads-per-write must be positive "
-          + "and at most " + MAX_READS_PER_WRITE.toPlainString());
+    // negated so that NaN, which fails every comparison, is rejected too
+    if (!(readPercent >= 0 && readPercent <= 100)) {
+      throw new IllegalArgumentException(
+          "--read-percent must be between 0 and 100");
     }
-    // Rounded away from zero, so a ratio finer than a credit unit still reads
-    // back now and then instead of silently never reading at all.
-    creditPerWrite = readsPerWrite.multiply(BigDecimal.valueOf(CREDIT_UNIT))
-        .setScale(0, RoundingMode.UP)
-        .longValueExact();
 
     super.init();
 
@@ -172,7 +149,7 @@ public Void call() throws Exception {
       writeTimer = getMetrics().timer("file-write");
       readTimer = getMetrics().timer("file-read-validate");
 
-      runTests(this::writeAndValidate);
+      runTests(this::readOrWrite);
     } finally {
       org.apache.hadoop.hdds.utils.IOUtils.closeQuietly(fileSystem);
     }
@@ -180,11 +157,30 @@ public Void call() throws Exception {
     return null;
   }
 
-  private void writeAndValidate(long counter) throws Exception {
+  /**
+   * One operation of the run, a read or a write. The counter of the task is not
+   * what the written file is named after: with the two kinds of operation drawn
+   * at random it no longer counts the writes of the thread, which is what has
+   * to stay within --max-files-per-thread.
+   */
+  private void readOrWrite(long counter) throws Exception {
     ThreadHistory history = threadHistory.get();
-    long fileId = counter % maxFilesPerThread;
-    Path file = objectPath(fileId);
+    if (history.isEmpty() || !readsNext()) {
+      writeAndRecord(history);
+    } else {
+      validateRandomFile(history);
+    }
+  }
+
+  /** Draws whether this operation reads, see --read-percent. */
+  private boolean readsNext() {
+    return ThreadLocalRandom.current().nextDouble(100) < readPercent;
+  }
+
+  private void writeAndRecord(ThreadHistory history) throws Exception {
     long marker = history.nextMarker();
+    long fileId = fileIdOf(marker);
+    Path file = objectPath(fileId);
 
     long checksum;
     try {
@@ -197,10 +193,15 @@ private void writeAndValidate(long counter) throws Exception {
       throw e;
     }
     history.record(fileId, checksum);
+  }
 
-    for (int reads = history.readsDue(creditPerWrite); reads > 0; reads--) {
-      validateRandomFile(history);
-    }
+  /**
+   * File a write goes to. The low half of its marker is the write sequence of
+   * the thread, so ids cycle within --max-files-per-thread and a thread keeps
+   * at most that many checksums however its reads and writes fall.
+   */
+  private long fileIdOf(long marker) {
+    return (marker & 0xFFFFFFFFL) % maxFilesPerThread;
   }
 
   /**
@@ -269,16 +270,13 @@ private long readChecksum(Path file) throws IOException {
    * is keyed by file id (an overwrite updates the checksum), so it holds one
    * entry per file the thread wrote and never more than
    * --max-files-per-thread. The id is kept rather than the {@link Path} it maps
-   * to, which {@link #objectPath} rebuilds on demand. It also carries the
-   * thread's share of the read/write ratio, see {@link #readsDue(long)}.
+   * to, which {@link #objectPath} rebuilds on demand.
    */
   private static final class ThreadHistory {
     private final long markerBase;
     private int markerSeq;
     private final Map<Long, Long> checksums = new HashMap<>();
     private final List<Long> fileIds = new ArrayList<>();
-    /** Owed reads, in {@link #CREDIT_UNIT}s, left over by earlier writes. */
-    private long readCredit;
 
     private ThreadHistory(long threadSequenceId) {
       this.markerBase = threadSequenceId << Integer.SIZE;
@@ -309,6 +307,11 @@ private void forget(long fileId) {
       }
     }
 
+    /** Whether the thread has written anything it could read back yet. */
+    private boolean isEmpty() {
+      return fileIds.isEmpty();
+    }
+
     private long randomFileId() {
       return fileIds.get(ThreadLocalRandom.current().nextInt(fileIds.size()));
     }
@@ -316,20 +319,5 @@ private long randomFileId() {
     private long checksumOf(long fileId) {
       return checksums.get(fileId);
     }
-
-    /**
-     * How many files to read back after the write that just completed. The
-     * fraction a ratio like 2.5 leaves over is carried to the next write
-     * instead of being rounded away, so the thread issues the requested number
-     * of reads per write on average, and a ratio below 1 reads back every
-     * n-th write rather than never reading at all. The carry is exact: it is
-     * counted in whole {@link #CREDIT_UNIT}s, not accumulated as a double.
-     */
-    private int readsDue(long creditPerWrite) {
-      readCredit += creditPerWrite;
-      int reads = (int) (readCredit / CREDIT_UNIT);
-      readCredit %= CREDIT_UNIT;
-      return reads;
-    }
   }
 }
diff --git a/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadPercent.java
similarity index 64%
rename from hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
rename to hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadPercent.java
index 26fcb8937249..3ab3bfacde54 100644
--- a/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteRatio.java
+++ b/hadoop-ozone/freon/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadPercent.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.ozone.freon;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.within;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
@@ -35,14 +36,23 @@
 import picocli.CommandLine;
 
 /**
- * Verifies that {@code --reads-per-write} of {@link HadoopFsReadWriteValidator}
- * (dfsrw) controls how many reads the run issues per write. The workload only
- * needs a Hadoop {@code FileSystem}, so these run against the local one instead
- * of a cluster.
+ * Verifies that {@code --read-percent} of {@link HadoopFsReadWriteValidator}
+ * (dfsrw) controls how the operations of a run split between reads and writes.
+ * The workload only needs a Hadoop {@code FileSystem}, so these run against the
+ * local one instead of a cluster.
  */
-public class TestHadoopFsReadWriteRatio {
+public class TestHadoopFsReadPercent {
 
-  private static final int WRITES = 8;
+  /**
+   * Operations per run of the tests that assert a split. Each one draws on its
+   * own, so a split is only exact at 0 and 100 percent; a run of this many
+   * leaves the share of reads far closer to the percentage than the tolerance
+   * of {@link #assertSplit} allows for.
+   */
+  private static final int OPS = 1000;
+
+  /** Operations of the runs that assert an exact count. */
+  private static final int FEW_OPS = 8;
 
   @TempDir
   private java.nio.file.Path tempDir;
@@ -58,77 +68,56 @@ void setUp() {
     CorruptingLocalFileSystem.corruptFromRead(Integer.MAX_VALUE);
   }
 
-  /**
-   * One read per write is the default, and it stays that way when the ratio is
-   * given explicitly.
-   */
+  /** Half of the operations read, unless the run asks for another split. */
   @Test
-  void pairsOneReadWithEveryWriteByDefault() {
-    CommandLine cmd = runValidator(2);
+  void splitsOperationsEvenlyByDefault() {
+    CommandLine cmd = runValidator(1, OPS);
 
-    assertEquals(WRITES, writeCount(cmd));
-    assertEquals(WRITES, readCount(cmd));
+    assertSplit(cmd, 50);
   }
 
   /**
-   * A whole number of reads per write holds however the writes are spread over
-   * the threads, so this can afford more than one of them.
+   * The requested share of the operations reads, whether that leaves the run
+   * write-heavy or read-heavy.
    */
   @ParameterizedTest
-  @ValueSource(ints = {2, 3, 5})
-  void issuesRequestedReadsPerWrite(int ratio) {
+  @ValueSource(ints = {10, 75, 90})
+  void splitsOperationsByPercent(int percent) {
     CommandLine cmd =
-        runValidator(2, "--reads-per-write", String.valueOf(ratio));
+        runValidator(1, OPS, "--read-percent", String.valueOf(percent));
 
-    assertEquals(WRITES, writeCount(cmd));
-    assertEquals((long) WRITES * ratio, readCount(cmd));
+    assertSplit(cmd, percent);
   }
 
-  /**
-   * A ratio below 1 reads back only every n-th write, and the fraction it
-   * leaves over is carried between writes rather than rounded away on each of
-   * them, which would read back nothing at all. The leftover is per thread, so
-   * only a single thread gives an exact count.
-   */
+  /** Nothing is read back at 0, which leaves a pure write load. */
   @Test
-  void spreadsFractionalRatioOverWrites() {
-    CommandLine cmd = runValidator(1, "--reads-per-write", "0.25");
+  void writesEveryOperationAtZeroPercent() {
+    CommandLine cmd = runValidator(1, FEW_OPS, "--read-percent", "0");
 
-    assertEquals(WRITES, writeCount(cmd));
-    assertEquals(WRITES / 4, readCount(cmd));
-  }
-
-  /** A ratio with a whole and a fractional part mixes the two behaviours. */
-  @Test
-  void spreadsMixedRatioOverWrites() {
-    CommandLine cmd = runValidator(1, "--reads-per-write", "1.5");
-
-    assertEquals(WRITES, writeCount(cmd));
-    assertEquals(WRITES * 3 / 2, readCount(cmd));
+    assertEquals(FEW_OPS, writeCount(cmd));
+    assertEquals(0, readCount(cmd));
   }
 
   /**
-   * The leftover of a fractional ratio is carried exactly, so the tenth write
-   * of a run at 0.1 is read back. A ratio accumulated as a double drifts —
-   * adding 0.1 ten times gives 0.9999999999999999 — which would put that read
-   * off to an eleventh write a ten-write run never makes.
+   * Even at 100 a thread writes once: a read validates a file the thread wrote,
+   * and its first operation has nothing to read back yet.
    */
   @Test
-  void carriesFractionalRatioWithoutDrift() {
-    CommandLine cmd = runValidator(1, 10, "--reads-per-write", "0.1");
+  void readsEveryOperationButTheFirstAtFullPercent() {
+    CommandLine cmd = runValidator(1, FEW_OPS, "--read-percent", "100");
 
-    assertEquals(10, writeCount(cmd));
-    assertEquals(1, readCount(cmd));
+    assertEquals(1, writeCount(cmd));
+    assertEquals(FEW_OPS - 1, readCount(cmd));
   }
 
   /**
-   * Every read of a task validates, not only the first or the last one: a
-   * single write read back three times fails the run wherever among the three
-   * the content is corrupted.
+   * Every read validates, not only the first or the last one: a run whose
+   * single write is read back three times fails wherever among the three the
+   * content is corrupted.
    */
   @ParameterizedTest
   @ValueSource(ints = {1, 2, 3})
-  void everyReadOfATaskValidatesContent(int corruptedRead) {
+  void everyReadValidatesContent(int corruptedRead) {
     CorruptingLocalFileSystem.corruptFromRead(corruptedRead);
 
     int exitCode = new Freon().getCmd().execute(
@@ -136,43 +125,56 @@ void everyReadOfATaskValidatesContent(int corruptedRead) {
         "dfsrw",
         "-r", rootPath,
         "-p", "dfsrw-corrupt",
-        "-n", "1",
+        // the first operation writes, so the other three all read that file
+        "-n", "4",
         "-t", "1",
         "-s", "1KB",
         "--buffer", "1024",
         "--copy-buffer", "1024",
-        "--reads-per-write", "3");
+        "--read-percent", "100");
 
     assertNotEquals(0, exitCode,
         "Corrupted content of read " + corruptedRead + " was not detected");
   }
 
   @Test
-  void rejectsNonPositiveRatio() {
-    assertThat(execute(new Freon().getCmd(), 1, WRITES, "--reads-per-write", "0"))
+  void rejectsPercentOutsideRange() {
+    assertThat(execute(new Freon().getCmd(), 1, FEW_OPS, "--read-percent", "-1"))
         .isNotZero();
-    assertThat(execute(new Freon().getCmd(), 1, WRITES, "--reads-per-write", "-1"))
+    assertThat(execute(new Freon().getCmd(), 1, FEW_OPS, "--read-percent", "101"))
         .isNotZero();
   }
 
-  private CommandLine runValidator(int threads, String... args) {
-    return runValidator(threads, WRITES, args);
+  /**
+   * Every operation was a read or a write, and the reads are close enough to
+   * the requested share of them. The tolerance is six standard deviations of
+   * the draw: wide enough that a passing run is not chance, narrow enough to
+   * catch a split that ignores the percentage.
+   */
+  private static void assertSplit(CommandLine cmd, int percent) {
+    long reads = readCount(cmd);
+    assertEquals(OPS, reads + writeCount(cmd),
+        "every operation is either a read or a write");
+
+    double fraction = percent / 100.0;
+    long tolerance =
+        (long) Math.ceil(6 * Math.sqrt(OPS * fraction * (1 - fraction)));
+    assertThat(reads).isCloseTo(Math.round(OPS * fraction), within(tolerance));
   }
 
-  private CommandLine runValidator(int threads, int writes, String... args) {
+  private CommandLine runValidator(int threads, int ops, String... args) {
     CommandLine cmd = new Freon().getCmd();
-    assertEquals(0, execute(cmd, threads, writes, args),
+    assertEquals(0, execute(cmd, threads, ops, args),
         "Freon dfsrw command failed");
     return cmd;
   }
 
-  private int execute(CommandLine cmd, int threads, int writes,
-      String... args) {
+  private int execute(CommandLine cmd, int threads, int ops, String... args) {
     String[] fixed = {
         "dfsrw",
         "-r", rootPath,
         "-p", "dfsrw",
-        "-n", String.valueOf(writes),
+        "-n", String.valueOf(ops),
         "-t", String.valueOf(threads),
         "-s", "1KB",
         "--buffer", "1024",

From 3cb4f21eb14ad3dc0190db213ec340c6f9654cf9 Mon Sep 17 00:00:00 2001
From: ermahesh <mahi.ramuk@gmail.com>
Date: Mon, 14 Sep 2026 16:20:18 -0500
Subject: [PATCH 4/4] HDDS-16354. Fix dfsrw integration test failures from the
 read-percent change

The file a write goes to is named after the task counter again, as it was
before --read-percent. Deriving it from the thread's own write sequence
broke the path recycling the framework provides: taskLoop passes
counter % testNo, so a time-based run cycles through -n ids and writes
over its paths, while the write sequence only wraps after
--max-files-per-thread. testValidateOverwrittenPaths saw 317 paths where
it expects at most 8, and the tool stopped exercising overwritten reads
in --duration runs, which is the point of that mode.

The memory bound the write sequence was meant to protect holds either
way, since the id is taken modulo --max-files-per-thread.

testWriteReadValidate expected -n files. -n now counts operations of both
kinds and the split is drawn, so the file count is a range rather than a
number: at least one file, at most one per operation, all with distinct
content. The task count itself is still exact.

testPathsWrapAtMaxFilesPerThread now runs at --read-percent 0. It asserts
an exact file count, and with reads drawn in, whether every path of the
cycle got written at all would have been left to chance.
---
 .../freon/HadoopFsReadWriteValidator.java     | 33 ++++++++-----------
 .../freon/TestHadoopFsReadWriteValidator.java | 20 ++++++++---
 2 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
index 6a2310063ed7..4f393bc29fe2 100644
--- a/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
+++ b/hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/HadoopFsReadWriteValidator.java
@@ -158,15 +158,13 @@ public Void call() throws Exception {
   }
 
   /**
-   * One operation of the run, a read or a write. The counter of the task is not
-   * what the written file is named after: with the two kinds of operation drawn
-   * at random it no longer counts the writes of the thread, which is what has
-   * to stay within --max-files-per-thread.
+   * One operation of the run, a read or a write, drawn per --read-percent. A
+   * thread that has written nothing yet has nothing to read back and writes.
    */
   private void readOrWrite(long counter) throws Exception {
     ThreadHistory history = threadHistory.get();
     if (history.isEmpty() || !readsNext()) {
-      writeAndRecord(history);
+      writeAndRecord(history, counter);
     } else {
       validateRandomFile(history);
     }
@@ -177,9 +175,10 @@ private boolean readsNext() {
     return ThreadLocalRandom.current().nextDouble(100) < readPercent;
   }
 
-  private void writeAndRecord(ThreadHistory history) throws Exception {
+  private void writeAndRecord(ThreadHistory history, long counter)
+      throws Exception {
     long marker = history.nextMarker();
-    long fileId = fileIdOf(marker);
+    long fileId = counter % maxFilesPerThread;
     Path file = objectPath(fileId);
 
     long checksum;
@@ -195,15 +194,6 @@ private void writeAndRecord(ThreadHistory history) throws Exception {
     history.record(fileId, checksum);
   }
 
-  /**
-   * File a write goes to. The low half of its marker is the write sequence of
-   * the thread, so ids cycle within --max-files-per-thread and a thread keeps
-   * at most that many checksums however its reads and writes fall.
-   */
-  private long fileIdOf(long marker) {
-    return (marker & 0xFFFFFFFFL) % maxFilesPerThread;
-  }
-
   /**
    * Read back one of the files this thread wrote, picked at random, and verify
    * that its content still matches the checksum of the latest write of that
@@ -223,10 +213,13 @@ private void validateRandomFile(ThreadHistory history) throws Exception {
   }
 
   /**
-   * Path of the file for the given counter. The thread sequence id is part of
-   * the path so each worker owns a private namespace; paths are reused once a
-   * thread has written --max-files-per-thread of them, and this keeps one
-   * thread from overwriting a file another thread is reading back.
+   * Path of the file for the given id. The thread sequence id is part of the
+   * path so each worker owns a private namespace, which keeps one thread from
+   * overwriting a file another thread is reading back. The id comes from the
+   * task counter, which the framework recycles within -n, so a time-based run
+   * writes over its paths rather than growing without bound; the modulo caps a
+   * count-based run at --max-files-per-thread distinct ids, and with it the
+   * checksums a thread has to remember.
    */
   private Path objectPath(long fileId) {
     return new Path(getRootPath() + "/" + generateObjectName(fileId)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteValidator.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteValidator.java
index 9bff0f416f27..236516170671 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteValidator.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestHadoopFsReadWriteValidator.java
@@ -85,7 +85,8 @@ public void testWriteReadValidate(BucketLayout layout) throws Exception {
 
     String rootPath = OZONE_URI_SCHEME + "://" + bucketName + "." + volumeName;
     String om = cluster().getConf().get(OZONE_OM_ADDRESS_KEY);
-    int exitCode = new Freon().getCmd().execute(
+    CommandLine cmd = new Freon().getCmd();
+    int exitCode = cmd.execute(
         "-D", OZONE_OM_ADDRESS_KEY + "=" + om,
         "dfsrw",
         "-n", String.valueOf(fileCount),
@@ -96,12 +97,20 @@ public void testWriteReadValidate(BucketLayout layout) throws Exception {
     );
     assertEquals(0, exitCode, "Freon dfsrw command failed");
 
+    // -n counts operations, and --read-percent decides per operation whether it
+    // writes or reads one back, so how many of them wrote is drawn rather than
+    // fixed.  Each write takes a path of its own here, so the run leaves
+    // somewhere between one file and one per operation.
+    BaseFreonGenerator subject = (BaseFreonGenerator)
+        cmd.getParseResult().subcommand().commandSpec().userObject();
+    assertEquals(fileCount, subject.getSuccessCount());
+
     // verify all files were written with the requested size
     OzoneConfiguration conf = new OzoneConfiguration(cluster().getConf());
     try (FileSystem fileSystem = FileSystem.get(URI.create(rootPath), conf)) {
       FileStatus[] files =
           fileSystem.listStatus(new Path(rootPath + "/" + prefix));
-      assertEquals(fileCount, files.length, "Unexpected number of files");
+      assertThat(files.length).isBetween(1, fileCount);
       Set<Long> checksums = new HashSet<>();
       for (FileStatus file : files) {
         assertEquals(fileSize, file.getLen(),
@@ -110,7 +119,7 @@ public void testWriteReadValidate(BucketLayout layout) throws Exception {
       }
       // distinct content across threads, otherwise reading the wrong file would
       // still validate
-      assertEquals(fileCount, checksums.size(), "Files share their content");
+      assertEquals(files.length, checksums.size(), "Files share their content");
     }
   }
 
@@ -128,7 +137,9 @@ private static long checksumOf(FileSystem fileSystem, Path file)
   /**
    * Once a thread has written --max-files-per-thread files its paths wrap, so
    * the run keeps writing without leaving files it has no checksum for.  The
-   * wrap is layout independent, so one layout covers it.
+   * wrap is layout independent, so one layout covers it.  It is a pure write
+   * run: with reads drawn in, whether every path of the cycle got written at
+   * all would be a matter of chance, and the count below could not be exact.
    */
   @Test
   public void testPathsWrapAtMaxFilesPerThread() throws Exception {
@@ -154,6 +165,7 @@ public void testPathsWrapAtMaxFilesPerThread() throws Exception {
         "-t", "1",
         "-s", fileSize + "B",
         "--max-files-per-thread", String.valueOf(maxFilesPerThread),
+        "--read-percent", "0",
         "-p", prefix,
         "-r", rootPath
     );