impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5282: Handle overflows in computeResourceProfile().
Date Mon, 05 Jun 2017 22:09:31 GMT
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 <alex.behm@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message