Skip to content

Commit 29e044c

Browse files
ivandika3ivandika3
committed
Fix test, refine local lease, rename variables
1 parent b8dabb2 commit 29e044c

File tree

6 files changed

+48
-25
lines changed

6 files changed

+48
-25
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public class OmConfig extends ReconfigurableConfig {
169169
description = "If the log lag between leader OM and follower OM is larger " +
170170
"than this number, the follower OM is not up-to-date."
171171
)
172-
private long followerReadLocalLeaseLagLimit;
172+
private long followerReadLocalLeaseLogLimit;
173173

174174
@Config(key = "ozone.om.follower.read.local.lease.time.ms",
175175
defaultValue = "5000",
@@ -230,12 +230,12 @@ public void setFollowerReadLocalLeaseEnabled(boolean newValue) {
230230
this.followerReadLocalLeaseEnabled = newValue;
231231
}
232232

233-
public long getFollowerReadLocalLeaseLagLimit() {
234-
return followerReadLocalLeaseLagLimit;
233+
public long getFollowerReadLocalLeaseLogLimit() {
234+
return followerReadLocalLeaseLogLimit;
235235
}
236236

237-
public void setFollowerReadLocalLeaseLagLimit(long newValue) {
238-
this.followerReadLocalLeaseLagLimit = newValue;
237+
public void setFollowerReadLocalLeaseLogLimit(long newValue) {
238+
this.followerReadLocalLeaseLogLimit = newValue;
239239
}
240240

241241
public long getFollowerReadLocalLeaseTimeMs() {

‎hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/HadoopRpcOMFollowerReadFailoverProxyProvider.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ public class HadoopRpcOMFollowerReadFailoverProxyProvider implements FailoverPro
8787
/** The last proxy that has been used. Only used for testing. */
8888
private volatile OMProxyInfo<OzoneManagerProtocolPB> lastProxy = null;
8989

90+
/** The read consistency hint used when follower read is enabled. */
9091
private final ReadConsistencyHint followerReadConsistency;
92+
/** The read consistency hint used when follower read is disabled or when follower read fails. */
9193
private final ReadConsistencyHint leaderReadConsistency;
9294

9395
public HadoopRpcOMFollowerReadFailoverProxyProvider(
@@ -103,7 +105,7 @@ public HadoopRpcOMFollowerReadFailoverProxyProvider(
103105
Preconditions.assertTrue(followerReadConsistencyType.allowFollowerRead(),
104106
"Invalid follower read consistency " + followerReadConsistencyType);
105107
Preconditions.assertTrue(!leaderReadConsistencyType.allowFollowerRead(),
106-
"Invalid leader read consistency" + leaderReadConsistencyType);
108+
"Invalid leader read consistency " + leaderReadConsistencyType);
107109
this.failoverProxy = failoverProxy;
108110
// Create a wrapped proxy containing all the proxies. Since this combined
109111
// proxy is just redirecting to other proxies, all invocations can share it.

‎hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAFollowerReadWithAllRunning.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ void testClientWithFollowerReadDisabled() throws Exception {
514514
void testClientWithLinearizableLeaderRead() throws Exception {
515515
OzoneConfiguration clientConf = new OzoneConfiguration(getConf());
516516
clientConf.setBoolean(OZONE_CLIENT_FOLLOWER_READ_ENABLED_KEY, false);
517-
clientConf.set(OZONE_CLIENT_LEADER_READ_DEFAULT_CONSISTENCY_KEY, "LINEARIZABLE");
517+
clientConf.set(OZONE_CLIENT_LEADER_READ_DEFAULT_CONSISTENCY_KEY, "LINEARIZABLE_LEADER_ONLY");
518518

519519
OzoneClient ozoneClient = null;
520520
try {

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

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
2121
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertNotNull;
2223

2324
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
2425
import org.apache.hadoop.ozone.OzoneConfigKeys;
26+
import org.apache.hadoop.ozone.om.OzoneManager;
2527
import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServerConfig;
2628
import org.apache.ratis.server.RaftServerConfigKeys;
2729
import org.junit.jupiter.api.Assertions;
@@ -81,25 +83,44 @@ public void testAllowFollowerReadLocalLease() throws Exception {
8183
OzoneConfiguration newConf2 = new OzoneConfiguration(newConf1);
8284
// All local lease should fail since the lease time is negative
8385
newConf2.setLong("ozone.om.follower.read.local.lease.time.ms", -1000);
84-
86+
OzoneManager omFollower1 = null;
87+
OzoneManager omFollower2 = null;
8588
try {
86-
getCluster().getOzoneManager(1).setConfiguration(newConf1);
87-
getCluster().getOzoneManager(2).setConfiguration(newConf2);
89+
for (OzoneManager om : getCluster().getOzoneManagersList()) {
90+
// Leader ignores the local lease and serve the read request
91+
// immediately so we should test on followers instead
92+
if (!om.isLeaderReady()) {
93+
if (omFollower1 == null) {
94+
omFollower1 = om;
95+
} else {
96+
omFollower2 = om;
97+
break;
98+
}
99+
}
100+
}
101+
assertNotNull(omFollower1, "Cannot find OM follower");
102+
assertNotNull(omFollower2, "Cannot find OM follower");
103+
omFollower1.setConfiguration(newConf1);
104+
omFollower2.setConfiguration(newConf2);
88105

89106
String[] args = new String[]{"volume", "list"};
90107
OzoneShell ozoneShell = new OzoneShell();
91108
ozoneShell.getOzoneConf().setBoolean("ozone.client.follower.read.enabled", true);
92-
ozoneShell.getOzoneConf().set("ozone.client.follower.read.default.consistency.type", "LOCAL_LEASE");
109+
ozoneShell.getOzoneConf().set("ozone.client.follower.read.default.consistency", "LOCAL_LEASE");
93110
for (int i = 0; i < 100; i++) {
94111
execute(ozoneShell, args);
95112
}
96-
assertThat(getCluster().getOzoneManager(1).getMetrics().getNumFollowerReadLocalLeaseSuccess() > 0).isTrue();
113+
assertThat(omFollower1.getMetrics().getNumFollowerReadLocalLeaseSuccess() > 0).isTrue();
97114
// Local lease time is set to negative, for this OM should fail all local lease read requests
98-
assertEquals(0, getCluster().getOzoneManager(2).getMetrics().getNumFollowerReadLocalLeaseSuccess());
99-
assertThat(getCluster().getOzoneManager(2).getMetrics().getNumFollowerReadLocalLeaseFailTime() > 0).isTrue();
115+
assertEquals(0, omFollower2.getMetrics().getNumFollowerReadLocalLeaseSuccess());
116+
assertThat(omFollower2.getMetrics().getNumFollowerReadLocalLeaseFailTime() > 0).isTrue();
100117
} finally {
101-
getCluster().getOzoneManager(1).setConfiguration(oldConf);
102-
getCluster().getOzoneManager(2).setConfiguration(oldConf);
118+
if (omFollower1 != null) {
119+
omFollower1.setConfiguration(oldConf);
120+
}
121+
if (omFollower2 != null) {
122+
omFollower2.setConfiguration(oldConf);
123+
}
103124
}
104125
}
105126
}

‎hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2408,7 +2408,7 @@ enum ReadConsistencyProto {
24082408
// or whether it is superseded by the OM server-side configuration.
24092409
message ReadConsistencyHint {
24102410
message LocalLeaseContext {
2411-
optional uint64 lagLimit = 1;
2411+
optional uint64 logLimit = 1;
24122412
optional uint64 leaseTimeMs = 2;
24132413
}
24142414

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,10 @@ private OMResponse submitReadRequestToOM(OMRequest request)
238238
// whether OM Raft server enables linearizable read).
239239
ReadConsistencyHint readConsistencyHint = request.getReadConsistencyHint();
240240
ReadConsistencyProto readConsistency = readConsistencyHint.getReadConsistency();
241-
RaftServerStatus raftServerStatus;
242241
switch (readConsistency) {
243242
case LOCAL_LEASE:
244-
return submitReadRequestToOmLocalLease(request, readConsistencyHint.getLocalLeaseContext());
243+
return submitReadRequestToOmLocalLease(request,
244+
readConsistencyHint.hasLocalLeaseContext() ? readConsistencyHint.getLocalLeaseContext() : null);
245245
case LINEARIZABLE_LEADER_ONLY:
246246
return submitReadRequestToOmLinearizableLeaderOnly(request);
247247
case LINEARIZABLE_ALLOW_FOLLOWER:
@@ -257,7 +257,7 @@ private OMResponse submitReadRequestToOmWithoutHint(OMRequest request) throws Se
257257
// Read from leader or followers using linearizable read
258258
if (ozoneManager.getConfig().isFollowerReadLocalLeaseEnabled() &&
259259
allowFollowerReadLocalLease(omRatisServer.getServerDivision(),
260-
ozoneManager.getConfig().getFollowerReadLocalLeaseLagLimit(),
260+
ozoneManager.getConfig().getFollowerReadLocalLeaseLogLimit(),
261261
ozoneManager.getConfig().getFollowerReadLocalLeaseTimeMs())) {
262262
ozoneManager.getMetrics().incNumFollowerReadLocalLeaseSuccess();
263263
return handler.handleReadRequest(request);
@@ -298,12 +298,12 @@ private OMResponse submitReadRequestToOmLocalLease(OMRequest request, LocalLease
298298
if (!ozoneManager.getConfig().isFollowerReadLocalLeaseEnabled()) {
299299
throw createLeaderErrorException(raftServerStatus);
300300
}
301-
long localLeaseLagLimit = localLeaseContext.hasLagLimit() ?
302-
localLeaseContext.getLagLimit() : ozoneManager.getConfig().getFollowerReadLocalLeaseLagLimit();
303-
long localLeaseLeaseTimeMs = localLeaseContext.hasLeaseTimeMs() ?
301+
long localLeaseLogLimit = localLeaseContext != null && localLeaseContext.hasLogLimit() ?
302+
localLeaseContext.getLogLimit() : ozoneManager.getConfig().getFollowerReadLocalLeaseLogLimit();
303+
long localLeaseLeaseTimeMs = localLeaseContext != null && localLeaseContext.hasLeaseTimeMs() ?
304304
localLeaseContext.getLeaseTimeMs() : ozoneManager.getConfig().getFollowerReadLocalLeaseTimeMs();
305305
if (allowFollowerReadLocalLease(omRatisServer.getServerDivision(),
306-
localLeaseLagLimit, localLeaseLeaseTimeMs)) {
306+
localLeaseLogLimit, localLeaseLeaseTimeMs)) {
307307
ozoneManager.getMetrics().incNumFollowerReadLocalLeaseSuccess();
308308
return handler.handleReadRequest(request);
309309
}
@@ -393,7 +393,7 @@ boolean allowFollowerReadLocalLease(Division ratisDivision, long leaseLogLimit,
393393
return false; // no leader
394394
}
395395

396-
if (leaseTimeMsLimit >= 0 && leaderInfo.getLastRpcElapsedTimeMs() > leaseTimeMsLimit) {
396+
if (leaseTimeMsLimit != -1 && leaderInfo.getLastRpcElapsedTimeMs() > leaseTimeMsLimit) {
397397
LOG.debug("FollowerRead Local Lease not allowed: Local lease Time expired. ");
398398
ozoneManager.getMetrics().incNumFollowerReadLocalLeaseFailTime();
399399
return false; // lease time expired

0 commit comments

Comments
 (0)