accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From joshelser <...@git.apache.org>
Subject [GitHub] accumulo pull request #159: ACCUMULO-1280: many changes for closing iterator...
Date Fri, 18 Nov 2016 21:35:16 GMT
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/159#discussion_r88744008
  
    --- Diff: core/src/main/java/org/apache/accumulo/core/iterators/SortedKeyValueIterator.java
---
    @@ -147,4 +147,13 @@
        *              if not supported.
        */
       SortedKeyValueIterator<K,V> deepCopy(IteratorEnvironment env);
    +
    +  /**
    +   * Closes the Iterator. This should be overridden by the implementing class that has
access to <tt>SortedKeyValueIterator source</tt> provided in the
    --- End diff --
    
    > They are very situational so there never seems to be a consistent way to handle them
    
    Well, we need to figure this out. We *need* to understand how the framework is supposed
to work. If that depends on how the context in how the iterators are being invoked (which
I really don't think it should), we need to encapsulate that somewhere because users will
have the same question. Complicated or not, we must be clear.
    
    > In general, I think iterators should avoid throwing RuntimeException here. However,
they should probably pass through any coming from a source.close(), after they are done cleaning
up their own resources.
    
    Hrm. I think I see where you are coming from (if there's some DFSClient exception, we
want to propagate that). What if I channel my inner @dhutchis -- I have some iterator which
I'm expecting results to be sent to some external system when close() is called but I run
into some I/O related exception. Should the Scan/Compaction itself fail and be retried? Do
we say "oh well"?
    
    I'm starting to think that we really should have a design doc for this feature. I'm worried
about this making Iterators even more complicated than they already are...


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