0. Nice work on removing the STOPPED state. Makes the system less complex.
1. In TestDecommission.waitNodeState, we wait for half second per iteration.
If this setting causes many prints of the form "Waiting for node to change.."
then maybe we can change the wait period to 1 second instead of half second.
2. In TestDecommission, we have the following logic:
I guess b. should be waitNodeState(DECOMMISSIONED)
Also, we can sneak in a call to checkFile() betwen steps b and c.
3. Maybe ClientProtocol.refreshNodes can return a void instead of
4. The original way to shutdown a datanode was to make the namenode send a
DNA_SHUTDOWN command in response to a heartbeat. You enahnced this logic
to make the datanode catch a UnregisteredDatanodeException and shut itself
down. Thus, we will now have two ways to shutdown a datanode. Do you think
that it is preferable to have only one way to shutdown a datanode?
5. An earlier patch introduced an async thread called the ReplicationMonitor.
The ReplicationMonitor thread invokes checkDecommissionState(). This
probably means that the DecommissionedMonitor thread is not needed anymore.
6. The FSNamesystem.verifyNodeRegistration needs to be synchronized since it
traverses hosts lists and datanode lists. Since
FSNamesystem.startDecommission and FSNamesystem.stopDecommission are private
and are always called from already-synchronized methods, we can remove
the "synchronized" keyword from their definitions.
7. I am kind-of reluctant to put in added complexity to pendingTransfers
to handle the case that a decommissioned node should not be the
source of a replication request. Especially because the above rule is
not enforced if there is only one replica in the cluster. Is there any
way that we can avoid putting in this special purpose case? What bad
thing can occur if we select the decommissioned node as the source of
a replication request?
8. Ideally, the FSNamesystem.verifyNodeShutdown needs to be synchronized
because it looks up the datanode descriptor. But this method is called
for every RPC request and a "synchronized" call just adds overhead.
Maybe we can make FSNamesystem.gotHeartBeat, FSNamesystem.processReport
and FSNamesystem.blockReceived call FSNamesystem.verifyNodeShutdown.