Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5282: Handle overflows in computeResourceProfile().
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7084/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

Line 993:     if (perInstanceMemEstimate < 0) perInstanceMemEstimate = 
Long.MAX_VALUE;
Should we do something similar to multiplyCardinalities() to encourage use of 
this pattern? multiplyBytes()?


http://gerrit.cloudera.org:8080/#/c/7084/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

Line 1019:     try {
Continuing without resource estimates set isn't safe. The scheduler assumes in 
GetPerHostMemoryEstimate() that a memory estimate is available.

Aside from that specific problem, it seems risky in general to try to continue 
when we've already hit a bug and are in somewhat unknown waters.

I think it's probably unwise in general to try to mask bugs here, but if we do 
that we should instead do this in computeResourceReqs() so that we actually set 
the values that are expected to be present.


-- 
To view, visit http://gerrit.cloudera.org:8080/7084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a83a76141853d3274f812e5a531f456e1b110b1
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to