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 [email protected] or file a JIRA ticket
with INFRA.
---
