Skip to content
Closed
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3f8321a
Integration of ProcessTreeMetrics with PR 21221
Jul 26, 2018
cd16a75
Changing the position of ptree and also make the computation configur…
Aug 7, 2018
94c2b04
Seperate metrics for jvm, python and others and update the tests
Aug 8, 2018
062f5d7
Update JsonProtocolSuite
Sep 25, 2018
245221d
[SPARK-24958] Add executors' process tree total memory information to…
Oct 2, 2018
c72be03
Adressing most of Imran's comments
Oct 3, 2018
8f3c938
Fixing the scala style and some minor comments
Oct 3, 2018
f2dca27
Removing types from the definitions where ever possible
Oct 4, 2018
a9f924c
Using Utils methods when possible or use ProcessBuilder
Oct 5, 2018
a11e3a2
make use of Utils.trywithresources
Oct 5, 2018
34ad625
Changing ExecutorMericType and ExecutorMetrics to use a map instead o…
Oct 9, 2018
415f976
Changing ExecutorMetric to use array instead of a map
Oct 10, 2018
067b81d
A small cosmetic change
Oct 10, 2018
18ee4ad
Merge branch 'master' of https://github.com/apache/spark into ptreeme…
Oct 17, 2018
7f7ed2b
Applying latest review commments. Using Arrays instead of Map for ret…
Oct 23, 2018
f3867ff
Merge branch 'master' of https://github.com/apache/spark into ptreeme…
Nov 5, 2018
0f8f3e2
Fix an issue with jsonProtoclSuite
Nov 5, 2018
ea08c61
Fix scalastyle issue
Nov 5, 2018
8f20857
Applying latest review comments
Nov 14, 2018
6e65360
Using the companion object and other stuff
Nov 27, 2018
4659f4a
Update the use of process builder and applying other review comments
Nov 28, 2018
ef4be38
Small style fixes based on reviews
Nov 30, 2018
805741c
Applying review comments, mostly style related
Nov 30, 2018
4c1f073
emove the unnecessary trywithresources
Nov 30, 2018
0a7402e
Applying the comment about error handling and some more style fixes
Dec 4, 2018
3d65b35
Removing a return
Dec 6, 2018
6eab315
Reordering of info in a test resource file to avoid confusion
Dec 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Removing types from the definitions where ever possible
  • Loading branch information
Reza Safi committed Oct 4, 2018
commit f2dca27359e2e51ac53df0a8fae2c3cd0b966946
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,20 @@ private[spark] case class ProcfsBasedSystemsMetrics(

// Some of the ideas here are taken from the ProcfsBasedProcessTree class in hadoop
// project.
private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Logging {
private[spark] class ProcfsBasedSystems(val procfsDir: String = "/proc/") extends Logging {
val procfsStatFile = "stat"
var pageSize = computePageSize()
var isAvailable: Boolean = isProcfsAvailable
private val pid: Int = computePid()
private val ptree: scala.collection.mutable.Map[ Int, Set[Int]] =
scala.collection.mutable.Map[ Int, Set[Int]]()
private val pid = computePid()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pageSize is only a var for testing -- instead just optionally pass it in to the constructor

also I think all of these can be private.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I can't call computePageSize() in the constructor signature to compute the default value. Another solution is to check for testing inside computePageSize and if we are testing assign a value to it that is provided in the constructor (default to 4096).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't put it as a default value, but if you make it a static method, then you can provide an overloaded method which uses it, see squito@cf00835

But, I think your other proposal is even better, if its testing just give it a fixed value (no need to even make it an argument to the constructor at all).

private val ptree = mutable.Map[ Int, Set[Int]]()

var allMetrics: ProcfsBasedSystemsMetrics = ProcfsBasedSystemsMetrics(0, 0, 0, 0, 0, 0)
private var latestJVMVmemTotal: Long = 0
private var latestJVMRSSTotal: Long = 0
private var latestPythonVmemTotal: Long = 0
private var latestPythonRSSTotal: Long = 0
private var latestOtherVmemTotal: Long = 0
private var latestOtherRSSTotal: Long = 0
private var latestJVMVmemTotal = 0L
private var latestJVMRSSTotal = 0L
private var latestPythonVmemTotal = 0L
private var latestPythonRSSTotal = 0L
private var latestOtherVmemTotal = 0L
private var latestOtherRSSTotal = 0L

computeProcessTree()

Expand Down Expand Up @@ -86,7 +85,7 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
// https://docs.oracle.com/javase/9/docs/api/java/lang/ProcessHandle.html
val cmd = Array("bash", "-c", "echo $PPID")
val length = 10
val out: Array[Byte] = Array.fill[Byte](length)(0)
val out = Array.fill[Byte](length)(0)
Runtime.getRuntime.exec(cmd).getInputStream.read(out)
val pid = Integer.parseInt(new String(out, "UTF-8").trim)
return pid;
Expand All @@ -105,7 +104,7 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
return 0;
}
val cmd = Array("getconf", "PAGESIZE")
val out: Array[Byte] = Array.fill[Byte](10)(0)
val out = Array.fill[Byte](10)(0)
Runtime.getRuntime.exec(cmd).getInputStream.read(out)
return Integer.parseInt(new String(out, "UTF-8").trim)
}
Expand All @@ -114,7 +113,7 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
if (!isAvailable) {
return
}
val queue: Queue[Int] = new Queue[Int]()
val queue = mutable.Queue.empty[Int]
queue += pid
while( !queue.isEmpty ) {
val p = queue.dequeue()
Expand All @@ -133,15 +132,15 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
try {
val cmd = Array("pgrep", "-P", pid.toString)
val input = Runtime.getRuntime.exec(cmd).getInputStream
val childPidsInByte: mutable.ArrayBuffer[Byte] = new mutable.ArrayBuffer()
val childPidsInByte = mutable.ArrayBuffer.empty[Byte]
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]()
val childPidsInInt = mutable.ArrayBuffer.empty[Int]
for (p <- childPids) {
if (p != "") {
childPidsInInt += Integer.parseInt(p)
Expand All @@ -152,7 +151,7 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
case e: IOException => logDebug("IO Exception when trying to compute process tree." +
" As a result reporting of ProcessTree metrics is stopped", e)
isAvailable = false
return new mutable.ArrayBuffer()
return mutable.ArrayBuffer.empty[Int]
}
}

Expand All @@ -164,11 +163,11 @@ private[spark] class ProcfsBasedSystems(procfsDir: String = "/proc/") extends Lo
* http://man7.org/linux/man-pages/man5/proc.5.html
*/
try {
val pidDir: File = new File(procfsDir, pid.toString)
val pidDir = new File(procfsDir, pid.toString)
val fReader = new InputStreamReader(
new FileInputStream(
new File(pidDir, procfsStatFile)), Charset.forName("UTF-8"))
val in: BufferedReader = new BufferedReader(fReader)
val in = new BufferedReader(fReader)
val procInfo = in.readLine
in.close
fReader.close
Expand Down