What's the method to re-trigger a check, eg if the disk is onlined again? Restart the TT, I guess?

I too have that concern of "TT not handling the case of 'bad disks becoming good'".

BTW, How expensive is this check in DiskChecker.checkDirs() ? This is not being called for bad local dirs in the current code of TT.offerService(). LocalDirAllocator object being held by TaskController is also updated by TT.initialize(). We need to see if we can update all LocalDirAllocator objects with good local dirs list, if we are not doing that already. Will it be good enough here and for HADOOP-7551 ?

@Todd - Yes, to re-trigger you need to restart the TT. This is how the code currently works - once a directory is removed from LocalStorage's "good list" it is never put back while the TT is running, ie once a dir is identified as bad it won't be used by the TT. LocalDirAllocator#confChanged tries to notice when a new dir is added to the conf but we don't add new MR local dirs at runtime so this feature isn't used. Per HADOOP-7551 LocalDirAllocator (common) and LocalStorage (mr) are currently independent but should be aware of each other.

@Ravi LocalDirAllocator already keeps track of the valid dirs itself. Once there is a bad dir LocalDirAllocator#confChanged executes for every call to get a local directory, it's this code that calls checkDirs on each local directory. It turns out the version of checkDirs that doesn't take a permissions parameter is not as expensive as I thought (the method that takes a permission forks a call to ls for each directory which is expensive). However confChanged creates a new DF object for each local dir which has the side effect of resetting the df interval which means forking a call to df instead of caching the last result when LocalDirAllocator uses each DF.

In short, I think it's expensive if the configured dirs are different from the list of valid dirs maintained by LocalDirAllocator. If we remove bad dirs from the conf in the TT then they won't differ. Alternatively, we could modify LocalDirAllocator to ignore bad directories but that would conflict with its current design that explicitly tries to notice a difference between the set of valid and configured dirs.

LocalDirAllocator#AllocatorPerContext#confChanged() is actually updating the savedLocalDirs everytime conf is changed. So every call to confChanged() is not resulting into disk checks (except once per configuration change i.e. once per every bad disk). Right ?

Updating savedLocalDirs is the reason why it runs through the disk checking code each time. The disk checking is done when newLocalDirs != savedLocalDirs, and because saveLocalDirs has the bad dirs removed and newLocalDirs is retrieved from the conf (and per this jira we don't modify the conf) we will run the disk checking code every time confChanged is called. Make sense?

AFAIK TT is modifying the conf object(s) based on disks' health. If you see some cases where we are missing updating conf, we can fix them too. Fine ?

You're right, I missed that we're always updating the conf the job is launched with the latest known good dirs in TT#launchTaskForJob. Thanks for the explanation. I verified we weren't missing other locations by failing a local dir, logging in the confChanged case and verifying that we only notice the change once.