[GitHub] spark pull request #19640: [SPARK-16986][CORE][WEB-UI] Support configure his...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/19640#discussion_r149087915
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2742,6 +2742,11 @@ private[spark] object Utils extends Logging {
}
}
+ def getTimeZone: TimeZone = {
--- End diff --
Could you please image that this method will be used outside scope of the
current issue?
What's about users who does not have spark history at all?
The name of method, the class and the package does not tell us that this
method somehow related ONLY to history server, such API makes me easy to do
wrong assumptions.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19640: [SPARK-16986][WEB-UI] Replace GMT with history se...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/19640#discussion_r148750999
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala
---
@@ -426,6 +426,10 @@ class SparkHadoopUtil extends Logging {
ugi.getAuthenticationMethod() ==
UserGroupInformation.AuthenticationMethod.PROXY
}
+ def getTimeZone: TimeZone = {
+TimeZone.getTimeZone(sparkConf.get("spark.history.timeZone", "GMT"))
--- End diff --
It's a bit confusing using `spark.history.timeZone` property in getTimeZone
method of SparkHadoopUtil.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19640: [SPARK-16986][WEB-UI] Replace GMT with history se...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/19640#discussion_r148750407
--- Diff: core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala
---
@@ -426,6 +426,10 @@ class SparkHadoopUtil extends Logging {
ugi.getAuthenticationMethod() ==
UserGroupInformation.AuthenticationMethod.PROXY
}
+ def getTimeZone: TimeZone = {
--- End diff --
I would write it like
```
def timeZone: TimeZone =
TimeZone.getTimeZone(sparkConf.get("spark.history.timeZone", "GMT"))
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18934: [SPARK-21721][SQL] Clear FileSystem deleteOnExit ...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/18934#discussion_r133002478
--- Diff:
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala
---
@@ -423,7 +423,14 @@ case class InsertIntoHiveTable(
// Attempt to delete the staging directory and the inclusive files. If
failed, the files are
// expected to be dropped at the normal termination of VM since
deleteOnExit is used.
try {
- createdTempDir.foreach { path =>
path.getFileSystem(hadoopConf).delete(path, true) }
+ createdTempDir.foreach { path =>
+val fs = path.getFileSystem(hadoopConf)
+fs.delete(path, true)
--- End diff --
`FileSystem.delete returns true if delete is successful else false.` So it
makes sense to re-write a bit
```
if (fs.delete(path, true)) {
fs.cancelDeleteOnExit(path)
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #18940: YSPARK-734 Change CacheLoader to limit entries based on ...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/18940 LGTM, btw, no unit tests for the change? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17482: [SPARK-9002][CORE] KryoSerializer initialization does no...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17482 Thank you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17482: [SPARK-9002][CORE] KryoSerializer initialization does no...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17482 @srowen Is it all right if I ask you to merge the PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17482: [SPARK-9002][CORE] KryoSerializer initialization ...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17482#discussion_r109007592
--- Diff:
core/src/test/scala/org/apache/spark/serializer/KryoSerializerSuite.scala ---
@@ -76,6 +76,9 @@ class KryoSerializerSuite extends SparkFunSuite with
SharedSparkContext {
}
test("basic types") {
+val conf = new SparkConf(false)
--- End diff --
Unit test have been updated, nothing new was found.
Tuples are registered starting from line 149 in KryoSerializer.scala.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17482: [SPARK-9002][CORE] KryoSerializer initialization does no...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17482 @srowen, @BenFradet Please take a look. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17482: [SPARK-9002][CORE] KryoSerializer initialization ...
GitHub user dbolshak opened a pull request: https://github.com/apache/spark/pull/17482 [SPARK-9002][CORE] KryoSerializer initialization does not include 'Array[Int]' [SPARK-9002][CORE] KryoSerializer initialization does not include 'Array[Int]' ## What changes were proposed in this pull request? Array[Int] has been registered in KryoSerializer. The following file has been changed core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ## How was this patch tested? First, the issue was reproduced by new unit test. Then, the issue was fixed to pass the failed test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dbolshak/spark SPARK-9002 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/17482.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 #17482 commit 972dc9b88088f71bd88876e16ac18d76ffe54fbc Author: Denis Bolshakov Date: 2017-03-30T16:18:08Z [SPARK-9002][CORE] KryoSerializer initialization does not include 'Array[Int]' --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17458: [SPARK-20127][CORE] few warning have been fixed which In...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17458 Corrected. Please take a look. There is only one change (in 2 places) related to calling toSeq which has comment, but it's not clear can I leave my change or not. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108890304
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/AllStagesPage.scala
---
@@ -34,9 +34,9 @@ private[ui] class AllStagesPage(parent: StagesTab)
extends WebUIPage("") {
listener.synchronized {
val activeStages = listener.activeStages.values.toSeq
val pendingStages = listener.pendingStages.values.toSeq
- val completedStages = listener.completedStages.reverse.toSeq
+ val completedStages = listener.completedStages.reverse
--- End diff --
For me not clear, do you request this change back or we can leave my
changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17458: [SPARK-20127][CORE] few warning have been fixed which In...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17458 @srowen, @HyukjinKwon could you please merge the PR if it's ok of course? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17458: [SPARK-20127][CORE] few warning have been fixed which In...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17458 Looks like all comments have been addressed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108678514
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/AllStagesPage.scala
---
@@ -34,9 +34,9 @@ private[ui] class AllStagesPage(parent: StagesTab)
extends WebUIPage("") {
listener.synchronized {
val activeStages = listener.activeStages.values.toSeq
val pendingStages = listener.pendingStages.values.toSeq
- val completedStages = listener.completedStages.reverse.toSeq
+ val completedStages = listener.completedStages.reverse
--- End diff --
If it's really matter, it's better to add type to `val completedStages`, it
describes your point much better then trying call no-op method. But it won't be
consistent, so I would prefer to leave it as is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request: https://github.com/apache/spark/pull/17458#discussion_r108677907 --- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagesTab.scala --- @@ -35,7 +35,7 @@ private[ui] class StagesTab(parent: SparkUI) extends SparkUITab(parent, "stages" attachPage(new StagePage(this)) attachPage(new PoolPage(this)) - def isFairScheduler: Boolean = progressListener.schedulingMode == Some(SchedulingMode.FAIR) + def isFairScheduler: Boolean = progressListener.schedulingMode.contains(SchedulingMode.FAIR) --- End diff -- BTW, this was reverted. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r10860
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -290,7 +290,7 @@ private[ui] class StagePage(parent: StagesTab) extends
WebUIPage("stage") {
val _taskTable = new TaskPagedTable(
parent.conf,
UIUtils.prependBaseUri(parent.basePath) +
-s"/stages/stage?id=${stageId}&attempt=${stageAttemptId}",
+s"/stages/stage?id=$stageId&attempt=$stageAttemptId",
--- End diff --
the change was reverted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108589069
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -103,7 +103,7 @@ private[ui] class StagePage(parent: StagesTab) extends
WebUIPage("stage") {
val taskSortColumn = Option(parameterTaskSortColumn).map {
sortColumn =>
UIUtils.decodeURLParameter(sortColumn)
}.getOrElse("Index")
- val taskSortDesc =
Option(parameterTaskSortDesc).map(_.toBoolean).getOrElse(false)
+ val taskSortDesc = Option(parameterTaskSortDesc).exists(_.toBoolean)
--- End diff --
Ok, I've reverted this back.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request: https://github.com/apache/spark/pull/17458#discussion_r108529238 --- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagesTab.scala --- @@ -35,7 +35,7 @@ private[ui] class StagesTab(parent: SparkUI) extends SparkUITab(parent, "stages" attachPage(new StagePage(this)) attachPage(new PoolPage(this)) - def isFairScheduler: Boolean = progressListener.schedulingMode == Some(SchedulingMode.FAIR) + def isFairScheduler: Boolean = progressListener.schedulingMode.contains(SchedulingMode.FAIR) --- End diff -- Thanks for that comment. I did not know that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108528243
--- Diff: core/src/main/scala/org/apache/spark/ui/storage/RDDPage.scala ---
@@ -69,7 +69,7 @@ private[ui] class RDDPage(parent: StorageTab) extends
WebUIPage("rdd") {
}
val blockTableHTML = try {
val _blockTable = new BlockPagedTable(
-UIUtils.prependBaseUri(parent.basePath) +
s"/storage/rdd/?id=${rddId}",
+UIUtils.prependBaseUri(parent.basePath) +
s"/storage/rdd/?id=$rddId",
--- End diff --
This one I've just reverted back.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108527691
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -103,7 +103,7 @@ private[ui] class StagePage(parent: StagesTab) extends
WebUIPage("stage") {
val taskSortColumn = Option(parameterTaskSortColumn).map {
sortColumn =>
UIUtils.decodeURLParameter(sortColumn)
}.getOrElse("Index")
- val taskSortDesc =
Option(parameterTaskSortDesc).map(_.toBoolean).getOrElse(false)
+ val taskSortDesc = Option(parameterTaskSortDesc).exists(_.toBoolean)
--- End diff --
There is no `Option.contains` call any more, it was in another place.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #17458: [SPARK-20127][CORE] few warning have been fixed which In...
Github user dbolshak commented on the issue: https://github.com/apache/spark/pull/17458 @HyukjinKwon could you please explain how to request to merge the PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
Github user dbolshak commented on a diff in the pull request:
https://github.com/apache/spark/pull/17458#discussion_r108421849
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -317,7 +317,7 @@ private[spark] object UIUtils extends Logging {
def getHeaderContent(header: String): Seq[Node] = {
if (newlinesInHeader) {
- { header.split("\n").map { case t => {t} } }
+ { header.split("\n").map(t => {t} )}
--- End diff --
I removed almost changes related to UI.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17458: [SPARK-20127][CORE] few warning have been fixed w...
GitHub user dbolshak opened a pull request: https://github.com/apache/spark/pull/17458 [SPARK-20127][CORE] few warning have been fixed which Intellij IDEA reported Intellij IDEA ## What changes were proposed in this pull request? Few changes related to Intellij IDEA inspection. ## How was this patch tested? Changes were tested by existing unit tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/dbolshak/spark SPARK-20127 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/17458.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 #17458 commit 8a31173b436eb7d39f2423eccda7932239f8d5b6 Author: Denis Bolshakov Date: 2017-03-28T11:59:01Z [SPARK-20127][CORE] few warning have been fixed which Intellij IDEA reported Intellij IDEA commit 6a7d2ccc1c529ddca1765640073ae4a5316e51f7 Author: Denis Bolshakov Date: 2017-03-28T12:30:16Z [SPARK-20127][CORE] Applying first comments from @srowen commit 12cc39ee3974e6412fa9bdfb8915508a02b75e57 Author: Denis Bolshakov Date: 2017-03-28T12:46:27Z [SPARK-20127][CORE] Second portion of applying comments --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
