accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From keith-turner <...@git.apache.org>
Subject [GitHub] accumulo pull request: ACCUMULO-1755: Modified TSBW so that all cl...
Date Tue, 01 Mar 2016 22:17:58 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message