Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-9266] Prevent "managed memory leak detected" exception from masking original exception #7603

Closed
wants to merge 3 commits into from

Conversation

JoshRosen
Copy link
Contributor

When a task fails with an exception and also fails to properly clean up its managed memory, the spark.unsafe.exceptionOnMemoryLeak memory leak detection mechanism's exceptions will mask the original exception that caused the task to fail. We should throw the memory leak exception only if no other exception occurred.

@JoshRosen JoshRosen changed the title [SPARK-9266] [SPARK-9266] Prevent "managed memory leak detected" exception from masking original exception Jul 22, 2015
@JoshRosen
Copy link
Contributor Author

Jenkins, retest this please.

@rxin
Copy link
Contributor

rxin commented Jul 23, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Jul 23, 2015

Test build #38129 has finished for PR 7603 at commit c1f0167.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class UnresolvedFunction(
    • case class Average(child: Expression) extends AlgebraicAggregate
    • case class Count(child: Expression) extends AlgebraicAggregate
    • case class First(child: Expression) extends AlgebraicAggregate
    • case class Last(child: Expression) extends AlgebraicAggregate
    • case class Max(child: Expression) extends AlgebraicAggregate
    • case class Min(child: Expression) extends AlgebraicAggregate
    • case class Sum(child: Expression) extends AlgebraicAggregate
    • abstract class AlgebraicAggregate extends AggregateFunction2 with Serializable
    • implicit class RichAttribute(a: AttributeReference)
    • trait AggregateExpression1 extends AggregateExpression
    • trait PartialAggregate1 extends AggregateExpression1
    • case class Min(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class MinFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class Max(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class MaxFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class Count(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class CountFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class CountDistinct(expressions: Seq[Expression]) extends PartialAggregate1
    • case class CollectHashSet(expressions: Seq[Expression]) extends AggregateExpression1
    • case class CombineSetsAndCount(inputSet: Expression) extends AggregateExpression1
    • case class Average(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class AverageFunction(expr: Expression, base: AggregateExpression1)
    • case class Sum(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class SumFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class CombineSum(child: Expression) extends AggregateExpression1
    • case class CombineSumFunction(expr: Expression, base: AggregateExpression1)
    • case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class SumDistinctFunction(expr: Expression, base: AggregateExpression1)
    • case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression1
    • case class First(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class FirstFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class Last(child: Expression) extends UnaryExpression with PartialAggregate1
    • case class LastFunction(expr: Expression, base: AggregateExpression1) extends AggregateFunction1
    • case class CreateArray(children: Seq[Expression]) extends Expression
    • case class CreateStruct(children: Seq[Expression]) extends Expression
    • case class CreateNamedStruct(children: Seq[Expression]) extends Expression
    • case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes
    • case class Aggregate2Sort(
    • case class FinalAndCompleteAggregate2Sort(
    • class GroupingIterator(
    • class PartialSortAggregationIterator(
    • class PartialMergeSortAggregationIterator(
    • class FinalSortAggregationIterator(
    • class FinalAndCompleteSortAggregationIterator(
    • abstract class UserDefinedAggregateFunction extends Serializable
    • case class ScalaUDAF(

@JoshRosen
Copy link
Contributor Author

Looks like the failed test might be an unrelated problem in master, but let's retest again just to be sure...

@JoshRosen
Copy link
Contributor Author

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Jul 23, 2015

Test build #38149 has finished for PR 7603 at commit c1f0167.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 23, 2015

Test build #38172 has finished for PR 7603 at commit c268cb5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Jul 23, 2015

I've merged this.

@asfgit asfgit closed this in ac3ae0f Jul 23, 2015
asfgit pushed a commit that referenced this pull request Jul 30, 2015
This pull request enables Unsafe mode by default in Spark SQL. In order to do this, we had to fix a number of small issues:

**List of fixed blockers**:

- [x] Make some default buffer sizes configurable so that HiveCompatibilitySuite can run properly (#7741).
- [x] Memory leak on grouped aggregation of empty input (fixed by #7560 to fix this)
- [x] Update planner to also check whether codegen is enabled before planning unsafe operators.
- [x] Investigate failing HiveThriftBinaryServerSuite test.  This turns out to be caused by a ClassCastException that occurs when Exchange tries to apply an interpreted RowOrdering to an UnsafeRow when range partitioning an RDD.  This could be fixed by #7408, but a shorter-term fix is to just skip the Unsafe exchange path when RangePartitioner is used.
- [x] Memory leak exceptions masking exceptions that actually caused tasks to fail (will be fixed by #7603).
- [x]  ~~https://issues.apache.org/jira/browse/SPARK-9162, to implement code generation for ScalaUDF.  This is necessary for `UDFSuite` to pass.  For now, I've just ignored this test in order to try to find other problems while we wait for a fix.~~ This is no longer necessary as of #7682.
- [x] Memory leaks from Limit after UnsafeExternalSort cause the memory leak detector to fail tests. This is a huge problem in the HiveCompatibilitySuite (fixed by f4ac642a4e5b2a7931c5e04e086bb10e263b1db6).
- [x] Tests in `AggregationQuerySuite` are failing due to NaN-handling issues in UnsafeRow, which were fixed in #7736.
- [x] `org.apache.spark.sql.ColumnExpressionSuite.rand` needs to be updated so that the planner check also matches `TungstenProject`.
- [x] After having lowered the buffer sizes to 4MB so that most of HiveCompatibilitySuite runs:
  - [x] Wrong answer in `join_1to1` (fixed by #7680)
  - [x] Wrong answer in `join_nulls` (fixed by #7680)
  - [x] Managed memory OOM / leak in `lateral_view`
  - [x] Seems to hang indefinitely in `partcols1`.  This might be a deadlock in script transformation or a bug in error-handling code? The hang was fixed by #7710.
  - [x] Error while freeing memory in `partcols1`: will be fixed by #7734.
- [x] After fixing the `partcols1` hang, it appears that a number of later tests have issues as well.
- [x] Fix thread-safety bug in codegen fallback expression evaluation (#7759).

Author: Josh Rosen <joshrosen@databricks.com>

Closes #7564 from JoshRosen/unsafe-by-default and squashes the following commits:

83c0c56 [Josh Rosen] Merge remote-tracking branch 'origin/master' into unsafe-by-default
f4cc859 [Josh Rosen] Merge remote-tracking branch 'origin/master' into unsafe-by-default
963f567 [Josh Rosen] Reduce buffer size for R tests
d6986de [Josh Rosen] Lower page size in PySpark tests
013b9da [Josh Rosen] Also match TungstenProject in checkNumProjects
5d0b2d3 [Josh Rosen] Add task completion callback to avoid leak in limit after sort
ea250da [Josh Rosen] Disable unsafe Exchange path when RangePartitioning is used
715517b [Josh Rosen] Enable Unsafe by default
@JoshRosen JoshRosen deleted the SPARK-9266 branch August 29, 2016 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants