Github user dhutchis commented on the pull request:
https://github.com/apache/accumulo/pull/96#issuecomment-215784988
> Instead of trying to wrap things up so that these classes can
pseudo-safely hold references to instances of `TabletLocator`
The current PR does allow any class to safely hold a reference to a
`TabletLocator`. It is simple and it works. Don't you think it is better to
permit classes the privilege of holding a reference? The Writer and Reader
classes locally cache TabletLocators because they write many entries to the
same tablets from multiple threads, which could create lock contention if they
accessed the static locators every time.
> While I'm dubious that it'd be a real concern, if we felt that this would
have significant performance impact (due to lock contention), then it should be
possible to partially ameliorate the hit by using a Guava Cache or even a
ConcurrentHashMap.
Yes, there are two items under discussion. The first is to allow a class
to hold a reference to a TabletLocator without syncing issues caused by
clearing of the static locators. The second is to make locators into a Guava
cache (or ConcurrentMap), which eliminates the synchronized restriction on the
TabletLocator methods.
The second item is not related to the original bug found for ACCUMULO-4229;
rather, it is an opportunity we found while thinking about the first item. We
could ignore it or act on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
