[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user eatoncys commented on the issue: https://github.com/apache/spark/pull/21084 @jerryshao @hvanhovell Ok, I will close it, thanks. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/21084 +0 as well. The downside to merging this is that it messes with the git blame. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user jerryshao commented on the issue: https://github.com/apache/spark/pull/21084 IIUC, null value will not be serialized (taskMemoryManager is only set in executor side), maybe Java will leave some footprints, but the overhead should be very small. I'm +0 to fix, seems no harm to the codebase. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user eatoncys commented on the issue: https://github.com/apache/spark/pull/21084 @jerryshao , There is not any issue without transient, but I think it is better to keep same to other fields, and make it clearly which fields do not need to be serialized. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user eatoncys commented on the issue: https://github.com/apache/spark/pull/21084 @jiangxb1987 It does not take significant time to serialize the taskMemoryManager, because the value is null in driver side, but I think it is better to keep same to other fields in the Task class, which are only used in executor side, like '@volatile @transient private var _reasonIfKilled: String = null'. In my test, the serialized size reduced from 8392 bytes to 8325 bytes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user jiangxb1987 commented on the issue: https://github.com/apache/spark/pull/21084 Does it really takes a significant time to serialize the `taskMemoryManager` ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user jerryshao commented on the issue: https://github.com/apache/spark/pull/21084 Seems OK to add `@transient`, but do you see any issue here without `transient`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21084 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89439/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21084 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21084 **[Test build #89439 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89439/testReport)** for PR 21084 at commit [`283e1ac`](https://github.com/apache/spark/commit/283e1ac91996e375cb9d1775fef39ea29eb85325). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user eatoncys commented on the issue: https://github.com/apache/spark/pull/21084 @hvanhovell The field 'taskMemoryManager' is only used in executor side, so it is not needed to serialize it when sending the task from driver to executor --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/21084 @eatoncys what issue are you trying to solve? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21084 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2380/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21084 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21084: [SPARK-23998][Core]It may be better to add @transient to...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21084 **[Test build #89439 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89439/testReport)** for PR 21084 at commit [`283e1ac`](https://github.com/apache/spark/commit/283e1ac91996e375cb9d1775fef39ea29eb85325). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
