Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/838#discussion_r117535728
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
---
@@ -213,17 +213,16 @@ public IterOutcome next() {
try {
currentReader.allocate(mutator.fieldVectorMap());
} catch (OutOfMemoryException e) {
- logger.debug("Caught OutOfMemoryException");
clearFieldVectorMap();
- return IterOutcome.OUT_OF_MEMORY;
+ throw UserException.memoryError(e).build(logger);
}
addImplicitVectors();
} catch (ExecutionSetupException e) {
- this.context.fail(e);
--- End diff --
Throwing an exception, it turns out, does exactly the same: it cancels the
query and causes the fragment executor to cascade close() calls to all the
operators (record batches) in the fragment tree. It seems some code kills the
query by throwing an exception, other code calls the fail method and bubbles up
STOP. But, since the proper way to handle STOP is to unwind the stack, STOP is
equivalent to throwing an exception.
The idea is, rather than have two ways to clean up, let's standardize on
one. Since we must handle unchecked exceptions in any case, the exception-based
solution is the logical choice for standardization.
---
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.
---
