HDDS-13954. Add localLease for followerRead by symious · Pull Request #9320 · apache/ozone

symious · 2025-11-18T09:37:54Z

What changes were proposed in this pull request?

This ticket is to add a fast path check for follower read.

In most cases, we don't need so strong consistency.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13954

How was this patch tested?

Test with Freon FollowerRead.

szetszwo

@symious , thanks for working on this! Please see the comment inlined.

szetszwo · 2025-11-19T18:16:46Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java

+  public static final String OZONE_OM_FOLLOWER_READ_LOCAL_LEASE_LAG_LIMIT =
+      "ozone.om.follower.read.local.lease.lag.limit";
+  public static final int OZONE_OM_FOLLOWER_READ_LOCAL_LEASE_LAG_LIMIT_DEFAULT = 30000;
+
+  public static final String OZONE_OM_FOLLOWER_READ_LOCAL_LEASE_TIME_MS =
+      "ozone.om.follower.read.local.lease.time.ms";
+  public static final long OZONE_OM_FOLLOWER_READ_LOCAL_LEASE_TIME_MS_DEFAULT = 15000;


The defaults seem too big. How about setting the lag to 1000 and the time to 3000ms (the same as OZONE_OM_RATIS_SERVER_REQUEST_TIMEOUT_DEFAULT)?

BTW, please move them to OMConfigKeys.

@szetszwo Thank you for the review.

Decrease the values to 10000 and 5000.
Log lag is kind of inflated during writing, better have a large buffer; time is set to the OM's ratis RPC timeout.

szetszwo

+1 the change looks good.

ivandika3

Thanks for the patch. LGTM +1.

peterxcli

Thanks @symious for this improvement — hoping it can push Ozone’s read performance to the next level.

I left a few comments. Beyond those, could we add at least two tests:

Verify the read state machine routes through the follower’s local path when the option is enabled.
With a very small commit-index gap timeout, verify the staleness detector triggers as expected.

...main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java

peterxcli · 2025-11-21T07:49:50Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java

  public static final String OZONE_OM_GRPC_PORT_KEY =
      "ozone.om.grpc.port";

+  public static final String OZONE_OM_FOLLOWER_READ_LOCAL_LEASE_ENABLED_KEY =


I left a comment in #9222 (comment). Could we group the read options so users and developers can understand them more easily?
We already have a strictness order in OzoneManagerProtocolServerSideTranslatorPB, which is part of this change:

if (stale read available) { ... } else if (follower linearizable read) { ... } else /* leader-only read */ { ... }

How about adding the follower stale read to this set of read options as well?

This change looks good, I'll update it then.

I think currently it's already the logic?

if (local lease read) else if (linearizable read) end if normal check leader and read from leader

Thanks—yes, I was referring to including follower stale reads within the read options. Your comment (#9222 (comment)) reminded me we should also cover fallback behavior. I’m thinking we introduce a clearer, self-contained set of read modes so users can configure intent without scattered flags:

leader-only-read

follower-linearizable-read

follower-stale-read-or-fallback-follower-linearizable-read

Prefer follower stale read; if not possible, fall back to follower linearizable read

follower-stale-read-or-fallback-leader-only-read

Prefer follower stale read; if not possible, fall back to leader-only read

These modes make the configuration atomic and avoid scattering multiple booleans. We should also add comprehensive documentation for each mode—consistency guarantees, when fallback triggers, and expected performance implications. The trade-off is that we’ll need a small translation layer to map these flexible modes to Ratis server settings and to the branching in OzoneManagerProtocolServerSideTranslatorPB. I’m fine to own that conversion logic so the config remains clean and self-contained.

I think these flags should make users quite confused.

leader-only-read

follower-linearizable-read

follower-stale-read + fallback option(leader or linear-read)

The above seems more fluent.

Sounds good.

@peterxcli For the configuration, I decided to keep the original check to isolate the locallease and linearizable part.

Linearizable read is set by OMs, but locallease configs can be set from OM and also client side for future usage.

adoroszlai

Thanks @symious for the patch.

In addition to the comment about new configurations, I'd like to understand the state of this PR: it is approved, but there is some discussion afterwards about further changes (e.g. two test cases). Are those required or not?

If this is already good to go, I can address the comment about configuration in a follow-up.

adoroszlai · 2025-11-25T12:49:25Z

hadoop-hdds/common/src/main/resources/ozone-default.xml

+  <property>
+    <name>ozone.om.follower.read.local.lease.enabled</name>
+    <value>false</value>
+    <tag>OZONE, OM, RATIS, MANAGEMENT</tag>
+    <description> If we enabled the local lease for Follower Read.
+      If enabled, follower OM will decide if return local data directly
+      based on lag log and time.
+    </description>
+  </property>
+  <property>
+    <name>ozone.om.follower.read.local.lease.lag.limit</name>
+    <value>10000</value>
+    <tag>OZONE, OM, RATIS, MANAGEMENT</tag>
+    <description> If the lag between leader OM and follower OM is larger
+      than this number, the follower OM is not up-to-date.
+    </description>
+  </property>
+  <property>
+    <name>ozone.om.follower.read.local.lease.time.ms</name>
+    <value>5000</value>
+    <tag>OZONE, OM, RATIS, MANAGEMENT</tag>
+    <description> If the lag time Ms between leader OM and follower OM is larger
+      than this number, the follower OM is not up-to-date.
+      By default, it's set to Ratis RPC timeout value.
+    </description>
+  </property>


I would be nice to introduce these new settings in OmConfig (see HDDS-12298), to avoid further bloating OzoneManager. Most importantly it would eliminate the boilerplate required for making the new properties reconfigurable to:

ozone/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java

Line 70 in 7c53356

reconfigurable = true,

adoroszlai

Thanks @symious for updating the patch. Two minor items noted in the new test. Can be addressed in follow-up (in itself or as part of other changes) if there are no comments from others.

@peterxcli would you like to take another look?

adoroszlai · 2025-12-05T17:24:42Z

...ation-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHAWithFollowerRead.java

+    Assertions.assertTrue(getCluster().getOzoneManager(1).getMetrics().getNumFollowerReadLocalLeaseSuccess() > 0);
+    Assertions.assertEquals(0, getCluster().getOzoneManager(2).getMetrics().getNumFollowerReadLocalLeaseSuccess());
+    Assertions.assertTrue(getCluster().getOzoneManager(2).getMetrics().getNumFollowerReadLocalLeaseFailTime() > 0);


nit, can be fixed in follow-up

please use assertThat(...).isPositive() instead of assertTrue(... > 0)

please add import static for assert...

adoroszlai · 2025-12-05T17:25:32Z

...ation-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHAWithFollowerRead.java

+    getCluster().getOzoneManager(1).setConfiguration(oldConf);
+    getCluster().getOzoneManager(2).setConfiguration(oldConf);


nit, can be fixed in follow-up

Restore original state in finally block.

adoroszlai

Thanks @symious for updating the patch. There is a checkstyle failure.

adoroszlai · 2025-12-08T09:43:50Z

...ation-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHAWithFollowerRead.java

+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+


Please move these before regular imports.

Also, please import org.assertj.core.api.Assertions.assertThat instead.

Updated, PTAL.

adoroszlai · 2025-12-08T12:26:53Z

Thanks @symious for the patch, @ivandika3, @peterxcli, @szetszwo for the review.

Yiyang Zhou added 2 commits November 18, 2025 17:36

HDDS-13954. Add localLease for followerRead

5a435f4

HDDS-13954. Fix checkstyle

bf8a553

ivandika3 requested a review from szetszwo November 19, 2025 01:09

szetszwo reviewed Nov 19, 2025

View reviewed changes

symious added 2 commits November 20, 2025 14:20

HDDS-13954. Decrease config value

b13de48

HDDS-13954. Fix checkstyle

6b15655

szetszwo approved these changes Nov 20, 2025

View reviewed changes

ivandika3 approved these changes Nov 21, 2025

View reviewed changes

ivandika3 requested review from peterxcli and whbing November 21, 2025 04:09

peterxcli mentioned this pull request Nov 21, 2025

HDDS-9279. Basic implementation of OM Follower read #9222

Merged

peterxcli reviewed Nov 21, 2025

View reviewed changes

adoroszlai reviewed Nov 25, 2025

View reviewed changes

symious and others added 5 commits December 4, 2025 17:06

Merge branch 'master' into HDDS-13954

777e2c7

HDDS-13954. Move the configs

7ab0229

HDDS-13954. Add metrics

0301368

remove duplicate configs

f011105

HDDS-13954. Add test case

5aa5f22

adoroszlai reviewed Dec 5, 2025

View reviewed changes

adoroszlai and others added 2 commits December 7, 2025 18:18

Merge remote-tracking branch 'origin/master' into HDDS-13954

e8217e1

HDDS-13954. Fix as comments

7f42886

adoroszlai reviewed Dec 8, 2025

View reviewed changes

symious added 2 commits December 8, 2025 17:46

HDDS-13954. fix checkstyle

67e5c98

HDDS-13954. Fix checkstyle

7ef052b

adoroszlai merged commit d1aecfd into apache:master Dec 8, 2025
43 checks passed

		getCluster().getOzoneManager(1).setConfiguration(oldConf);
		getCluster().getOzoneManager(2).setConfiguration(oldConf);

		import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
		import static org.junit.jupiter.api.Assertions.assertEquals;

Conversation

symious commented Nov 18, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

ivandika3 left a comment

Choose a reason for hiding this comment

Uh oh!

peterxcli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adoroszlai commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants