ChenSammi commented on a change in pull request #1233:
URL: https://github.com/apache/hadoop-ozone/pull/1233#discussion_r482675817
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java
##########
@@ -53,26 +114,27 @@ public long getSize() {
* @return Unit in MB, GB or TB
*/
public Units getUnit() {
- return unit;
+ return this.rawQuotaInBytes.getUnit();
}
/**
* Constructs a default Quota object.
*/
- public OzoneQuota() {
- this.size = 0;
- this.unit = Units.UNDEFINED;
+ private OzoneQuota() {
+ this.quotaInCounts = OzoneConsts.QUOTA_RESET;
+ this.quotaInBytes = OzoneConsts.QUOTA_RESET;
Review comment:
rawQuotaInBytes initialization is missed.
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java
##########
@@ -82,118 +144,130 @@ public OzoneQuota(long size, Units unit) {
* @return string representation of quota
*/
public static String formatQuota(OzoneQuota quota) {
- return String.valueOf(quota.size) + quota.unit;
+ return String.valueOf(quota.getRawSize())+ quota.getUnit();
Review comment:
need space before "+"
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java
##########
@@ -19,32 +19,93 @@
package org.apache.hadoop.hdds.client;
import org.apache.hadoop.ozone.OzoneConsts;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+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
* a storage volume.
*/
-public class OzoneQuota {
+public final class OzoneQuota {
Review comment:
Shall we rename this class name to VolumeQuota?
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java
##########
@@ -82,118 +144,130 @@ public OzoneQuota(long size, Units unit) {
* @return string representation of quota
*/
public static String formatQuota(OzoneQuota quota) {
- return String.valueOf(quota.size) + quota.unit;
+ return String.valueOf(quota.getRawSize())+ quota.getUnit();
}
/**
* Parses a user provided string and returns the
* Quota Object.
*
- * @param quotaString Quota String
+ * @param quotaInBytes Volume quota in bytes
+ * @param quotaInCounts Volume quota in counts
*
* @return OzoneQuota object
*/
- public static OzoneQuota parseQuota(String quotaString) {
+ public static OzoneQuota parseQuota(String quotaInBytes,
+ long quotaInCounts) {
- if ((quotaString == null) || (quotaString.isEmpty())) {
+ if ((quotaInBytes == null) || (quotaInBytes.isEmpty())) {
throw new IllegalArgumentException(
"Quota string cannot be null or empty.");
}
- String uppercase = quotaString.toUpperCase().replaceAll("\\s+", "");
+ String uppercase = quotaInBytes.toUpperCase()
+ .replaceAll("\\s+", "");
String size = "";
- int nSize;
+ long nSize = 0;
Units currUnit = Units.MB;
- boolean found = false;
- if (uppercase.endsWith(OZONE_QUOTA_MB)) {
- size = uppercase
- .substring(0, uppercase.length() - OZONE_QUOTA_MB.length());
- currUnit = Units.MB;
- found = true;
- }
+ long quotaMultiplyExact = 0;
- if (uppercase.endsWith(OZONE_QUOTA_GB)) {
- size = uppercase
- .substring(0, uppercase.length() - OZONE_QUOTA_GB.length());
- currUnit = Units.GB;
- found = true;
- }
+ try {
+ if (uppercase.endsWith(OZONE_QUOTA_KB)) {
+ size = uppercase
+ .substring(0, uppercase.length() - OZONE_QUOTA_KB.length());
+ currUnit = Units.KB;
+ quotaMultiplyExact = Math.multiplyExact(Long.parseLong(size), KB);
+ }
- if (uppercase.endsWith(OZONE_QUOTA_TB)) {
- size = uppercase
- .substring(0, uppercase.length() - OZONE_QUOTA_TB.length());
- currUnit = Units.TB;
- found = true;
- }
+ if (uppercase.endsWith(OZONE_QUOTA_MB)) {
+ size = uppercase
+ .substring(0, uppercase.length() - OZONE_QUOTA_MB.length());
+ currUnit = Units.MB;
+ quotaMultiplyExact = Math.multiplyExact(Long.parseLong(size), MB);
+ }
- if (uppercase.endsWith(OZONE_QUOTA_BYTES)) {
- size = uppercase
- .substring(0, uppercase.length() - OZONE_QUOTA_BYTES.length());
- currUnit = Units.BYTES;
- found = true;
- }
+ if (uppercase.endsWith(OZONE_QUOTA_GB)) {
+ size = uppercase
+ .substring(0, uppercase.length() - OZONE_QUOTA_GB.length());
+ currUnit = Units.GB;
+ quotaMultiplyExact = Math.multiplyExact(Long.parseLong(size), GB);
+ }
- if (!found) {
- throw new IllegalArgumentException("Quota unit not recognized. " +
- "Supported values are BYTES, MB, GB and TB.");
+ if (uppercase.endsWith(OZONE_QUOTA_TB)) {
+ size = uppercase
+ .substring(0, uppercase.length() - OZONE_QUOTA_TB.length());
+ currUnit = Units.TB;
+ quotaMultiplyExact = Math.multiplyExact(Long.parseLong(size), TB);
+ }
+
+ if (uppercase.endsWith(OZONE_QUOTA_BYTES)) {
+ size = uppercase
+ .substring(0, uppercase.length() - OZONE_QUOTA_BYTES.length());
+ currUnit = Units.BYTES;
+ quotaMultiplyExact = Math.multiplyExact(Long.parseLong(size), 1L);
+ }
+ nSize = Long.parseLong(size);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Invalid values for quota, to ensure"
+
+ " that the Quota format is legal(supported values are BYTES, KB, " +
+ "MB, GB and TB).");
+ } catch (ArithmeticException e) {
+ LOG.debug("long overflow:\n{}", quotaMultiplyExact);
+ throw new IllegalArgumentException("Invalid values for quota, the quota"
+
+ " value cannot be greater than Long.MAX_VALUE BYTES");
}
- nSize = Integer.parseInt(size);
if (nSize < 0) {
throw new IllegalArgumentException("Quota cannot be negative.");
}
- return new OzoneQuota(nSize, currUnit);
+ return new OzoneQuota(quotaInCounts,
+ new RawQuotaInBytes(currUnit, nSize));
}
- /**
- * Returns size in Bytes or -1 if there is no Quota.
- */
- public long sizeInBytes() {
- switch (this.unit) {
- case BYTES:
- return this.getSize();
- case MB:
- return this.getSize() * OzoneConsts.MB;
- case GB:
- return this.getSize() * OzoneConsts.GB;
- case TB:
- return this.getSize() * OzoneConsts.TB;
- case UNDEFINED:
- default:
- return -1;
- }
- }
-
/**
* Returns OzoneQuota corresponding to size in bytes.
*
- * @param sizeInBytes size in bytes to be converted
+ * @param quotaInBytes in bytes to be converted
+ * @param quotaInCounts in counts to be converted
*
* @return OzoneQuota object
*/
- public static OzoneQuota getOzoneQuota(long sizeInBytes) {
+ public static OzoneQuota getOzoneQuota(long quotaInBytes,
+ long quotaInCounts) {
long size;
Units unit;
- if (sizeInBytes % OzoneConsts.TB == 0) {
- size = sizeInBytes / OzoneConsts.TB;
+ if (quotaInBytes % TB == 0) {
+ size = quotaInBytes / TB;
unit = Units.TB;
- } else if (sizeInBytes % OzoneConsts.GB == 0) {
- size = sizeInBytes / OzoneConsts.GB;
+ } else if (quotaInBytes % GB == 0) {
+ size = quotaInBytes / GB;
unit = Units.GB;
- } else if (sizeInBytes % OzoneConsts.MB == 0) {
- size = sizeInBytes / OzoneConsts.MB;
+ } else if (quotaInBytes % MB == 0) {
+ size = quotaInBytes / MB;
unit = Units.MB;
+ } else if (quotaInBytes % KB == 0) {
+ size = quotaInBytes / KB;
+ unit = Units.KB;
} else {
- size = sizeInBytes;
+ size = quotaInBytes;
unit = Units.BYTES;
}
- return new OzoneQuota((int)size, unit);
+ return new OzoneQuota(quotaInCounts, new RawQuotaInBytes(unit, size));
+ }
+
+ public long getQuotaInCounts() {
+ return quotaInCounts;
+ }
+
+ public long getQuotaInBytes() {
+ return quotaInBytes;
}
@Override
public String toString() {
- return size + " " + unit;
+ return "Bytes Quota: " + rawQuotaInBytes.toString() + "\n" +
Review comment:
Bytes Quota -> Space Bytes Quota
Counts Quota -> Bucket Counts Quota
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java
##########
@@ -82,118 +144,130 @@ public OzoneQuota(long size, Units unit) {
* @return string representation of quota
*/
public static String formatQuota(OzoneQuota quota) {
- return String.valueOf(quota.size) + quota.unit;
+ return String.valueOf(quota.getRawSize())+ quota.getUnit();
}
/**
* Parses a user provided string and returns the
* Quota Object.
*
- * @param quotaString Quota String
+ * @param quotaInBytes Volume quota in bytes
+ * @param quotaInCounts Volume quota in counts
*
* @return OzoneQuota object
*/
- public static OzoneQuota parseQuota(String quotaString) {
+ public static OzoneQuota parseQuota(String quotaInBytes,
+ long quotaInCounts) {
- if ((quotaString == null) || (quotaString.isEmpty())) {
+ if ((quotaInBytes == null) || (quotaInBytes.isEmpty())) {
Review comment:
Use Strings.isNullOrEmpty instead.
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/CreateVolumeHandler.java
##########
@@ -40,10 +41,13 @@
description = "Owner of the volume")
private String ownerName;
+ @Option(names = {"--spaceQuota", "-s"},
+ description = "Quota in bytes of the newly created volume (eg. 1GB)")
+ private String quotaInBytes;
+
@Option(names = {"--quota", "-q"},
Review comment:
--quota -> bucketQuota
-q -> -bq
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/UpdateVolumeHandler.java
##########
@@ -39,22 +40,33 @@
description = "Owner of the volume to set")
private String ownerName;
- @Option(names = {"--quota"},
- description = "Quota of the volume to set"
- + "(eg. 1G)")
- private String quota;
+ @Option(names = {"--spaceQuota", "-s"},
Review comment:
same as above
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -204,6 +204,16 @@ public static Versioning getVersioning(boolean versioning)
{
*/
public static final long MAX_QUOTA_IN_BYTES = 1024L * 1024 * TB;
+ /**
+ * Quota RESET default is -1, which means it does not take effect.
Review comment:
which means it does not take effect -> which means quota is not set
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/CreateVolumeHandler.java
##########
@@ -40,10 +41,13 @@
description = "Owner of the volume")
private String ownerName;
+ @Option(names = {"--spaceQuota", "-s"},
Review comment:
-s -> -sq
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
