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/09 18:31:32 UTC
[GitHub] [incubator-crail] mbrodmann opened a new pull request #84: Resource Elasticity [1/3]
mbrodmann opened a new pull request #84:
URL: https://github.com/apache/incubator-crail/pull/84
This PR is the first of three planned PRs for implementing resource elasticity within Crail. More specifically, the goal is to allow to automatically and dynamically adapt the number of running datanodes based on different possible parameters (e.g. resource consumption, available storage capacities, ...). This requires support for starting and terminating running datanodes automatically. This functionality will be implemented mostly within the Crail namenode. Therefore, the Crail namenode will not only maintain information on the current state of the datanodes and the deployment but will also start or terminate datanodes as required. For deciding how to adjust the number of datanodes based on the current state observed in the Crail namenode so-called _policies_ are used.
The following changes will be included within the seperate PRs:
1) This first PR implements a new RPC (for both darpc and narpc) that allows to specify a datanode (using IP-address and port number) that should be shutdown. Additionally, a `status` field is added to `DataNodeStatistics`. This will be used in the `getDataNode` RPC to allow the namenode to inform a datanode that it can safely shutdown.
2) The seconds PR will implement the actual mechanism for shutting down a datanode.
3) The third PR implements a simple policy for implementing elasticity based on the system's storage consumption.
----------------------------------------------------------------
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] [incubator-crail] PepperJo commented on a change in pull request #84: Resource Elasticity [1/3]
Posted by GitBox <gi...@apache.org>.
PepperJo commented on a change in pull request #84:
URL: https://github.com/apache/incubator-crail/pull/84#discussion_r576081665
##########
File path: client/src/main/java/org/apache/crail/metadata/DataNodeStatistics.java
##########
@@ -22,25 +22,29 @@
import java.nio.ByteBuffer;
public class DataNodeStatistics {
- public static final int CSIZE = 12;
+ public static final int CSIZE = 14;
private long serviceId;
private int freeBlockCount;
+ private short status;
Review comment:
Can you provide possible values for the status? If I understand it correctly it uses RpcErrors.ERR_DATANODE_STOP only? Error seems not to fit to status. Maybe we should define a new class DataNodeStatus where we define the possible status?
##########
File path: client/src/main/java/org/apache/crail/rpc/RpcErrors.java
##########
@@ -55,7 +55,8 @@
public static short ERR_DIR_LOCATION_AFFINITY_MISMATCH = 26;
public static short ERR_ADD_BLOCK_FAILED = 27;
public static short ERR_CREATE_FILE_BUG = 28;
-
+ public static short ERR_DATANODE_STOP = 29;
Review comment:
See above.
##########
File path: client/src/main/java/org/apache/crail/rpc/RpcDispatcher.java
##########
@@ -134,6 +135,12 @@ public RpcDispatcher(ConcurrentLinkedQueue<RpcConnection> connectionList) {
return connections[0].pingNameNode();
}
+ @Override
+ public RpcFuture<RpcRemoveDataNode> removeDataNode(
+ InetAddress ipaddr, int port) throws Exception {
+ return connections[0].removeDataNode(ipaddr, port);
Review comment:
This will not work in a multi-namenode environment. We would have to send a removeDataNode to all Namenodes. For now we should at least check if there is only one namenode and throw an exception otherwise (e.g. datanode removal only possible with one namenode at the moment).
##########
File path: rpc/src/main/java/org/apache/crail/rpc/RpcRequestMessage.java
##########
@@ -571,7 +573,63 @@ public int write(ByteBuffer buffer) {
public void update(ByteBuffer buffer) {
op = buffer.getInt();
}
- }
+ }
+
+ public static class RemoveDataNodeReq implements RpcProtocol.NameNodeRpcMessage {
+ private InetAddress ipAddr;
+ private int port;
+
+ public RemoveDataNodeReq(){
+ this.ipAddr = null;
+ this.port = 0;
+ }
+
+ public RemoveDataNodeReq(InetAddress addr, int port){
+ this.ipAddr = addr;
+ this.port = port;
+ }
+
+ public int size() {
+ // sizeof(ip-addr) + sizeof(port)
+ return 4 + Integer.BYTES;
+ }
+
+ public short getType(){
+ return RpcProtocol.REQ_REMOVE_DATANODE;
+ }
+
+ public int write(ByteBuffer buffer) throws IOException {
+ int size = size();
+
+ checkSize(buffer.remaining());
+
+ buffer.put(this.getIPAddress().getAddress());
+ buffer.putInt(this.port());
+ return size;
+ }
+
+ private void checkSize(int remaining) throws IOException {
+ if(this.size() > remaining)
+ throw new IOException("Only " + remaining + " remaining bytes stored in buffer, however " + this.size() + " bytes are required");
+ }
+
+ public void update(ByteBuffer buffer) throws IOException {
+ checkSize(buffer.remaining());
+
+ byte[] b = new byte[4];
+ buffer.get(b);
+ this.ipAddr = InetAddress.getByAddress(b);
+ this.port = buffer.getInt();
+ }
+
+ public InetAddress getIPAddress(){
Review comment:
Minor. Be consistent with the { sometimes there is a space sometimes there isn't. (I prefer space)
##########
File path: rpc/src/main/java/org/apache/crail/rpc/RpcResponseMessage.java
##########
@@ -652,4 +656,49 @@ public void setError(short error) {
this.error = error;
}
}
+
+ public static class RemoveDataNodeRes implements RpcProtocol.NameNodeRpcMessage, RpcRemoveDataNode {
+ public static int CSIZE = Short.BYTES;
+
+ private short data;
+ private short error;
+
+ public RemoveDataNodeRes() {
+ this.data = 0;
+ this.error = 0;
+ }
+
+ public int size() {
+ return CSIZE;
+ }
+
+ public short getType(){
+ return RpcProtocol.RES_REMOVE_DATANODE;
+ }
+
+ public int write(ByteBuffer buffer) {
+ buffer.putShort(data);
+ return CSIZE;
+ }
+
+ public void update(ByteBuffer buffer) {
+ data = buffer.getShort();
+ }
+
+ public short getData(){
Review comment:
What data? Be a bit more specific. It looks like in https://github.com/apache/incubator-crail/pull/85 it is used as an error.
----------------------------------------------------------------
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] [incubator-crail] patrickstuedi commented on pull request #84: Resource Elasticity [1/3]
Posted by GitBox <gi...@apache.org>.
patrickstuedi commented on pull request #84:
URL: https://github.com/apache/incubator-crail/pull/84#issuecomment-776974853
looks good
----------------------------------------------------------------
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] [incubator-crail] mbrodmann commented on a change in pull request #84: Resource Elasticity [1/3]
Posted by GitBox <gi...@apache.org>.
mbrodmann commented on a change in pull request #84:
URL: https://github.com/apache/incubator-crail/pull/84#discussion_r576394716
##########
File path: client/src/main/java/org/apache/crail/metadata/DataNodeStatistics.java
##########
@@ -22,25 +22,29 @@
import java.nio.ByteBuffer;
public class DataNodeStatistics {
- public static final int CSIZE = 12;
+ public static final int CSIZE = 14;
private long serviceId;
private int freeBlockCount;
+ private short status;
Review comment:
Yes, that is correct. Right now the status field is only changed to RpcErrors.ERR_DATANODE_STOP when a datanode is selected for shutdown. Replacing it with a new class seems like a good idea.
----------------------------------------------------------------
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] [incubator-crail] PepperJo merged pull request #84: Resource Elasticity [1/3]
Posted by GitBox <gi...@apache.org>.
PepperJo merged pull request #84:
URL: https://github.com/apache/incubator-crail/pull/84
----------------------------------------------------------------
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