cshannon commented on code in PR #4790:
URL: https://github.com/apache/accumulo/pull/4790#discussion_r1711448399
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletStateStore.java:
##########
@@ -52,6 +56,22 @@ public interface TabletStateStore extends
Iterable<TabletLocationState> {
@Override
ClosableIterator<TabletLocationState> iterator();
+ /**
+ * Create a stream of TabletLocationState that automatically closes the
underlying iterator.
+ */
+ default Stream<TabletLocationState> stream() {
+ ClosableIterator<TabletLocationState> iterator = this.iterator();
+ return StreamSupport
+ .stream(Spliterators.spliteratorUnknownSize(iterator,
Spliterator.ORDERED), false)
+ .onClose(() -> {
+ try {
+ iterator.close();
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to close iterator", e);
Review Comment:
A couple things with the try/catch block here:
1. I think it would make sense to just catch `IOException` only and not all
exceptions. Other runtime exceptions can just bubble up normally and then when
rethrowing it can be more specific and just use `UncheckedIOException`
2. I think the approach here to re-throw the exception as a runtime error is
probably the best way to handle it, but I know in some cases we have just
logged a warning when close fails and continue. If we are closing we may not
care if there is an exception as we are closing and done using it anyways.
@keith-turner - what do you think about re-throwing the checked exception
here vs just logging in this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
