Skip to content

HDDS-5283. getStorageSize cast to int can cause issue #2303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mukul1987 merged 17 commits into from
Aug 11, 2021
Merged

HDDS-5283. getStorageSize cast to int can cause issue #2303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
2ae3968
HDDS-5283.getStorageSize cast to int can cause issue
Jun 3, 2021
144b524
modified another variable
Jun 3, 2021
bb93d1b
trigger new CI check
Jun 3, 2021
8e63aca
trigger new CI check
Jun 4, 2021
ed61777
address comments
Jun 4, 2021
cc18490
fix checkstyle
Jun 4, 2021
1e73937
fix checkstyle
Jun 4, 2021
71af3c4
data size change
Jun 24, 2021
c1389fc
add method to round up mb
Jul 12, 2021
c678612
fix acceptance test failure
Jul 12, 2021
2f77c59
fix checkstyle
Jul 12, 2021
cc43587
fix checkstyle
Jul 12, 2021
720c9e5
trigger new CI check
Jul 12, 2021
350ef4a
fix acceptance
Jul 15, 2021
0f237e4
Update ratis version
Aug 4, 2021
2512a9d
Us Math.ceil instead
Aug 5, 2021
b553e78
trigger new CI check
Aug 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_KEY;
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES;

import org.apache.ratis.util.SizeInBytes;
import org.apache.hadoop.ozone.conf.OzoneServiceConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -90,6 +91,8 @@ public final class HddsUtils {
ScmConfigKeys.OZONE_SCM_NAMES + " must contain a single hostname."
+ " Multiple SCM hosts are currently unsupported";

private static final int ONE_MB = SizeInBytes.valueOf("1m").getSizeInt();

private static final int NO_PORT = -1;

private HddsUtils() {
Expand Down Expand Up @@ -612,4 +615,11 @@ public static String format(List<String> nodes) {
public static long getShutDownTimeOut(ConfigurationSource conf) {
return conf.getObject(OzoneServiceConfig.class).getServiceShutdownTimeout();
}

/**
* Utility method to round up bytes into the nearest MB.
*/
public static int roundupMb(long bytes) {
return (int)Math.ceil((double) bytes/(double) ONE_MB);
}
}
8 changes: 5 additions & 3 deletions ...rg/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,15 @@ public ContainerStateMachine(RaftGroupId gid, ContainerDispatcher dispatcher,
int numPendingRequests = conf
.getObject(DatanodeRatisServerConfig.class)
.getLeaderNumPendingRequests();
int pendingRequestsByteLimit = (int) conf.getStorageSize(
long pendingRequestsBytesLimit = (long)conf.getStorageSize(
OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT,
OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT_DEFAULT,
StorageUnit.BYTES);
int pendingRequestsMegaBytesLimit =
HddsUtils.roundupMb(pendingRequestsBytesLimit);
stateMachineDataCache = new ResourceLimitCache<>(new ConcurrentHashMap<>(),
(index, data) -> new int[] {1, data.size()}, numPendingRequests,
pendingRequestsByteLimit);
(index, data) -> new int[] {1, HddsUtils.roundupMb(data.size())},
numPendingRequests, pendingRequestsMegaBytesLimit);

this.chunkExecutors = chunkExecutors;

Expand Down
18 changes: 11 additions & 7 deletions ...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.hadoop.hdds.HddsUtils;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.DatanodeRatisServerConfig;
import org.apache.hadoop.hdds.conf.StorageUnit;
Expand Down Expand Up @@ -102,6 +103,7 @@
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.ratis.util.SizeInBytes;
import org.apache.ratis.util.TimeDuration;
import org.apache.ratis.util.TraditionalBinaryPrefix;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -219,7 +221,7 @@ private RaftProperties newRaftProperties() {
setRaftSegmentAndWriteBufferSize(properties);

// set raft segment pre-allocated size
final int raftSegmentPreallocatedSize =
final long raftSegmentPreallocatedSize =
setRaftSegmentPreallocatedSize(properties);

TimeUnit timeUnit;
Expand Down Expand Up @@ -380,8 +382,8 @@ private void setTimeoutForRetryCache(RaftProperties properties) {
.setExpiryTime(properties, retryCacheTimeout);
}

private int setRaftSegmentPreallocatedSize(RaftProperties properties) {
final int raftSegmentPreallocatedSize = (int) conf.getStorageSize(
private long setRaftSegmentPreallocatedSize(RaftProperties properties) {
final long raftSegmentPreallocatedSize = (long) conf.getStorageSize(
OzoneConfigKeys.DFS_CONTAINER_RATIS_SEGMENT_PREALLOCATED_SIZE_KEY,
OzoneConfigKeys.DFS_CONTAINER_RATIS_SEGMENT_PREALLOCATED_SIZE_DEFAULT,
StorageUnit.BYTES);
Expand All @@ -404,7 +406,7 @@ private int setRaftSegmentPreallocatedSize(RaftProperties properties) {
}

private void setRaftSegmentAndWriteBufferSize(RaftProperties properties) {
final int raftSegmentSize = (int)conf.getStorageSize(
final long raftSegmentSize = (long) conf.getStorageSize(
OzoneConfigKeys.DFS_CONTAINER_RATIS_SEGMENT_SIZE_KEY,
OzoneConfigKeys.DFS_CONTAINER_RATIS_SEGMENT_SIZE_DEFAULT,
StorageUnit.BYTES);
Expand All @@ -425,12 +427,14 @@ private RpcType setRpcType(RaftProperties properties) {

private void setPendingRequestsLimits(RaftProperties properties) {

final int pendingRequestsByteLimit = (int)conf.getStorageSize(
long pendingRequestsBytesLimit = (long) conf.getStorageSize(
OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT,
OzoneConfigKeys.DFS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT_DEFAULT,
StorageUnit.BYTES);
RaftServerConfigKeys.Write.setByteLimit(properties,
SizeInBytes.valueOf(pendingRequestsByteLimit));
final int pendingRequestsMegaBytesLimit =
HddsUtils.roundupMb(pendingRequestsBytesLimit);
RaftServerConfigKeys.Write.setByteLimit(properties, SizeInBytes
.valueOf(pendingRequestsMegaBytesLimit, TraditionalBinaryPrefix.MEGA));
}

public static XceiverServerRatis newXceiverServerRatis(
Expand Down
4 changes: 2 additions & 2 deletions ...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ private RaftProperties newRaftProperties(ConfigurationSource conf) {
RaftServerConfigKeys.LeaderElection.setPreVote(properties, false);

// Set RAFT segment size
final int raftSegmentSize = (int) conf.getStorageSize(
final long raftSegmentSize = (long) conf.getStorageSize(
OMConfigKeys.OZONE_OM_RATIS_SEGMENT_SIZE_KEY,
OMConfigKeys.OZONE_OM_RATIS_SEGMENT_SIZE_DEFAULT,
StorageUnit.BYTES);
Expand All @@ -562,7 +562,7 @@ private RaftProperties newRaftProperties(ConfigurationSource conf) {
RaftServerConfigKeys.Log.setPurgeUptoSnapshotIndex(properties, true);

// Set RAFT segment pre-allocated size
final int raftSegmentPreallocatedSize = (int) conf.getStorageSize(
final long raftSegmentPreallocatedSize = (long) conf.getStorageSize(
OMConfigKeys.OZONE_OM_RATIS_SEGMENT_PREALLOCATED_SIZE_KEY,
OMConfigKeys.OZONE_OM_RATIS_SEGMENT_PREALLOCATED_SIZE_DEFAULT,
StorageUnit.BYTES);
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
<declared.ozone.version>${ozone.version}</declared.ozone.version>

<!-- Apache Ratis version -->
<ratis.version>2.1.0</ratis.version>
<ratis.version>2.1.0-03f3b68-SNAPSHOT</ratis.version>
Copy link
Contributor

@adoroszlai adoroszlai Aug 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this seems to be wrong. Ratis 2.1.0 is already released, we cannot have a new snapshot version for it. I think we should go for a new 2.1.1 release with RATIS-1384 (and RATIS-1386) bugfix.


<!-- Apache Ratis thirdparty version -->
<ratis.thirdparty.version>0.7.0</ratis.thirdparty.version>
Expand Down