Aaron, if we cannot commit based on unit tests then the process is broken. I do not want to waste time bringing up cluster and manually testing it, if possible.
Most of the time I would agree with you, but these sorts of changes aren't well covered by existing tests.
Since I'm tired of arguing about manual testing, I just tested this patch manually on a cluster myself, and it appears to have broken haadmin. In particular, `hdfs haadmin -failover ...' always prints "Failover failed: Can't failover to an active service" and never causes an NN failover, even if both NNs are in the Standby state. If I revert this patch, `hdfs haadmin -failover ...' works again. `hdfs haadmin -getServiceState' appears to always return 'Initializing' with this patch applied, but works as intended without this patch. `hdfs haadmin -transitionToActive ...' and `hdfs haadmin -transitionToStandby ...' do appear to work with or without this patch.
And for the record, it took me about 20 minutes to test this on a cluster.