Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.hadoop.metrics2.lib.MutableCounterLong;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.util.PerformanceMetrics;
import org.apache.hadoop.util.PerformanceMetricsInitializer;

/**
* The client metrics for the Storage Container protocol.
Expand Down Expand Up @@ -76,7 +75,7 @@ public void init() {
"number of" + ContainerProtos.Type.forNumber(i + 1) + " ops",
(long) 0);
containerOpsLatency[i] =
PerformanceMetricsInitializer.getMetrics(registry,
new PerformanceMetrics(registry,
ContainerProtos.Type.forNumber(i + 1) + "Latency",
"latency of " + ContainerProtos.Type.forNumber(i + 1),
"Ops", "Time", intervals);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@

import com.google.common.base.Strings;

import static org.apache.hadoop.ozone.OzoneConsts.GB;
import static org.apache.hadoop.ozone.OzoneConsts.KB;
import static org.apache.hadoop.ozone.OzoneConsts.MB;
import static org.apache.hadoop.ozone.OzoneConsts.TB;


/**
* represents an OzoneQuota Object that can be applied to
Expand All @@ -44,27 +39,17 @@ public enum Units { B, KB, MB, GB, TB }
// Quota to decide how many buckets can be created.
private long quotaInNamespace;
// Quota to decide how many storage space will be used in bytes.
private long quotaInBytes;
private RawQuotaInBytes rawQuotaInBytes;
private final long quotaInBytes;
private final RawQuotaInBytes rawQuotaInBytes;
// Data class of Quota.
private static QuotaList quotaList;

/** Setting QuotaList parameters from large to small. */
static {
quotaList = new QuotaList();
quotaList.addQuotaList(OZONE_QUOTA_TB, Units.TB, TB);
quotaList.addQuotaList(OZONE_QUOTA_GB, Units.GB, GB);
quotaList.addQuotaList(OZONE_QUOTA_MB, Units.MB, MB);
quotaList.addQuotaList(OZONE_QUOTA_KB, Units.KB, KB);
quotaList.addQuotaList(OZONE_QUOTA_B, Units.B, 1L);
}
private static final QuotaList QUOTA_LIST = new QuotaList();

@szetszwo szetszwo May 18, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OzoneQuota used to initalize contents of QuotaList, moved to the latter

The QuotaList class is inefficient. Let's remove it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Deferred to HDDS-10897.


/**
* Used to convert user input values into bytes such as: 1MB-> 1048576.
*/
private static class RawQuotaInBytes {
private Units unit;
private long size;
private final Units unit;
private final long size;

RawQuotaInBytes(Units unit, long size) {
this.unit = unit;
Expand All @@ -84,9 +69,9 @@ public long getSize() {
*/
public long sizeInBytes() {
long sQuota = -1L;
for (Units quota : quotaList.getUnitQuotaArray()) {
for (Units quota : QUOTA_LIST.getUnitQuotaArray()) {
if (quota == this.unit) {
sQuota = quotaList.getQuotaSize(quota);
sQuota = QUOTA_LIST.getQuotaSize(quota);
break;
}
}
Expand Down Expand Up @@ -162,11 +147,11 @@ public static OzoneQuota parseSpaceQuota(String quotaInBytes) {
Units currUnit = Units.B;

try {
for (String quota : quotaList.getOzoneQuotaArray()) {
for (String quota : QUOTA_LIST.getOzoneQuotaArray()) {
if (uppercase.endsWith((quota))) {
size = uppercase
.substring(0, uppercase.length() - quota.length());
currUnit = quotaList.getUnits(quota);
currUnit = QUOTA_LIST.getUnits(quota);
break;
}
}
Expand Down Expand Up @@ -242,10 +227,10 @@ public static OzoneQuota getOzoneQuota(long quotaInBytes,
long quotaInNamespace) {
long size = 1L;
Units unit = Units.B;
for (Long quota : quotaList.getSizeQuotaArray()) {
for (Long quota : QUOTA_LIST.getSizeQuotaArray()) {
if (quotaInBytes % quota == 0) {
size = quotaInBytes / quota;
unit = quotaList.getQuotaUnit(quota);
unit = QUOTA_LIST.getQuotaUnit(quota);
}
}
return new OzoneQuota(quotaInNamespace, new RawQuotaInBytes(unit, size));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@
package org.apache.hadoop.hdds.client;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.apache.hadoop.ozone.OzoneConsts.GB;
import static org.apache.hadoop.ozone.OzoneConsts.KB;
import static org.apache.hadoop.ozone.OzoneConsts.MB;
import static org.apache.hadoop.ozone.OzoneConsts.TB;

/**
*This class contains arraylist for storage constant used in OzoneQuota.
*/
Expand All @@ -33,25 +39,32 @@ public QuotaList() {
ozoneQuota = new ArrayList<>();
unitQuota = new ArrayList<>();
sizeQuota = new ArrayList<>();

// Setting QuotaList parameters from large to small.
addQuotaList(OzoneQuota.OZONE_QUOTA_TB, OzoneQuota.Units.TB, TB);
addQuotaList(OzoneQuota.OZONE_QUOTA_GB, OzoneQuota.Units.GB, GB);
addQuotaList(OzoneQuota.OZONE_QUOTA_MB, OzoneQuota.Units.MB, MB);
addQuotaList(OzoneQuota.OZONE_QUOTA_KB, OzoneQuota.Units.KB, KB);
addQuotaList(OzoneQuota.OZONE_QUOTA_B, OzoneQuota.Units.B, 1L);
}

public void addQuotaList(
private void addQuotaList(
String oQuota, OzoneQuota.Units uQuota, Long sQuota) {
ozoneQuota.add(oQuota);
unitQuota.add(uQuota);
sizeQuota.add(sQuota);
}

public List<String> getOzoneQuotaArray() {
return this.ozoneQuota;
return Collections.unmodifiableList(this.ozoneQuota);
}

public List<Long> getSizeQuotaArray() {
return this.sizeQuota;
return Collections.unmodifiableList(this.sizeQuota);
}

public List<OzoneQuota.Units> getUnitQuotaArray() {
return this.unitQuota;
return Collections.unmodifiableList(this.unitQuota);
}

public OzoneQuota.Units getUnits(String oQuota) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -163,15 +164,11 @@ public XMLConfiguration() {
}

public XMLConfiguration(List<Property> properties) {
this.properties = properties;
this.properties = new ArrayList<>(properties);
}

public List<Property> getProperties() {
return properties;
}

public void setProperties(List<Property> properties) {
this.properties = properties;
return Collections.unmodifiableList(properties);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.hadoop.hdds.freon;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;
import java.util.UUID;
Expand All @@ -36,20 +37,20 @@
* Class to store pre-generated topology information for load-tests.
*/
@SuppressWarnings("java:S2245") // no need for secure random
public class FakeClusterTopology {
public final class FakeClusterTopology {

private static final Logger LOGGER =
LoggerFactory.getLogger(FakeClusterTopology.class);

public static final FakeClusterTopology INSTANCE = new FakeClusterTopology();

private List<DatanodeDetailsProto> datanodes = new ArrayList<>();
private final List<DatanodeDetailsProto> datanodes = new ArrayList<>();

private List<Pipeline> pipelines = new ArrayList<>();
private final List<Pipeline> pipelines = new ArrayList<>();

private Random random = new Random();
private final Random random = new Random();

public FakeClusterTopology() {
private FakeClusterTopology() {
try {
for (int i = 0; i < 9; i++) {
datanodes.add(createDatanode());
Expand Down Expand Up @@ -87,7 +88,7 @@ public Pipeline getRandomPipeline() {
return pipelines.get(random.nextInt(pipelines.size()));
}

public List<DatanodeDetailsProto> getAllDatanodes() {
return datanodes;
public Iterable<DatanodeDetailsProto> getAllDatanodes() {
return Collections.unmodifiableList(datanodes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public synchronized void setPort(Name name, int port) {
* @return DataNode Ports
*/
public synchronized List<Port> getPorts() {
return ports;
return new ArrayList<>(ports);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also change the callers in order to reduce copying:

  • NodeDecommissionManager
//NodeDecommissionManager
  private boolean validateDNPortMatch(int port, DatanodeDetails dn) {
    return dn.containPort(port);
  }
//DatanodeDetails
  public synchronized boolean containPort(int port) {
    for (Port p : ports) {
      if (p.getValue() == port) {
        return true;
      }
    }
    return false;
  }
  • DatanodeIdYaml
@@ -238,8 +239,9 @@ private static DatanodeDetailsYaml getDatanodeDetailsYaml(
         = new DatanodeLayoutStorage(conf, datanodeDetails.getUuidString());
 
     Map<String, Integer> portDetails = new LinkedHashMap<>();
-    if (!CollectionUtils.isEmpty(datanodeDetails.getPorts())) {
-      for (DatanodeDetails.Port port : datanodeDetails.getPorts()) {
+    final List<DatanodeDetails.Port> datanodePorts = datanodeDetails.getPorts();
+    if (!CollectionUtils.isEmpty(datanodePorts)) {
+      for (DatanodeDetails.Port port : datanodePorts) {
         Field f = null;
         try {
           f = DatanodeDetails.Port.Name.class

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, updated.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,25 @@
package org.apache.hadoop.hdds.scm;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
* ScmInfo wraps the result returned from SCM#getScmInfo which
* contains clusterId and the SCM Id.
*/
public final class ScmInfo {
private String clusterId;
private String scmId;
private List<String> peerRoles;
private final String clusterId;
private final String scmId;
private final List<String> peerRoles;

/**
* Builder for ScmInfo.
*/
public static class Builder {
private String clusterId;
private String scmId;
private List<String> peerRoles;
private final List<String> peerRoles;

public Builder() {
peerRoles = new ArrayList<>();
Expand Down Expand Up @@ -104,6 +105,6 @@ public String getScmId() {
* @return List of peer address
*/
public List<String> getRatisPeerRoles() {
return peerRoles;
return Collections.unmodifiableList(peerRoles);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do it in constructor.

  private ScmInfo(String clusterId, String scmId, List<String> peerRoles) {
    this.clusterId = clusterId;
    this.scmId = scmId;
    this.peerRoles = Collections.unmodifiableList(peerRoles);
  }

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
.ContainerCommandResponseProto;

import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -60,7 +61,7 @@ public void setLogIndex(long logIndex) {
}

public List<DatanodeDetails> getDatanodes() {
return datanodes;
return Collections.unmodifiableList(datanodes);
}

public void addDatanode(DatanodeDetails dn) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.time.Clock;
import java.time.ZoneOffset;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
Expand All @@ -38,28 +39,24 @@
*/
public class ExcludeList {

private final Map<DatanodeDetails, Long> datanodes;
private final Set<ContainerID> containerIds;
private final Set<PipelineID> pipelineIds;
private final Map<DatanodeDetails, Long> datanodes = new ConcurrentHashMap<>();
private final Set<ContainerID> containerIds = new HashSet<>();
private final Set<PipelineID> pipelineIds = new HashSet<>();
private long expiryTime = 0;
private java.time.Clock clock;
private final Clock clock;


public ExcludeList() {
datanodes = new ConcurrentHashMap<>();
containerIds = new HashSet<>();
pipelineIds = new HashSet<>();
clock = Clock.system(ZoneOffset.UTC);
}

public ExcludeList(long autoExpiryTime, java.time.Clock clock) {
this();
public ExcludeList(long autoExpiryTime, Clock clock) {
this.expiryTime = autoExpiryTime;
this.clock = clock;
}

public Set<ContainerID> getContainerIds() {
return containerIds;
return Collections.unmodifiableSet(containerIds);
}

public Set<DatanodeDetails> getDatanodes() {
Expand Down Expand Up @@ -99,7 +96,7 @@ public void addPipeline(PipelineID pipelineId) {
}

public Set<PipelineID> getPipelineIds() {
return pipelineIds;
return Collections.unmodifiableSet(pipelineIds);
}

public HddsProtos.ExcludeListProto getProtoBuf() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public final class SCMHAUtils {
public static final Logger LOG =
LoggerFactory.getLogger(SCMHAUtils.class);

private static final List<Class<? extends Exception>>
private static final ImmutableList<Class<? extends Exception>>
RETRIABLE_WITH_NO_FAILOVER_EXCEPTION_LIST =
ImmutableList.<Class<? extends Exception>>builder()
.add(LeaderNotReadyException.class)
Expand All @@ -74,7 +74,7 @@ public final class SCMHAUtils {
.add(ResourceUnavailableException.class)
.build();

private static final List<Class<? extends Exception>>
private static final ImmutableList<Class<? extends Exception>>
NON_RETRIABLE_EXCEPTION_LIST =
ImmutableList.<Class<? extends Exception>>builder()
.add(SCMException.class)
Expand Down Expand Up @@ -316,7 +316,7 @@ public static Throwable getExceptionForClass(Exception e,
return null;
}

public static List<Class<? extends
private static List<Class<? extends
Exception>> getRetriableWithNoFailoverExceptionList() {
return RETRIABLE_WITH_NO_FAILOVER_EXCEPTION_LIST;
}
Expand Down
Loading