Github user ShawnWalker commented on the pull request:
https://github.com/apache/accumulo/pull/96#issuecomment-215765483
Instead of trying to wrap things up so that these classes can pseudo-safely
hold references to instances of `TabletLocator`, I feel that it would be much
simpler to establish the rule "thou shalt not hold a reference to this for an
extended period", and make changes appropriately.
By which I mean:
1. Refactor `ConditionalWriterImpl` to obtain a temporary `TabletLocator`
reference at the beginning of each method that needs it (`queue(...)`,
`sendToServer(...)`, `invalidateSession(...)`), instead of obtaining a single
reference in the constructor.
2. Modify `TimeoutTabletLocator` so that its constructor takes a
`ClientContext` and a tableId, like your current pull request already does.
But have each of it's methods invoke `TabletLocator.getLocator(...)` anew.
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`.
---
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.
---
