You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2021/06/23 10:42:07 UTC
[GitHub] [ratis] sadanand48 opened a new pull request #483: RATIS-1384.Change pending request limit unit to MB
sadanand48 opened a new pull request #483:
URL: https://github.com/apache/ratis/pull/483
## What changes were proposed in this pull request?
Currently the pendingRequestLimit is in Bytes and this byteLimit is converted to int
```
RequestLimits(int elementLimit, SizeInBytes byteLimit) {
super(elementLimit, byteLimit.getSizeInt());
}
```
This means the maximum bytes that can be set is the INTEGER limit i.e the pendingRequestSize cannot be set beyond 2147483647 bytes (2GB) .
The change here is to change the unit to Megabytes instead of bytes.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1384
## How was this patch tested?
Unit test
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664306062
##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/Message.java
##########
@@ -65,6 +68,10 @@ static int getSize(Message message) {
ByteString getContent();
default int size() {
- return Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ int size = Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ if (size > 0 && size < (double) (ONE_MB.getSizeInt())) {
+ return 1;
+ }
+ return size/ SizeInBytes.valueOf("1mb").getSizeInt();
Review comment:
Reverted this change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r667664781
##########
File path: ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java
##########
@@ -205,12 +205,12 @@ void runTestLeaderRestart(MiniRaftClusterWithGrpc cluster) throws Exception {
public void testRaftClientMetrics() throws Exception {
runWithNewCluster(3, this::testRaftClientRequestMetrics);
}
-
+
Review comment:
Please revert this white space change.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
##########
@@ -82,19 +96,24 @@ void release(Message message) {
private final Map<Permit, Permit> permits = new HashMap<>();
/** Track and limit the number of requests and the total message size. */
private final RequestLimits resource;
+ /** The size (in byte) of all the requests in this map. */
+ private final AtomicLong requestSize = new AtomicLong();
+
- RequestMap(Object name, int elementLimit, SizeInBytes byteLimit, RaftServerMetricsImpl raftServerMetrics) {
+ RequestMap(Object name, int elementLimit, int megabyteLimit, RaftServerMetricsImpl raftServerMetrics) {
this.name = name;
- this.resource = new RequestLimits(elementLimit, byteLimit);
+ this.resource = new RequestLimits(elementLimit, megabyteLimit);
this.raftServerMetrics = raftServerMetrics;
raftServerMetrics.addNumPendingRequestsGauge(resource::getElementCount);
- raftServerMetrics.addNumPendingRequestsByteSize(resource::getByteSize);
+ raftServerMetrics.addNumPendingRequestsMegaByteSize(resource::getMegaByteSize);
}
Permit tryAcquire(Message message) {
- final ResourceSemaphore.ResourceAcquireStatus acquired = resource.tryAcquire(message);
- LOG.trace("tryAcquire? {}", acquired);
+ final int messageSize = Message.getSize(message);
+ final int messageSizeMb = roundUpMb(messageSize );
+ final ResourceSemaphore.ResourceAcquireStatus acquired = resource.tryAcquire(messageSizeMb);
+ LOG.trace("tryAcquire {} MB? {}", messageSizeMb, acquired);
Review comment:
This is an extra space in the beginning of this line.
##########
File path: ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java
##########
@@ -54,7 +55,6 @@ public static SizeInBytes valueOf(String input) {
return new SizeInBytes(size, input, description);
}
-
Review comment:
Please revert this white space change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-877294312
Thanks a lot @szetszwo for helping me with this patch. I have updated the patch now.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-874529955
Thanks @szetszwo for the review. I have reused the same BYTE_LIMIT_KEY now and converted it to MB.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r667687659
##########
File path: ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java
##########
@@ -54,7 +55,6 @@ public static SizeInBytes valueOf(String input) {
return new SizeInBytes(size, input, description);
}
-
Review comment:
done
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
##########
@@ -82,19 +96,24 @@ void release(Message message) {
private final Map<Permit, Permit> permits = new HashMap<>();
/** Track and limit the number of requests and the total message size. */
private final RequestLimits resource;
+ /** The size (in byte) of all the requests in this map. */
+ private final AtomicLong requestSize = new AtomicLong();
+
- RequestMap(Object name, int elementLimit, SizeInBytes byteLimit, RaftServerMetricsImpl raftServerMetrics) {
+ RequestMap(Object name, int elementLimit, int megabyteLimit, RaftServerMetricsImpl raftServerMetrics) {
this.name = name;
- this.resource = new RequestLimits(elementLimit, byteLimit);
+ this.resource = new RequestLimits(elementLimit, megabyteLimit);
this.raftServerMetrics = raftServerMetrics;
raftServerMetrics.addNumPendingRequestsGauge(resource::getElementCount);
- raftServerMetrics.addNumPendingRequestsByteSize(resource::getByteSize);
+ raftServerMetrics.addNumPendingRequestsMegaByteSize(resource::getMegaByteSize);
}
Permit tryAcquire(Message message) {
- final ResourceSemaphore.ResourceAcquireStatus acquired = resource.tryAcquire(message);
- LOG.trace("tryAcquire? {}", acquired);
+ final int messageSize = Message.getSize(message);
+ final int messageSizeMb = roundUpMb(messageSize );
+ final ResourceSemaphore.ResourceAcquireStatus acquired = resource.tryAcquire(messageSizeMb);
+ LOG.trace("tryAcquire {} MB? {}", messageSizeMb, acquired);
Review comment:
done
##########
File path: ratis-test/src/test/java/org/apache/ratis/grpc/TestRaftServerWithGrpc.java
##########
@@ -205,12 +205,12 @@ void runTestLeaderRestart(MiniRaftClusterWithGrpc cluster) throws Exception {
public void testRaftClientMetrics() throws Exception {
runWithNewCluster(3, this::testRaftClientRequestMetrics);
}
-
+
Review comment:
done
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] bshashikant commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-868190454
@szetszwo , can you please have a look?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #483:
URL: https://github.com/apache/ratis/pull/483
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-874578543
@sadanand48 The math does not work well -- Suppose the limit is 128MB and the message sizes are all 1KB. However, it only allows 128 messages since each 1KB-message acquires 1MB space.
I will think about how to fix this problem.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664306269
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -109,6 +109,18 @@ static SizeInBytes byteLimit(RaftProperties properties) {
static void setByteLimit(RaftProperties properties, SizeInBytes byteLimit) {
setSizeInBytes(properties::set, BYTE_LIMIT_KEY, byteLimit, requireMin(1L));
}
+
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
done.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664255023
##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/Message.java
##########
@@ -65,6 +68,10 @@ static int getSize(Message message) {
ByteString getContent();
default int size() {
- return Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ int size = Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ if (size > 0 && size < (double) (ONE_MB.getSizeInt())) {
+ return 1;
+ }
+ return size/ SizeInBytes.valueOf("1mb").getSizeInt();
Review comment:
Since this method is a in the protocol package. We cannot change it. Otherwise, it is an incompatible change.
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -100,15 +100,17 @@ static void setElementLimit(RaftProperties properties, int limit) {
setInt(properties::setInt, ELEMENT_LIMIT_KEY, limit, requireMin(1));
}
- String BYTE_LIMIT_KEY = PREFIX + ".byte-limit";
- SizeInBytes BYTE_LIMIT_DEFAULT = SizeInBytes.valueOf("64MB");
- static SizeInBytes byteLimit(RaftProperties properties) {
- return getSizeInBytes(properties::getSizeInBytes,
- BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog());
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
We cannot rename the key. It is an incompatible change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664255023
##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/Message.java
##########
@@ -65,6 +68,10 @@ static int getSize(Message message) {
ByteString getContent();
default int size() {
- return Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ int size = Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ if (size > 0 && size < (double) (ONE_MB.getSizeInt())) {
+ return 1;
+ }
+ return size/ SizeInBytes.valueOf("1mb").getSizeInt();
Review comment:
Since this method is a in the protocol package. We cannot change it. Otherwise, it is an incompatible change.
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -100,15 +100,17 @@ static void setElementLimit(RaftProperties properties, int limit) {
setInt(properties::setInt, ELEMENT_LIMIT_KEY, limit, requireMin(1));
}
- String BYTE_LIMIT_KEY = PREFIX + ".byte-limit";
- SizeInBytes BYTE_LIMIT_DEFAULT = SizeInBytes.valueOf("64MB");
- static SizeInBytes byteLimit(RaftProperties properties) {
- return getSizeInBytes(properties::getSizeInBytes,
- BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog());
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
We cannot rename the key. It is an incompatible change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664305794
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -100,15 +100,17 @@ static void setElementLimit(RaftProperties properties, int limit) {
setInt(properties::setInt, ELEMENT_LIMIT_KEY, limit, requireMin(1));
}
- String BYTE_LIMIT_KEY = PREFIX + ".byte-limit";
- SizeInBytes BYTE_LIMIT_DEFAULT = SizeInBytes.valueOf("64MB");
- static SizeInBytes byteLimit(RaftProperties properties) {
- return getSizeInBytes(properties::getSizeInBytes,
- BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog());
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
done.
##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/Message.java
##########
@@ -65,6 +68,10 @@ static int getSize(Message message) {
ByteString getContent();
default int size() {
- return Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ int size = Optional.ofNullable(getContent()).map(ByteString::size).orElse(0);
+ if (size > 0 && size < (double) (ONE_MB.getSizeInt())) {
+ return 1;
+ }
+ return size/ SizeInBytes.valueOf("1mb").getSizeInt();
Review comment:
Reverted this change.
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -109,6 +109,18 @@ static SizeInBytes byteLimit(RaftProperties properties) {
static void setByteLimit(RaftProperties properties, SizeInBytes byteLimit) {
setSizeInBytes(properties::set, BYTE_LIMIT_KEY, byteLimit, requireMin(1L));
}
+
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
done.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r664305794
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -100,15 +100,17 @@ static void setElementLimit(RaftProperties properties, int limit) {
setInt(properties::setInt, ELEMENT_LIMIT_KEY, limit, requireMin(1));
}
- String BYTE_LIMIT_KEY = PREFIX + ".byte-limit";
- SizeInBytes BYTE_LIMIT_DEFAULT = SizeInBytes.valueOf("64MB");
- static SizeInBytes byteLimit(RaftProperties properties) {
- return getSizeInBytes(properties::getSizeInBytes,
- BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog());
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
done.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] sadanand48 commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-874529955
Thanks @szetszwo for the review. I have reused the same BYTE_LIMIT_KEY now and converted it to MB.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-874578543
@sadanand48 The math does not work well -- Suppose the limit is 128MB and the message sizes are all 1KB. However, it only allows 128 messages since each 1KB-message acquires 1MB space.
I will think about how to fix this problem.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #483:
URL: https://github.com/apache/ratis/pull/483#discussion_r658471467
##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
##########
@@ -109,6 +109,18 @@ static SizeInBytes byteLimit(RaftProperties properties) {
static void setByteLimit(RaftProperties properties, SizeInBytes byteLimit) {
setSizeInBytes(properties::set, BYTE_LIMIT_KEY, byteLimit, requireMin(1L));
}
+
+ String MEGA_BYTE_LIMIT_KEY = PREFIX + ".megabyte-limit";
Review comment:
Let's reuse BYTE_LIMIT_KEY. Otherwise, it is confusing to have two keys.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #483: RATIS-1384.Change pending request limit unit to MB
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #483:
URL: https://github.com/apache/ratis/pull/483#issuecomment-875539025
@sadanand48 , we may add a requestSize to track the size of all the request. Then, we can release the extra MB afterward. See below for the idea.
```
diff --git a/ratis-common/src/main/java/org/apache/ratis/conf/ConfUtils.java b/ratis-common/src/main/java/org/apache/ratis/conf/ConfUtils.java
index b662dffd..989070f5 100644
--- a/ratis-common/src/main/java/org/apache/ratis/conf/ConfUtils.java
+++ b/ratis-common/src/main/java/org/apache/ratis/conf/ConfUtils.java
@@ -81,6 +81,15 @@ public interface ConfUtils {
};
}
+ static BiConsumer<String, SizeInBytes> requireMinSizeInByte(SizeInBytes min) {
+ return (key, value) -> {
+ if (value.getSize() < min.getSize()) {
+ throw new IllegalArgumentException(
+ key + " = " + value + " < min = " + min);
+ }
+ };
+ }
+
static BiConsumer<String, Long> requireMax(long max) {
return (key, value) -> {
if (value > max) {
diff --git a/ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java b/ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java
index 160cf448..25667a37 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/SizeInBytes.java
@@ -24,6 +24,7 @@ import java.util.Objects;
*/
public final class SizeInBytes {
public static final SizeInBytes ONE_KB = valueOf("1k");
+ public static final SizeInBytes ONE_MB = valueOf("1m");
public static SizeInBytes valueOf(long size) {
final String s = String.valueOf(size);
diff --git a/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java b/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
index 892cc162..586f359a 100644
--- a/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
+++ b/ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java
@@ -114,7 +114,7 @@ public interface RaftServerConfigKeys {
SizeInBytes BYTE_LIMIT_DEFAULT = SizeInBytes.valueOf("64MB");
static SizeInBytes byteLimit(RaftProperties properties) {
return getSizeInBytes(properties::getSizeInBytes,
- BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog());
+ BYTE_LIMIT_KEY, BYTE_LIMIT_DEFAULT, getDefaultLog(), requireMinSizeInByte(SizeInBytes.ONE_MB));
}
static void setByteLimit(RaftProperties properties, SizeInBytes byteLimit) {
setSizeInBytes(properties::set, BYTE_LIMIT_KEY, byteLimit, requireMin(1L));
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
index cda61df2..c01b3393 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/PendingRequests.java
@@ -32,6 +32,7 @@ import org.apache.ratis.statemachine.TransactionContext;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.Preconditions;
import org.apache.ratis.util.ResourceSemaphore;
+import org.apache.ratis.util.ResourceSemaphore.ResourceAcquireStatus;
import org.apache.ratis.util.SizeInBytes;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -44,16 +45,24 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
class PendingRequests {
public static final Logger LOG = LoggerFactory.getLogger(PendingRequests.class);
+ private static final int ONE_MB = SizeInBytes.ONE_MB.getSizeInt();
+
+ /** Round up to the nearest MB. */
+ static int roundUpMb(long bytes) {
+ return Math.toIntExact((bytes - 1) / ONE_MB + 1);
+ }
+
static class Permit {}
static class RequestLimits extends ResourceSemaphore.Group {
- RequestLimits(int elementLimit, SizeInBytes byteLimit) {
- super(elementLimit, byteLimit.getSizeInt());
+ RequestLimits(int elementLimit, int megabyteLimit) {
+ super(elementLimit, megabyteLimit);
}
int getElementCount() {
@@ -64,12 +73,16 @@ class PendingRequests {
return get(1).used();
}
- ResourceSemaphore.ResourceAcquireStatus tryAcquire(Message message) {
- return tryAcquire(1, Message.getSize(message));
+ ResourceAcquireStatus tryAcquire(int messageSizeMb) {
+ return tryAcquire(1, messageSizeMb);
+ }
+
+ void releaseExtraMb(int extraMb) {
+ release(0, extraMb);
}
- void release(Message message) {
- release(1, Message.getSize(message));
+ void release(int diffMb) {
+ release(1, diffMb);
}
}
@@ -82,10 +95,12 @@ class PendingRequests {
private final Map<Permit, Permit> permits = new HashMap<>();
/** Track and limit the number of requests and the total message size. */
private final RequestLimits resource;
+ /** The size (in byte) of all the requests in this map. */
+ private final AtomicLong requestSize = new AtomicLong();
- RequestMap(Object name, int elementLimit, SizeInBytes byteLimit, RaftServerMetricsImpl raftServerMetrics) {
+ RequestMap(Object name, int elementLimit, int megabyteLimit, RaftServerMetricsImpl raftServerMetrics) {
this.name = name;
- this.resource = new RequestLimits(elementLimit, byteLimit);
+ this.resource = new RequestLimits(elementLimit, megabyteLimit);
this.raftServerMetrics = raftServerMetrics;
raftServerMetrics.addNumPendingRequestsGauge(resource::getElementCount);
@@ -93,8 +108,11 @@ class PendingRequests {
}
Permit tryAcquire(Message message) {
- final ResourceSemaphore.ResourceAcquireStatus acquired = resource.tryAcquire(message);
- LOG.trace("tryAcquire? {}", acquired);
+ final int messageSize = Message.getSize(message);
+ final int messageSizeMb = roundUpMb(messageSize );
+
+ final ResourceAcquireStatus acquired = resource.tryAcquire(messageSizeMb);
+ LOG.trace("tryAcquire {} MB? {}", messageSizeMb, acquired);
if (acquired == ResourceSemaphore.ResourceAcquireStatus.FAILED_IN_ELEMENT_LIMIT) {
raftServerMetrics.onRequestQueueLimitHit();
raftServerMetrics.onResourceLimitHit();
@@ -104,6 +122,14 @@ class PendingRequests {
raftServerMetrics.onResourceLimitHit();
return null;
}
+
+ // release extra MB
+ final long oldSize = requestSize.getAndAdd(messageSize);
+ final long newSize = oldSize + messageSize;
+ final int diffMb = roundUpMb(newSize) - roundUpMb(oldSize);
+ if (messageSizeMb > diffMb) {
+ resource.releaseExtraMb(messageSizeMb - diffMb);
+ }
return putPermit();
}
@@ -140,8 +166,13 @@ class PendingRequests {
if (r == null) {
return null;
}
- resource.release(r.getRequest().getMessage());
- LOG.trace("release");
+
+ final int messageSize = Message.getSize(r.getRequest().getMessage());
+ final long oldSize = requestSize.getAndAdd(-messageSize);
+ final long newSize = oldSize - messageSize;
+ final int diffMb = roundUpMb(oldSize) - roundUpMb(newSize);
+ resource.release(diffMb);
+ LOG.trace("release {} MB", diffMb);
return r;
}
@@ -183,7 +214,7 @@ class PendingRequests {
this.name = id + "-" + JavaUtils.getClassSimpleName(getClass());
this.pendingRequests = new RequestMap(id,
RaftServerConfigKeys.Write.elementLimit(properties),
- RaftServerConfigKeys.Write.byteLimit(properties),
+ RaftServerConfigKeys.Write.byteLimit(properties).getSizeInt()/ONE_MB, //round down
raftServerMetrics);
}
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org