1) The hashmap for delegation tokens could use the JobID object as the key
yes it could. it could use jobid.toString() too.
2) I don't see a good motivation for having equals and hashcode implementations in the private class DelegationTokenToRenew. The implementations can be improved as well but I don't see a strong reason for introducing them in this patch.
Equals is needed to for contains() to work correctly (comparing actual Tokens instead of DelegationTokenToRenew.
3) The initial value of newExpirationDate could be 60 minutes. I don't see the need for initializing it to -1 and then setting it to some other value based on that.
This setting is protecting from unexpected/erroneous returns from dfs.renewDelegationToken().
4) In removeDelegationTokenRenewal, the checks for the jobid for the tokens is redundant. The hashmap already provides you with that list.
5) The DelegationTokenToRenew class doesn't need to store the jobID at all. Everywhere the jobID could be passed as argument to the methods where it is required.
This is how it is passed around.
6) When would alreadyInMap ever return true? If it never does, i suggest we remove this check.
This is to protect from erroneous calls, to avoid same DelegationToken to be added twice.
7) You haven't synchronized on the accesses to delegationTokens object everywhere. Maybe, a better approach would be to just define the object as a synchronized.
Well, the only non-protected write access is with clear() method in close() which is called when JT is shutting down.
But, just to make the things safer, I will make the map synchronized.