HDDS-2592 Add Datanode command to allow the datanode to persist its admin state #521
Conversation
…state to the datanode via the existing command queue
…t the values in the datanode yaml file (datanode.id) and ensure the parameters are sent with registration and the heartbeat
…uring registration and heartbeat
...r-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java
Show resolved
Hide resolved
| persistDatanodeDetails(dni); | ||
| } catch (IOException ioe) { | ||
| LOG.error("Failed to persist the datanode state", ioe); | ||
| // TODO - this should probably be raised, but it will break the command |
There was a problem hiding this comment.
We can add this to the Dn reports if you want to propagate this back into SCM.
...r-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java
Show resolved
Hide resolved
|
@anuengineer Thanks for the review on this. I probably need to add some tests around these changes. I did not do that initially, as I wanted to be sure we were happy with the approach (storing the state in the datanode details, allowing it to be included in the heartbeat and the DN not making further use of the value). Now you have reviewed and are happy with this direction, I will get some tests added and check your comments in more detail. |
|
/pending "I will get some tests added and check your comments in more detail." |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
We have parked the decommission work at the moment. Anu approved this change, but I feel we need some unit tests around the changes. I have raised HDDS-3303 to log the fact we need some tests. I also raised HDDS-3304 for the "use INFINITY as a constant" suggestion. As this is only going onto a branch which we will need to review in more detail later, I propose we commit this to the branch as is to get it out of the PR queue and leave the decommission work parked in a good state. |
|
/ready |
github-actions
bot
dismissed
their stale review
Blocking review request is removed.
What changes were proposed in this pull request?
When the datanode state is change in SCMNodeManager a Datanode Command is created to transmit the new datanode state to the datanode. The datanode will then persist this state in the "datanodeID.yaml" file.
The current datanode.yaml file simply persists a DatanodeDetails object in YAML format. Therefore two new fields have been added to DatanodeDetails to support this:
persistedOpState,
persistedOpStateExpiryEpochSec
The first is the operational state and the second is the seconds from the epoch the setting should expire (for maintenance mode).
When a datanode starts up, it will read these values and supply them in the heartbeat to SCM.
If SCM notices the DN was registered before, SCM is seen as the source of truth and will update the DN state to that set in SCM if they differ.
If the DN was never registered with SCM (eg if SCM was restarted), then the DN state is seen as the source of truth and SCM will be updated accordingly.
This change also moves the state expiry time into the NodeStatus object, so it now carries operationalState, healthState and State expiry.
Note that the DN does not do anything with the persisted operational state. They are simply persisted and reported in the heartbeat. In this respect the DNs are being used as a form of distributed storage for the operational state so the decommission process can survive a restart.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2592
How was this patch tested?
Tested manually using a docker build. This change still needs unit tests added if the current approach is agreed upon.