-
Notifications
You must be signed in to change notification settings - Fork 0
[SPARK-24958] [WIP] Report executors' process tree total memory information to heartbeat signals #1
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
Changes from all commits
04875b8
162b9b2
29a44c7
3671427
c79b5ab
8f97b50
b14cebc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.executor | ||
|
|
||
| private[spark] trait ProcessTreeMetrics { | ||
| def isAvailable: Boolean | ||
| def pid: Int | ||
| def computePid(): Int | ||
| def createProcessTree() | ||
| def updateProcessTree() | ||
| def getJVMRSSInfo(): Long | ||
| def getJVMVirtualMemInfo(): Long | ||
| def getPythonRSSInfo(): Long | ||
| def getPythonVirtualMemInfo(): Long | ||
| def getOtherRSSInfo(): Long | ||
| def getOtherVirtualMemInfo(): Long | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,271 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.executor | ||
|
|
||
| import java.io._ | ||
| import java.nio.charset.Charset | ||
| import java.nio.file.{Files, Paths} | ||
|
|
||
| import scala.collection.mutable | ||
| import scala.collection.mutable.ArrayBuffer | ||
| import scala.collection.mutable.Queue | ||
|
|
||
| import org.apache.spark.internal.Logging | ||
|
|
||
|
|
||
| // Some of the ideas here are taken from the ProcfsBasedProcessTree class in hadoop | ||
| // project. | ||
| class ProcfsBasedSystems extends ProcessTreeMetrics with Logging { | ||
rezasafi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| val procfsDir = "/proc/" | ||
| var isAvailable: Boolean = isItProcfsBased | ||
| val pid: Int = computePid() | ||
| val ptree: scala.collection.mutable.Map[ Int, Set[Int]] = | ||
| scala.collection.mutable.Map[ Int, Set[Int]]() | ||
| val PROCFS_STAT_FILE = "stat" | ||
| var latestJVMVmemTotal: Long = 0 | ||
| var latestJVMRSSTotal: Long = 0 | ||
| var latestPythonVmemTotal: Long = 0 | ||
| var latestPythonRSSTotal: Long = 0 | ||
| var latestOtherVmemTotal: Long = 0 | ||
| var latestOtherRSSTotal: Long = 0 | ||
|
|
||
| createProcessTree | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this method has a side-effect, so you should call it with parens |
||
|
|
||
| def isItProcfsBased: Boolean = { | ||
| val testing = sys.env.contains("SPARK_TESTING") || sys.props.contains("spark.testing") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this cause problems when someone tries to run tests on windows?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't test this on a windows machine. but since I will catch exceptions during the process tree creation/update and make isAvailable false in case of the exceptions I think there won't be a problem. |
||
| if (testing) { | ||
| return true | ||
| } | ||
| try { | ||
| if (!Files.exists(Paths.get(procfsDir))) { | ||
| return false | ||
| } | ||
| } | ||
| catch { | ||
| case f: FileNotFoundException => return false | ||
| } | ||
|
|
||
| val shouldLogStageExecutorProcessTreeMetrics = org.apache.spark.SparkEnv.get.conf. | ||
| getBoolean("spark.eventLog.logStageExecutorProcessTreeMetrics.enabled", true) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better to use the config entry
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use this so that the user can disable process tree metrics even when the normal executor metrics are enabled. So that they have more freedom in case they feel reporting process tree metrics introduces a lot of overhead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok I can see having an extra config here, but then you should create a config entry for it.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw in other places that we didn't do this. It would be headache to pass down a config object and I need a lot of changes
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a config entry without the need to send config object. thanks foe the comment. |
||
| true && shouldLogStageExecutorProcessTreeMetrics | ||
| } | ||
|
|
||
|
|
||
| def computePid(): Int = { | ||
| if (!isAvailable) { | ||
| return -1; | ||
| } | ||
| try { | ||
| // This can be simplified in java9: | ||
| // https://docs.oracle.com/javase/9/docs/api/java/lang/ProcessHandle.html | ||
| val cmd = Array("bash", "-c", "echo $PPID") | ||
| val length = 10 | ||
| var out: Array[Byte] = Array.fill[Byte](length)(0) | ||
| Runtime.getRuntime.exec(cmd).getInputStream.read(out) | ||
| val pid = Integer.parseInt(new String(out, "UTF-8").trim) | ||
| return pid; | ||
| } | ||
| catch { | ||
| case e: IOException => logDebug("IO Exception when trying to compute process tree." + | ||
| " As a result reporting of ProcessTree metrics is stopped") | ||
| isAvailable = false | ||
| return -1 | ||
| case _ => logDebug("Some exception occurred when trying to compute process tree. " + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe catch NonFatal instead?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you very much, @wypoon for the reviews. I though maybe better to not have an exception in the logs to avoid frightening the user There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this an exception I should be worried about? I am running Spark Core 3.0.1 and I saw this warning (at least now it is a warning). I am running Windows 10. I spent hours trying to find why this is happening. I get the following warning |
||
| "As a result reporting of ProcessTree metrics is stopped") | ||
| isAvailable = false | ||
| return -1 | ||
| } | ||
| } | ||
|
|
||
|
|
||
| def createProcessTree(): Unit = { | ||
| if (!isAvailable) { | ||
| return | ||
| } | ||
| val queue: Queue[Int] = new Queue[Int]() | ||
| queue += pid | ||
| while( !queue.isEmpty ) { | ||
| val p = queue.dequeue() | ||
| val c = getChildPIds(p) | ||
| if(!c.isEmpty) { | ||
| queue ++= c | ||
| ptree += (p -> c.toSet) | ||
| } | ||
| else { | ||
| ptree += (p -> Set[Int]()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| def updateProcessTree(): Unit = { | ||
| if (!isAvailable) { | ||
| return | ||
| } | ||
| val queue: Queue[Int] = new Queue[Int]() | ||
| queue += pid | ||
| while( !queue.isEmpty ) { | ||
| val p = queue.dequeue() | ||
| val c = getChildPIds(p) | ||
| if(!c.isEmpty) { | ||
| queue ++= c | ||
| val preChildren = ptree.get(p) | ||
| preChildren match { | ||
| case Some(children) => if (!c.toSet.equals(children)) { | ||
| val diff: Set[Int] = children -- c.toSet | ||
| ptree.update(p, c.toSet ) | ||
| diff.foreach(ptree.remove(_)) | ||
| } | ||
| case None => ptree.update(p, c.toSet ) | ||
| } | ||
| } | ||
| else { | ||
| ptree.update(p, Set[Int]()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Hadoop ProcfsBasedProcessTree class used regex and pattern matching to retrive the memory | ||
| * info. I tried that but found it not correct during tests, so I used normal string analysis | ||
| * instead. The computation of RSS and Vmem are based on proc(5): | ||
| * http://man7.org/linux/man-pages/man5/proc.5.html | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this changes at all, would we know? Or would we just start reporting wrong values?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah if it changes we need to fix this. probably it won't change though. I didn't find another way to retrieve this info. |
||
| */ | ||
| def getProcessInfo(pid: Int): Unit = { | ||
| try { | ||
| val pidDir: File = new File(procfsDir, pid.toString) | ||
rezasafi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| val fReader = new InputStreamReader( | ||
| new FileInputStream( | ||
| new File(pidDir, PROCFS_STAT_FILE)), Charset.forName("UTF-8")) | ||
| val in: BufferedReader = new BufferedReader(fReader) | ||
| val procInfo = in.readLine | ||
| in.close | ||
| fReader.close | ||
| val procInfoSplit = procInfo.split(" ") | ||
| if ( procInfoSplit != null ) { | ||
| if (procInfoSplit(1).toLowerCase.contains("java")) { | ||
| latestJVMVmemTotal += procInfoSplit(22).toLong | ||
| latestJVMRSSTotal += procInfoSplit(23).toLong | ||
| } | ||
| else if (procInfoSplit(1).toLowerCase.contains("python")) { | ||
| latestPythonVmemTotal += procInfoSplit(22).toLong | ||
| latestPythonRSSTotal += procInfoSplit(23).toLong | ||
| } | ||
| else { | ||
| latestOtherVmemTotal += procInfoSplit(22).toLong | ||
| latestOtherRSSTotal += procInfoSplit(23).toLong } | ||
| } | ||
| } catch { | ||
| case f: FileNotFoundException => return null | ||
rezasafi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
|
|
||
| def getOtherRSSInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| updateProcessTree | ||
| val pids = ptree.keySet | ||
| latestJVMRSSTotal = 0 | ||
| latestJVMVmemTotal = 0 | ||
| latestPythonRSSTotal = 0 | ||
| latestPythonVmemTotal = 0 | ||
| latestOtherRSSTotal = 0 | ||
| latestOtherVmemTotal = 0 | ||
| for (p <- pids) { | ||
| getProcessInfo(p) | ||
| } | ||
| latestOtherRSSTotal | ||
| } | ||
|
|
||
|
|
||
| def getOtherVirtualMemInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| // We won't call updateProcessTree and also compute total virtual memory here | ||
| // since we already did all of this when we computed RSS info | ||
rezasafi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| latestOtherVmemTotal | ||
| } | ||
|
|
||
|
|
||
| def getJVMRSSInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| latestJVMRSSTotal | ||
| } | ||
|
|
||
|
|
||
| def getJVMVirtualMemInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| latestJVMVmemTotal | ||
| } | ||
|
|
||
|
|
||
| def getPythonRSSInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| latestPythonRSSTotal | ||
| } | ||
|
|
||
|
|
||
| def getPythonVirtualMemInfo(): Long = { | ||
| if (!isAvailable) { | ||
| return -1 | ||
| } | ||
| latestPythonVmemTotal | ||
| } | ||
|
|
||
|
|
||
| def getChildPIds(pid: Int): ArrayBuffer[Int] = { | ||
| try { | ||
| val cmd = Array("pgrep", "-P", pid.toString) | ||
| val input = Runtime.getRuntime.exec(cmd).getInputStream | ||
| val childPidsInByte: mutable.ArrayBuffer[Byte] = new mutable.ArrayBuffer() | ||
| var d = input.read() | ||
| while (d != -1) { | ||
| childPidsInByte.append(d.asInstanceOf[Byte]) | ||
| d = input.read() | ||
| } | ||
| input.close() | ||
| val childPids = new String(childPidsInByte.toArray, "UTF-8").split("\n") | ||
| val childPidsInInt: ArrayBuffer[Int] = new ArrayBuffer[Int]() | ||
| for (p <- childPids) { | ||
| if (p != "") { | ||
| childPidsInInt += Integer.parseInt(p) | ||
| } | ||
| } | ||
| childPidsInInt | ||
| } catch { | ||
| case e: IOException => logDebug("IO Exception when trying to compute process tree." + | ||
| " As a result reporting of ProcessTree metrics is stopped") | ||
| isAvailable = false | ||
| return new mutable.ArrayBuffer() | ||
| case _ => logDebug("Some exception occurred when trying to compute process tree. As a result" + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NonFatal, maybe? |
||
| " reporting of ProcessTree metrics is stopped") | ||
| isAvailable = false | ||
| return new mutable.ArrayBuffer() | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo, its not worth creating the trait now when there is only one implementation. We can add that abstraction later if its useful. (this isn't exposed to users at all anyway.)
If we are keeping this, some of these should be private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about future, when someone want to add new implementation, so that they use the same methods and not having differences across platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I understand that goal, but who knows when / if that will ever happen. in the meantime, it just makes this a slightly harder to follow. Whenever someone does want to put in another implementation, its pretty easy for them to add the interface at that time.