accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ACCUMULO-4229) BatchWriter Locator cache out-of-sync when shared with tserver
Date Fri, 29 Apr 2016 01:49:12 GMT

    [ https://issues.apache.org/jira/browse/ACCUMULO-4229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15263392#comment-15263392
] 

ASF GitHub Bot commented on ACCUMULO-4229:
------------------------------------------

Github user dhutchis commented on the pull request:

    https://github.com/apache/accumulo/pull/96#issuecomment-215609729
  
    The desired behavior is: remove a TabletLocator from the locators map one hour *after*
no other strong references to it exist. Reset the timer when a new thread accesses the TabletLocator.
    
    Sadly I don't see an easy way to implement this behavior.  
    
    1. If we use a timer with Guava, then a TabletLocator may be removed from the map if a
thread uses the TabletLocator for at least an hour, which could easily happen if the thread
locally caches the locator.
    2. If we use soft references, then TabletLocators may be removed from the map (in an LRU
manner) at any time there are no other references to them if the gc is pressured. This could
affect performance in edge cases.
    3. Using both a timer and soft references has both problems.
    
    The approach in this PR forces threads to re-fetch a TabletLocator from the map if the
locators in the map are cleared.  This happens once per hour and is done automatically by
SyncingTabletLocator.  This solves the problem on its own and may be good enough. 
    
    Perhaps the best solution is to combine it with the Guava timer (point 1 above): replacing
the locators map with a Guava cache, letting entries expire one hour after they are last accessed.
We may then remove the `synchronized` statements on TabletLocator.
    
    Think its worth going for, or shall we stick with this? 


> BatchWriter Locator cache out-of-sync when shared with tserver
> --------------------------------------------------------------
>
>                 Key: ACCUMULO-4229
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-4229
>             Project: Accumulo
>          Issue Type: Bug
>          Components: client
>    Affects Versions: 1.6.5, 1.7.1
>            Reporter: Dylan Hutchison
>            Assignee: Dylan Hutchison
>
> BatchWriters that run a long time have write rates that sometimes mysteriously decrease
after the table it is writing to goes through a major compaction or a split.  The decrease
can be as bad as reducing throughput to 0.
> This was first first mentioned in this [email thread|https://mail-archives.apache.org/mod_mbox/accumulo-user/201406.mbox/%3CCAMz+DuvmmHegOn9EJeHR9H_rRpP50L2QZ53BbdruVO0pirArQw@mail.gmail.com%3E]
for major compactions.  
> I discovered this in this [email thread|https://mail-archives.apache.org/mod_mbox/accumulo-dev/201604.mbox/%3CCAPx%3DJkaY7fVh-U0O%2Bysx2d98LOGMcA4oEQOYgoPxR-0em4hdvg%40mail.gmail.com%3E]
for splits.  See the thread for some log messages.
> I turned on TRACE logs and I think I pinned it down: the TabletLocator cached by a BatchWriter
gets out of sync with the static cache of TabletLocators.
> # The TabletServerBatchWriter caches a TabletLocator from the static collection of TabletLocators
when it starts writing.  Suppose it is writing to tablet T1.
> # The TabletServerBatchWriter uses its locally cached TabletLocator inside its `binMutations`
method for its entire lifespan; this cache is never refreshed or updated to sync up with the
static collection of TabletLocators.
> # Every hour, the static collection of TabletLocators clears itself.  The next call to
get a TabletLocator from the static collection allocates a new TabletLocator.  Unfortunately,
the TabletServerBatchWriter does not reflect this change and continues to use the old, locally
cached TabletLocator.
> # Tablet T1 splits into T2 and T3, which closes T1.  As such, it no longer exists and
the tablet server that receives the entries meant to go to T1 all fail to write because T1
is closed.
> # The TabletServerBatchWriter receives the response from the tablet server that all entries
failed to write.  It invalidates the cache of the *new* TabletLocator obtained from the static
collection of TabletLocators.  The old TabletLocator that is cached locally does not get invalidated.
> # The TabletServerBatchWriter re-queues the failed entries and tries to write them to
the same closed tablet T1, because it is still looking up tablets using the old TabletLocator.
> This behavior subsumes the circumstances William wrote about in the thread he mentioned.
 The problem would occur as a result of either splits or major compactions.  It would only
stop the BatchWriter if its entire memory filled up with writes to the same tablet that was
closed as a result of a majc or split; otherwise it would just slow down the BatchWriter by
failing to write some number of entries with every RPC.
> There are a few solutions we can think of.  
> # Not have the MutationWriter inside the TabletServerBatchWriter locally cache TabletLocators.
 I suspect this was done for performance reasons, so it's probably not a good solution. 
> # Have all the MutationWriters clear their cache at the same time the static TabletLocator
cache clears.  We could store a reference to the Map that each MutationWriter has inside a
static synchronized WeakHashMap.  The only time the weak map needs to be accessed is:
> ## When a MutationWriter is constructed (from constructing a TabletServerBatchWriter),
add its new local TabletLocator cache to the weak map.
> ## When the static TabletLocator cache is cleared, also clear every map in the weak map.
> # Another solution is to make the invalidate calls on the local TabletLocator cache rather
than the global static one.  If we go this route we should double check the idea to make sure
it does not impact the correctness of any other pieces of code that use the cache. 
> # Perhaps the simplest solution is to put an extra Boolean variable inside the Locators
indicating whether they are valid.  When they are cleared, their Boolean variables set to
false.  Before a client uses a locator from cache, it checks its Boolean indicator.
> The TimeoutTabletLocator does not help when no timeout is set on the BatchWriter (the
default behavior).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message