You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@crail.apache.org by GitBox <gi...@apache.org> on 2021/02/22 10:37:53 UTC
[GitHub] [incubator-crail] PepperJo commented on a change in pull request #86: Resource Elasticity [3/3]
PepperJo commented on a change in pull request #86:
URL: https://github.com/apache/incubator-crail/pull/86#discussion_r580136092
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -107,6 +107,85 @@ short prepareDataNodeForRemoval(DataNodeInfo dn) throws IOException {
return RpcErrors.ERR_DATANODE_NOT_REGISTERED;
}
+ public double getStorageUsage() throws Exception {
Review comment:
Naming. Maybe `getStorageUsedPercentage` or `getFractionStorageUsed`?
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -107,6 +107,85 @@ short prepareDataNodeForRemoval(DataNodeInfo dn) throws IOException {
return RpcErrors.ERR_DATANODE_NOT_REGISTERED;
}
+ public double getStorageUsage() throws Exception {
+ long total = 0;
+ long free = 0;
+ for(StorageClass storageClass : storageClasses) {
+ total += storageClass.getTotalCapacity();
+ free += storageClass.getFreeCapacity();
+ }
+
+ // if there is no available capacity (i.e. total number of available blocks is 0),
+ // return 1.0 which tells that all storage is used
+ if(total != 0) {
+ double available = (double) free / (double) total;
+ return 1.0 - available;
+ } else {
+ return 1.0;
+ }
+
+ }
+
+ public int getBlockUsage() throws Exception {
+ int total = 0;
+
+ for(StorageClass storageClass: storageClasses) {
+ total += (storageClass.getTotalCapacity() - storageClass.getFreeCapacity());
+ }
+
+ return total;
+ }
+
+ public int getBlockCapacity() throws Exception {
Review comment:
Should return a long since `getTotalCapacity` returns a long otherwise this might overflow.
Again naming: Maybe `getNumberOfBlocks` ?
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -107,6 +107,85 @@ short prepareDataNodeForRemoval(DataNodeInfo dn) throws IOException {
return RpcErrors.ERR_DATANODE_NOT_REGISTERED;
}
+ public double getStorageUsage() throws Exception {
+ long total = 0;
+ long free = 0;
+ for(StorageClass storageClass : storageClasses) {
+ total += storageClass.getTotalCapacity();
+ free += storageClass.getFreeCapacity();
+ }
+
+ // if there is no available capacity (i.e. total number of available blocks is 0),
+ // return 1.0 which tells that all storage is used
+ if(total != 0) {
+ double available = (double) free / (double) total;
+ return 1.0 - available;
+ } else {
+ return 1.0;
+ }
+
+ }
+
+ public int getBlockUsage() throws Exception {
Review comment:
Should return `long` as `getTotalCapacity` and `getFreeCapactiy` return long. Otherwise this could overflow.
Also Naming: Maybe `getNumberOfBlocksUsed`
##########
File path: namenode/src/main/java/org/apache/crail/namenode/DataNodeBlocks.java
##########
@@ -86,6 +86,10 @@ public boolean isScheduleForRemoval(){
return this.scheduleForRemoval;
}
+ public long getMaxBlockCount() {
Review comment:
I guess currently we do not shrink block count so block count is always increasing for a datanode. Maybe in the future this might change so I propose to name the method instead: `getTotalBlockCount` or `getTotalNumberOfBlocks`.
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -222,6 +303,33 @@ short removeDatanode(DataNodeInfo dn) throws IOException {
//---------------
+ public long getTotalCapacity() {
+ long capacity = 0;
+
+ for(DataNodeBlocks datanode : membership.values()) {
+ capacity += datanode.getMaxBlockCount();
+ }
+
+ return capacity;
+ }
+
+ public long getFreeCapacity() {
Review comment:
Also naming. Maybe `getNumberOfFreeBlocks` or `getFreeBlockCount`?
##########
File path: namenode/src/main/java/org/apache/crail/namenode/NameNode.java
##########
@@ -81,8 +81,15 @@ public static void main(String args[]) throws Exception {
CrailConstants.NAMENODE_ADDRESS = namenode + "?id=" + serviceId + "&size=" + serviceSize;
CrailConstants.printConf();
CrailConstants.verify();
-
- RpcNameNodeService service = RpcNameNodeService.createInstance(CrailConstants.NAMENODE_RPC_SERVICE);
+
+ RpcNameNodeService service;
+
+ if(CrailConstants.ELASTICSTORE) {
Review comment:
Do we really need this? Wouldn't it be sufficient if we use `CrailConstants.NAMENODE_RPC_SERVICE` and specify the elastic namenode rpc service class there?
##########
File path: namenode/src/main/java/org/apache/crail/namenode/NameNodeService.java
##########
@@ -552,6 +552,26 @@ public short getLocation(RpcRequestMessage.GetLocationReq request, RpcResponseMe
return RpcErrors.ERR_OK;
}
+ public double getStorageUsage() throws Exception {
Review comment:
Naming. See above.
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -222,6 +303,33 @@ short removeDatanode(DataNodeInfo dn) throws IOException {
//---------------
+ public long getTotalCapacity() {
Review comment:
The naming does not clearly state that this is number of blocks. Maybe `getTotalNumberOfBlocks` or `getTotalNumberBlocks` or `getTotalBlockCount`?
##########
File path: namenode/src/main/java/org/apache/crail/namenode/BlockStore.java
##########
@@ -222,6 +303,33 @@ short removeDatanode(DataNodeInfo dn) throws IOException {
//---------------
+ public long getTotalCapacity() {
+ long capacity = 0;
+
+ for(DataNodeBlocks datanode : membership.values()) {
+ capacity += datanode.getMaxBlockCount();
+ }
+
+ return capacity;
+ }
+
+ public long getFreeCapacity() {
+ long capacity = 0;
+
+ for(DataNodeBlocks datanode : membership.values()) {
+ capacity += datanode.getBlockCount();
+ }
+
+ return capacity;
+ }
+
+ public Collection<DataNodeBlocks> getDataNodeBlocks() {
+ return this.membership.values();
+ }
+
+ public int getRunningDatanodes() {
Review comment:
Naming. Maybe `getNumberOfRunningDatanodes()`?
I know its long but it could be misinterpreted otherwise.
----------------------------------------------------------------
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