Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/75#discussion_r54645749
--- Diff:
core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java
---
@@ -427,11 +437,11 @@ public void updateBinningStats(int count, long time,
Map<String,TabletServerMuta
updateBatchStats(binnedMutations);
}
- private synchronized void
updateBatchStats(Map<String,TabletServerMutations<Mutation>> binnedMutations) {
- tabletServersBatchSum += binnedMutations.size();
+ private void
updateBatchStats(Map<String,TabletServerMutations<Mutation>> binnedMutations) {
+ tabletServersBatchSum.addAndGet(binnedMutations.size());
- minTabletServersBatch = Math.min(minTabletServersBatch,
binnedMutations.size());
- maxTabletServersBatch = Math.max(maxTabletServersBatch,
binnedMutations.size());
+ minTabletServersBatch.set(Math.min(minTabletServersBatch.get(),
binnedMutations.size()));
+ maxTabletServersBatch.set(Math.max(maxTabletServersBatch.get(),
binnedMutations.size()));
--- End diff --
This method of updating has a race condition. Multiple threads could call
get() before calling set(). Also all of these atomic vars require round trips
to main memory (not sure how much this matters). I can think of two possible
solutions. Both involve creating a BatchWriterStats class to make the code
more managable.
1. Could add a syncrhonized updateBatchStats method to BatchWriterStats.
No longer syncing on main lock or making lots of trips to main mem.
2. Could have an AtomicRef<BatchWriterStats>. To update batch writer
stats read the ref, clone it, make updates to clone, update ref using CAS to
ensure ref has not changed. If ref changed, then start over. This avoids
lock, race conditions, and lots of trips to main memory.
---
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.
---
