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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
