[
https://issues.apache.org/jira/browse/DRILL-7583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17037050#comment-17037050
]
ASF GitHub Bot commented on DRILL-7583:
---------------------------------------
ihuzenko commented on pull request #1981: DRILL-7583: Remove STOP status from
operator outcome
URL: https://github.com/apache/drill/pull/1981#discussion_r379430005
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
##########
@@ -386,52 +398,52 @@ protected void buildSchema() {
* Prefetches the first build side data holding batch.
*/
private void prefetchFirstBuildBatch() {
- rightUpstream = prefetchFirstBatch(rightUpstream,
- prefetchedBuild,
- buildSideIsEmpty,
- RIGHT_INDEX,
- buildBatch,
- () -> {
- batchMemoryManager.update(RIGHT_INDEX, 0, true);
- RecordBatchStats.logRecordBatchStats(RecordBatchIOType.INPUT_RIGHT,
- batchMemoryManager.getRecordBatchSizer(RIGHT_INDEX),
- getRecordBatchStatsContext());
- });
+ rightUpstream = prefetchFirstBatch(rightUpstream, prefetchedBuild,
+ buildSideIsEmpty, RIGHT_INDEX, buildBatch, () -> {
+ batchMemoryManager.update(RIGHT_INDEX, 0, true);
+ RecordBatchStats.logRecordBatchStats(RecordBatchIOType.INPUT_RIGHT,
+ batchMemoryManager.getRecordBatchSizer(RIGHT_INDEX),
+ getRecordBatchStatsContext());
+ });
}
/**
* Prefetches the first build side data holding batch.
*/
private void prefetchFirstProbeBatch() {
- leftUpstream = prefetchFirstBatch(leftUpstream,
- prefetchedProbe,
- probeSideIsEmpty,
- LEFT_INDEX,
- probeBatch,
- () -> {
- batchMemoryManager.update(LEFT_INDEX, 0);
- RecordBatchStats.logRecordBatchStats(RecordBatchIOType.INPUT_LEFT,
- batchMemoryManager.getRecordBatchSizer(LEFT_INDEX),
- getRecordBatchStatsContext());
- });
+ leftUpstream = prefetchFirstBatch(leftUpstream, prefetchedProbe,
+ probeSideIsEmpty, LEFT_INDEX, probeBatch, () -> {
+ batchMemoryManager.update(LEFT_INDEX, 0);
+ RecordBatchStats.logRecordBatchStats(RecordBatchIOType.INPUT_LEFT,
+ batchMemoryManager.getRecordBatchSizer(LEFT_INDEX),
+ getRecordBatchStatsContext());
+ });
}
/**
- * Used to fetch the first data holding batch from either the build or probe
side.
- * @param outcome The current upstream outcome for either the build or probe
side.
- * @param prefetched A flag indicating if we have already done a prefetch of
the first data holding batch for the probe or build side.
- * @param isEmpty A flag indicating if the probe or build side is empty.
- * @param index The upstream index of the probe or build batch.
- * @param batch The probe or build batch itself.
- * @param memoryManagerUpdate A lambda function to execute the memory
manager update for the probe or build batch.
- * @return The current {@link
org.apache.drill.exec.record.RecordBatch.IterOutcome}.
+ * Used to fetch the first data holding batch from either the build or probe
+ * side.
+ *
+ * @param outcome
+ * The current upstream outcome for either the build or probe side.
+ * @param prefetched
+ * A flag indicating if we have already done a prefetch of the first
+ * data holding batch for the probe or build side.
+ * @param isEmpty
+ * A flag indicating if the probe or build side is empty.
+ * @param index
+ * The upstream index of the probe or build batch.
+ * @param batch
+ * The probe or build batch itself.
+ * @param memoryManagerUpdate
+ * A lambda function to execute the memory manager update for the
+ * probe or build batch.
+ * @return The current
+ * {@link org.apache.drill.exec.record.RecordBatch.IterOutcome}.
*/
private IterOutcome prefetchFirstBatch(IterOutcome outcome,
- MutableBoolean prefetched,
- MutableBoolean isEmpty,
- int index,
- RecordBatch batch,
- Runnable memoryManagerUpdate) {
+ MutableBoolean prefetched, MutableBoolean isEmpty, int index,
Review comment:
Such shallow indentations may confuse readers since at first glance look
like method body started here. I would suggest
```java
private IterOutcome prefetchFirstBatch(IterOutcome outcome, MutableBoolean
prefetched,
MutableBoolean isEmpty, int index,
RecordBatch batch,
Runnable memoryManagerUpdate) {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
> Remove STOP status in favor of fail-fast
> ----------------------------------------
>
> Key: DRILL-7583
> URL: https://issues.apache.org/jira/browse/DRILL-7583
> Project: Apache Drill
> Issue Type: Improvement
> Affects Versions: 1.17.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
> Priority: Major
> Fix For: 1.18.0
>
>
> The original error solution was a complex process of a) setting a failed
> flag, b) telling all upstream operators they have failed, c) returning a
> {{STOP}} status. Drill has long supported a "fail-fast" error path based on
> throwing an exception; relying on the fragment executor to clean up the
> operator stack. Recent revisions have converted most operators to use the
> simpler fail-fast strategy based on throwing an exception instead of using
> the older {{STOP}} approach. This change simply removes the old, now-unused
> {{STOP}} based path.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
