[GitHub] [bookkeeper] michaeljmarshall commented on issue #2506: Autorecovery Process Tries to Connect to Bookie that Cleanly Exited Cluster

[GitBox <gi...@apache.org>]

michaeljmarshall commented on issue #2506:
URL: https://github.com/apache/bookkeeper/issues/2506#issuecomment-775732839


   @eolivelli - I took a look at this a bit, and I have some questions about design. 
   
   In order to make this code easiest to test, I think it makes sense to include a list of `badBookies` instead of `availableBookies`. In looking at the available methods in the bookkeeper admin, there is `getAvailableBookies` (which seems more like `getWritableBookies`), `getReadOnlyBookies`, and `getAllBookies`. I believe `getAllBookies` would include bookies that are unavailable but not yet decommissioned, but I haven't validated that yet. If it does include them, then the only way I currently see to `getUnavailableBookies` is to call all 3 methods and find the bookies that are in the "all" list but not in the other two.
   
   I think it might be reasonable to call all three methods to get the unavailable bookies, given that this code is not run that often, but I wanted to check before I looked into it more. Do you think it's okay to require three zk calls to get this state? The alternative is to add more state to zk, which I assume is a more cumbersome task.
   
   I can put up a tentative PR to show you what I mean, if that'd be helpful.
   
   Thanks for any insight you can give!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org