You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hvanhovell <gi...@git.apache.org> on 2017/09/27 14:02:52 UTC
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
GitHub user hvanhovell opened a pull request:
https://github.com/apache/spark/pull/19367
[SPARK-22143][SQL] Fix memory leak in OffHeapColumnVector
## What changes were proposed in this pull request?
`WriteableColumnVector` does not close its child column vectors. This can create memory leaks for `OffHeapColumnVector` where we do not clean up the memory allocated by a vectors children. This can be especially bad for string columns (which uses a child byte column vector).
## How was this patch tested?
I have updated the existing tests to always use both on-heap and off-heap vectors. Testing and diagnoses was done locally.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hvanhovell/spark SPARK-22143
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19367.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19367
----
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19367
Merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82248/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82247/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82248 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82248/testReport)** for PR 19367 at commit [`84caf03`](https://github.com/apache/spark/commit/84caf03375f905d98f3bfe318487f1069077c96e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82251 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82251/testReport)** for PR 19367 at commit [`84caf03`](https://github.com/apache/spark/commit/84caf03375f905d98f3bfe318487f1069077c96e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141383477
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -38,31 +38,45 @@ import org.apache.spark.unsafe.types.CalendarInterval
class ColumnarBatchSuite extends SparkFunSuite {
- def allocate(capacity: Int, dt: DataType, memMode: MemoryMode): WritableColumnVector = {
+ private def allocate(capacity: Int, dt: DataType, memMode: MemoryMode): WritableColumnVector = {
if (memMode == MemoryMode.OFF_HEAP) {
new OffHeapColumnVector(capacity, dt)
} else {
new OnHeapColumnVector(capacity, dt)
}
}
- test("Null Apis") {
- (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
- val reference = mutable.ArrayBuffer.empty[Boolean]
+ private def testVector(
+ name: String,
+ size: Int,
+ dt: DataType,
+ modes: MemoryMode*)(
--- End diff --
Can we do `modes: Seq[MemoryMode] = Seq(MemoryMode.ON_HEAP, MemoryMode.ON_HEAP))(`? Then, we can avoid repeat `testVector(..., MemoryMode.ON_HEAP, MemoryMode.OFF_HEAP)` at each suite.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82247 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82247/testReport)** for PR 19367 at commit [`6156758`](https://github.com/apache/spark/commit/615675842aee6eeadb883f55e5933775fa5ec959).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141365149
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -718,62 +705,69 @@ class ColumnarBatchSuite extends SparkFunSuite {
column.putArray(0, 0, array.length)
assert(ColumnVectorUtils.toPrimitiveJavaArray(column.getArray(0)).asInstanceOf[Array[Int]]
=== array)
- }}
}
test("toArray for primitive types") {
- // (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
- (MemoryMode.ON_HEAP :: Nil).foreach { memMode => {
+ (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
--- End diff --
We can remove a pair of brace like `.foreach { memMode => ...`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141369722
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnVectorSuite.scala ---
@@ -25,19 +25,25 @@ import org.apache.spark.sql.types._
import org.apache.spark.unsafe.types.UTF8String
class ColumnVectorSuite extends SparkFunSuite with BeforeAndAfterEach {
-
- var testVector: WritableColumnVector = _
-
- private def allocate(capacity: Int, dt: DataType): WritableColumnVector = {
- new OnHeapColumnVector(capacity, dt)
+ private def withVector(
+ vector: WritableColumnVector)(
+ block: WritableColumnVector => Unit): Unit = {
+ try block(vector) finally vector.close()
}
- override def afterEach(): Unit = {
- testVector.close()
+ private def testVectors(
+ name: String,
+ size: Int,
+ dt: DataType)(
+ block: WritableColumnVector => Unit): Unit = {
+ test(name) {
+ val c1 = new OnHeapColumnVector(size, dt)
--- End diff --
What is `c1` for?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141356477
--- Diff: sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/OffHeapColumnVector.java ---
@@ -85,6 +85,7 @@ public long nullsNativeAddress() {
@Override
public void close() {
+ super.close();
Platform.freeMemory(nulls);
--- End diff --
This comment is not related to this fix directly. However, is it better to check whether each field is not `0` before calling `Platform.freeMemory()`? For example, `data` or `lengthData/offsetData may be `0`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82251 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82251/testReport)** for PR 19367 at commit [`84caf03`](https://github.com/apache/spark/commit/84caf03375f905d98f3bfe318487f1069077c96e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141360887
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnVectorSuite.scala ---
@@ -25,19 +25,25 @@ import org.apache.spark.sql.types._
import org.apache.spark.unsafe.types.UTF8String
class ColumnVectorSuite extends SparkFunSuite with BeforeAndAfterEach {
-
- var testVector: WritableColumnVector = _
-
- private def allocate(capacity: Int, dt: DataType): WritableColumnVector = {
- new OnHeapColumnVector(capacity, dt)
+ private def withVector(
+ vector: WritableColumnVector)(
+ block: WritableColumnVector => Unit): Unit = {
+ try block(vector) finally vector.close()
}
- override def afterEach(): Unit = {
- testVector.close()
+ private def testVectors(
+ name: String,
+ size: Int,
+ dt: DataType)(
+ block: WritableColumnVector => Unit): Unit = {
+ test(name) {
+ val c1 = new OnHeapColumnVector(size, dt)
--- End diff --
What's this `c1` for?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141366208
--- Diff: sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/OffHeapColumnVector.java ---
@@ -85,6 +85,7 @@ public long nullsNativeAddress() {
@Override
public void close() {
+ super.close();
Platform.freeMemory(nulls);
--- End diff --
Yeah, it is a bit weird. The doc in the JDK8's `Unsafe. freeMemory()` states the following: `The address passed to this method may be null, in which case no action is taken.` See: http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/misc/Unsafe.java
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19367
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82248 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82248/testReport)** for PR 19367 at commit [`84caf03`](https://github.com/apache/spark/commit/84caf03375f905d98f3bfe318487f1069077c96e).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82243 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82243/testReport)** for PR 19367 at commit [`4b494c5`](https://github.com/apache/spark/commit/4b494c5ffe0ca1c4d5c5a3d52335450492ba8c3c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141381707
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnVectorSuite.scala ---
@@ -181,9 +177,8 @@ class ColumnVectorSuite extends SparkFunSuite with BeforeAndAfterEach {
assert(array.get(3, arrayType).asInstanceOf[ArrayData].toIntArray() === Array(3, 4, 5))
}
- test("struct") {
- val schema = new StructType().add("int", IntegerType).add("double", DoubleType)
- testVector = allocate(10, schema)
+ val structType: StructType = new StructType().add("int", IntegerType).add("double", DoubleType)
--- End diff --
nit: Do we need `: StructType`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82243 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82243/testReport)** for PR 19367 at commit [`4b494c5`](https://github.com/apache/spark/commit/4b494c5ffe0ca1c4d5c5a3d52335450492ba8c3c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19367
cc @ala @michal-databricks @ueshin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141370506
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -38,31 +38,45 @@ import org.apache.spark.unsafe.types.CalendarInterval
class ColumnarBatchSuite extends SparkFunSuite {
- def allocate(capacity: Int, dt: DataType, memMode: MemoryMode): WritableColumnVector = {
+ private def allocate(capacity: Int, dt: DataType, memMode: MemoryMode): WritableColumnVector = {
if (memMode == MemoryMode.OFF_HEAP) {
new OffHeapColumnVector(capacity, dt)
} else {
new OnHeapColumnVector(capacity, dt)
}
}
- test("Null Apis") {
- (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
- val reference = mutable.ArrayBuffer.empty[Boolean]
+ private def testVector(
+ name: String,
+ size: Int,
+ dt: DataType,
+ modes: MemoryMode*)(
+ block: (WritableColumnVector, MemoryMode) => Unit): Unit = {
+ test(name) {
+ modes.foreach { mode =>
+ val vector = allocate(size, dt, mode)
+ try block(vector, mode) finally {
--- End diff --
The way you use `{}` here is weird.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19367
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82251/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141371354
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -422,17 +420,14 @@ class ColumnarBatchSuite extends SparkFunSuite {
assert(v._1 == Platform.getLong(null, addr + 8 * v._2))
}
}
- }}
}
- test("Float APIs") {
- (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
+ testVector("Float Apis", 1024, FloatType, MemoryMode.ON_HEAP, MemoryMode.OFF_HEAP) {
--- End diff --
I think changing `Apis` to `APIs` would be better than the other way around.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82243/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19367
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141372248
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -829,13 +823,15 @@ class ColumnarBatchSuite extends SparkFunSuite {
assert(column.getArray(2).getArray(1).getInt(1) === 4)
assert(column.getArray(2).getArray(1).getInt(2) === 5)
assert(column.isNullAt(3))
- }
}
- test("Nest Struct in Array.") {
- (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode =>
- val schema = new StructType().add("int", IntegerType).add("long", LongType)
- val column = allocate(10, new ArrayType(schema, true), memMode)
+ private val structType: StructType = new StructType().add("i", IntegerType).add("l", LongType)
+
+ testVector(
+ "Nest Struct in Array",
+ 10,
+ new ArrayType(new StructType().add("int", IntegerType).add("long", LongType), true),
--- End diff --
Did you mean `new ArrayType(structType)`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141382923
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnVectorSuite.scala ---
@@ -181,9 +177,8 @@ class ColumnVectorSuite extends SparkFunSuite with BeforeAndAfterEach {
assert(array.get(3, arrayType).asInstanceOf[ArrayData].toIntArray() === Array(3, 4, 5))
}
- test("struct") {
- val schema = new StructType().add("int", IntegerType).add("double", DoubleType)
- testVector = allocate(10, schema)
+ val structType: StructType = new StructType().add("int", IntegerType).add("double", DoubleType)
--- End diff --
It is a class level field now, so scalastyle wants me type it...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColu...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19367#discussion_r141365196
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala ---
@@ -718,62 +705,69 @@ class ColumnarBatchSuite extends SparkFunSuite {
column.putArray(0, 0, array.length)
assert(ColumnVectorUtils.toPrimitiveJavaArray(column.getArray(0)).asInstanceOf[Array[Int]]
=== array)
- }}
}
test("toArray for primitive types") {
- // (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
- (MemoryMode.ON_HEAP :: Nil).foreach { memMode => {
+ (MemoryMode.ON_HEAP :: MemoryMode.OFF_HEAP :: Nil).foreach { memMode => {
+ // (MemoryMode.ON_HEAP :: Nil).foreach { memMode => {
--- End diff --
Can we remove this line?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19367: [SPARK-22143][SQL] Fix memory leak in OffHeapColumnVecto...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19367
**[Test build #82247 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82247/testReport)** for PR 19367 at commit [`6156758`](https://github.com/apache/spark/commit/615675842aee6eeadb883f55e5933775fa5ec959).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org