accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ACCUMULO-3555) TabletServerBatchReaderIterator doesn't maintain reference to TabletServerBatchReader
Date Tue, 03 Feb 2015 16:30:34 GMT

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

Josh Elser commented on ACCUMULO-3555:
--------------------------------------

Yup, you hit on lots of good stuff, [~dlmarion]. 

bq. TSBR.iterator() creates a new threadpool for each invocation

I don't think we have to do this. I don't think it's entirely clear how the {{numThreads}}
argument on the createBatchScanner call actually works WRT multiple calls to iterator(), so
it will be all the more important to not change those semantics. Ultimately, the numThreads
controls the size of the thread pool used by any iterators created from that BS. I don't see
a reason why we would have to provide any more isolation -- each runnable submitted to the
pool should be isolated already.

Your original code snippet does bring up a good concern with what Bill noted:

{code}
BatchScanner bs = conn.createBatchScanner(tableName, Authorizations.EMPTY, 4);
bs.setRanges(Collections.singleton(new Range()));
Iterator<Entry<Key,Value>> iterator = bs.iterator();
doThings(iterator);
....
{code}

After exhausting the first iterator, we don't know if the user might create more iterators
using that BS *unless* we get the finalize() call from the JVM. So, perhaps, if we know that
the BS has been reaped (finalize was called), and all iterators created from that BS have
been exhausted, then we could auto-close the BS. The only change in semantics there would
be that finalize() wouldn't automatically close the BS as it has been doing since 1.5.0. I'm
not sure how that would fit in semver since that's more of a side-effect than a users' use
of the BS (but it's probably not worth it to talk about that at this point).

> TabletServerBatchReaderIterator doesn't maintain reference to TabletServerBatchReader
> -------------------------------------------------------------------------------------
>
>                 Key: ACCUMULO-3555
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-3555
>             Project: Accumulo
>          Issue Type: Bug
>          Components: client
>    Affects Versions: 1.5.0, 1.5.1, 1.5.2, 1.6.0, 1.6.1
>            Reporter: Josh Elser
>            Assignee: Josh Elser
>            Priority: Blocker
>
> Had a user in IRC run into this again today upgrading a 1.4 instance to 1.6.0.
> ACCUMULO-587 introduced a {{finalize}} implementation into {{TabletServerBatchReader}}
in an attempt to close the {{BatchScanner}} when the user might have forgotten to do so themselves.
The problem, however, is that the {{TabletServerBatchReaderIterator}} doesn't maintain a reference
to the {{TabletServerBatchReader}} (notice how it only uses it to create a new instnace of
{{ScannerOptions}} using the copy constructor).
> In other words, when the {{TabletServerBatchReaderIterator}} is constructed, it has no
references in the object graph to the {{TabletServerBatchReader}} it was created from. This
means that if clients don't hold onto the BatchScanner instance, it's possible that it gets
closed by the JVM calling {{finalize()}}.



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

Mime
View raw message