accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dhutchis <...@git.apache.org>
Subject [GitHub] accumulo pull request: ACCUMULO-4229 BatchWriter Locator cache out...
Date Sun, 24 Apr 2016 19:53:46 GMT
Github user dhutchis commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/96#discussion_r60850768
  
    --- Diff: core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java
---
    @@ -329,6 +331,8 @@ private void processFailures(Map<KeyExtent,List<Range>>
failures, ResultReceiver
     
         // since the first call to binRanges clipped the ranges to within a tablet, we should
not get only
         // bin to the set of failed tablets
    +    if (!locator.isValid())
    +      locator = new TimeoutTabletLocator(TabletLocator.getLocator(instance, new Text(table)),
timeout);
    --- End diff --
    
    I see what you are saying @wjsl -- various QueryTask threads could potentially read that
their locators are not valid and then all replace the locator.  If n threads try to do this
with unfortunate timing, then there are n-1 creations of a new TimeoutTabletLocator that are
thrown away.  It's not a particularly bad problem since the end result is correct no matter
what, but it would be nice to guard against it.
    
    What do you think of this:
    
    ```java
    if (!locator.isValid())
      synchronized (TabletServerBatchReaderIterator.this) {
        if (!locator.isValid())
          locator = new TimeoutTabletLocator(TabletLocator.getLocator(instance, new Text(table)),
timeout);
      }
    ```
    
    I would do something similar for `ConditionalWriterImpl`. The `TabletServerBatchWriter`
is safe.


---
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