impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5282: Handle overflows in computeResourceProfile().
Date Mon, 05 Jun 2017 22:38:47 GMT
Alex Behm 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 
Done


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
I agree that this try/catch is a crutch for missing tests.
My understanding is that before the computePerHostResources() change we did not rely on the
estimates that much.

My fear is that we increased our reliance on these estimates and will break someone's workload
due to overflow or similar bugs like this one.

If the computePerHostResources() change is not safe, we should consider backing it out of
2.9, and put it into the next release once we have high confidence (with tests) that we will
not break existing workloads.


-- 
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: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message