meszibalu commented on pull request #2685: URL: https://github.com/apache/hbase/pull/2685#issuecomment-731377478
> Please make the connections member variable in AbstractRpcConnection private and include a comment above it that all access must be synchronized. (at least on glance it doesn’t look like anything outside the class needs the current protected access modifier.) It is thread-safe again. > Does `PoolMap` actually need to implement `java.util.Map`? PoolMap does not look general purpose and this deviation from the interface contract makes it harder to look for correctness problems. I’d like to see us define the expected behavior for the methods we actually use. I interpreted this comment as we can do a bigger refactor. `PoolMap` does not implement `Map` anymore. With the reduced functionality it was easier to properly ensure thread safeness. ---------------------------------------------------------------- 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: [email protected]
