Improve bad tserver handling code

[keith-turner]

The Accumulo master has some code that kills bad servers. This code caused #1456. While reviewing a fix to lessen the possible bite of this code, the question was raised what positive value does the code have? Need to research this in order to make an informed decision about the future direction of this code.

[keith-turner]

Looking into this code a bit, below are some potential problems.

1. When there is a problem connecting to a tserver, it tries to connect to the tserver to ask it to halt.
2. Since the code for dealing with potentially bad tservers is in the code path for getting stats, it could potentially impede getting status from functioning tservers.
3. The code uses a hard coded count of the number of times a tserver could not be connected to decide if a tserver is bad.
4. The code has no concept of the big picture, if there is 1/1000 bad server vs 999/1000. Each bad tserver is handled independently. The rate limiting changes made in Prevent catastrophic tserver shutdown by restricting the status shutdown #1456 dealt with this in a way.

I think there is value in what the code is attempting to do (react to tservers that the manager can not connect to), but it could be improved. Accumulo tservers must maintain a heartbeat with a ZK server inorder to be considered alive. There are situations where a tservers is maintaining this ZK heartbeat, but new connections can not be made to the tserver. When the manager attempts to get status from a tserver, it does not currently use a pooled connection. If the manager can not form a new connection, then clients likely can not either.

One possible improvement that could be made to address problems above is the creation of an internal service in the manager to deal with bad tservers. The code that gathers status would queue tservers it had problems connecting with to this service. The service would decide when to react and have its own threads for reacting. The service can consider the big picture and queuing something for it to process does not need to block gathering status.

For problem 3, the hard coded count, this criteria for classifying a tserver as bad does not seem ideal. Seems like a configurable criteria based on time and count would be better. Like if at least Y attempt to contact a tserver in a X minute period failed, classify it as bad.

For problem 1, asking the tserver to halt, if the manager can not connect to the tserver to get status then its chances of connecting to the tserver to ask it to halt are diminished. May be better to delete its ZK lock and/or ask it to halt.

Overall, I am thinking there is value in what the code is attempting to do and that it needs to be improved.

[keith-turner]

I removed the blocker label, I don't think the code should be removed even though it does need improvement. The improvements needed are not well defined and therefore should not block a release.