java stapbm: handle auto deletion of old files

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

java stapbm: handle auto deletion of old files

Tetsuo Handa
Hello.

I'm tempted to rewrite /usr/libexec/systemtap/stapbm using C language for
faster processing, smaller memory footprint and robust error handling. ;-)

But here is a suggestion for

  # XXX: Erase file to keep it from sticking around indefinitely,
  # in case process ends, machine reboots, pid gets reused

part without rewriting.

Currently you are embedding output of hostname command into filename.
But under what environment will that be helpful? Virtual hosts where
hostname differs each other? To me, sharing writable $flagdir between
hosts sounds like asking for troubles. If no usecase, I think we can
remove it (and I removed it in the patch below).

Also, what are two

  pkglibexecdir=`eval echo $pkglibexecdir`

lines for? When do these lines make sense?

By the way, I'm waiting for your comment regarding
https://sourceware.org/ml/systemtap/2017-q3/msg00230.html .

----------------------------------------
>From 0223d184146c1fa3c2828fdf6ab25f75dad3e67c Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <[hidden email]>
Date: Mon, 23 Oct 2017 15:39:06 +0900
Subject: [PATCH] java stapbm: handle auto deletion of old files

Linux has /proc/sys/kernel/random/boot_id interface which is determined
at boot time. Since this ID is unlikely same for each boot, we can use
this ID for detecting whether a system has restarted or not.

Linux has starttime field in /proc/$pid/stat interface which is determined
at process creation time. Since this stamp is unlikely same when PID is
reused, we can use this ID for detecting whether PID is reused or not.
(Be careful when parsing that field, for comm field might include spaces
and/or parentheses.)

Thus, change the lock file from `hostname`-${target_pid}-bm to
${boot_id}-${target_pid}-${start_time}-bm and automatically delete old
lock files when system restart or PID reuse is detected when a Byteman
agent is installed.

We can delete rule files from filesystem as soon as we hold a file
descriptor of the rule file, for we can specify /proc/$pid/fd/ interface
for rule files which will be automatically disappear when byteman-submit
completes. In case stapbm was unexpectedly terminated between creating
and deleting a rule file, change the rule file from `hostname`-$$.btm to
${boot_id}-${target_pid}-${start_time}-$$.btm so that old rule files
will be automatically deleted as with old lock files.

Signed-off-by: Tetsuo Handa <[hidden email]>
---
 java/stapbm.in | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/java/stapbm.in b/java/stapbm.in
index f483d20..e520828 100755
--- a/java/stapbm.in
+++ b/java/stapbm.in
@@ -146,7 +146,9 @@ fi
 # JVM is ready for further bytemanning without a prior setup step,
 # and include in it the designated byteman agent listening-port number.
 #
-byteman_installed_portfile=$flagdir/`hostname`-${target_pid}-bm
+read boot_id < /proc/sys/kernel/random/boot_id
+start_time=`awk -F ')' ' { split($NF, array, " "); print array[20]; } ' /proc/${target_pid}/stat`
+byteman_installed_portfile=$flagdir/${boot_id}-${target_pid}-${start_time}-bm
 
 exec 200>>$byteman_installed_portfile # open/create lock file
 flock -x 200  # exclusive-lock it
@@ -188,10 +190,9 @@ else
     if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
         echo "Byteman agent installed for java pid $target_pid, port $bmport"
     fi
-    # XXX: Erase file to keep it from sticking around indefinitely,
-    # in case process ends, machine reboots, pid gets reused
-    # XXX: consider explicit notification to stapbm via process("java").begin/end ?
-    # ... or else: liveness-check below
+    # Erase file to keep it from sticking around indefinitely,
+    # in case process ends, machine reboots, pid gets reused.
+    find -- $flagdir/ -maxdepth 1 -type f \( -name '*-bm' -o -name '*.btm' \) \( ! -name ${boot_id}'-*' -o \( -name ${boot_id}'-'${target_pid}'-*' ! -name ${boot_id}'-'${target_pid}'-'${start_time}'-*' \) \) -print0 | xargs -0r rm -f --
 fi
 exec 200>&-   # close file & release flock
 
@@ -245,13 +246,14 @@ function echo_bytemanrule()
 
 
 # Generate the byteman rule file on-the-fly
-btmfile=$flagdir/`hostname`-$$.btm
-echo_bytemanrule > $btmfile
-trap 'rm -f $btmfile' 0 1 2 3 4 5 9 15
+btmfile=$flagdir/${boot_id}-${target_pid}-${start_time}-$$.btm
+exec 201>$btmfile # open/create rule file
+rm -f $btmfile
+echo_bytemanrule >&201
 
 if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
     echo "Byteman rule file:"
-    cat $btmfile
+    cat /proc/$$/fd/201
 fi
 
 if [ $mode = "uninstall" ]; then
@@ -261,7 +263,7 @@ else
 fi
 
 if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
-    echo java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd $btmfile
+    echo java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd /proc/$$/fd/201
 fi
 
-exec java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd $btmfile
+exec java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd /proc/$$/fd/201
--
1.8.3.1
Reply | Threaded
Open this post in threaded view
|

Re: java stapbm: handle auto deletion of old files

David Smith-19
Tetsuo,

I'm sorry that we haven't responded to your emails about systemtap's
java support. Unfortunately, the guy who worked on this has moved to
another project, and no one has really stepped up to take over java
support. Luckily, not many problems have been reported with it
(besides the things you've mentioned).

I've touched the java support a couple of times, but haven't spent the
time to really understand it to the level your patches require.

I'm not really sure how to proceed here, just wanted you to know that
you weren't being completely ignored.

If someone had the time, it might not be a bad idea to take a step
back and look at the current state of java debugging and see if
byteman is still the right answer here. One of the things that have
always bugged me about the current systemtap java support is the lack
of support for listing java class/methods
(<https://sourceware.org/bugzilla/show_bug.cgi?id=15773>).


On Mon, Oct 23, 2017 at 6:19 AM, Tetsuo Handa
<[hidden email]> wrote:

> Hello.
>
> I'm tempted to rewrite /usr/libexec/systemtap/stapbm using C language for
> faster processing, smaller memory footprint and robust error handling. ;-)
>
> But here is a suggestion for
>
>   # XXX: Erase file to keep it from sticking around indefinitely,
>   # in case process ends, machine reboots, pid gets reused
>
> part without rewriting.
>
> Currently you are embedding output of hostname command into filename.
> But under what environment will that be helpful? Virtual hosts where
> hostname differs each other? To me, sharing writable $flagdir between
> hosts sounds like asking for troubles. If no usecase, I think we can
> remove it (and I removed it in the patch below).
>
> Also, what are two
>
>   pkglibexecdir=`eval echo $pkglibexecdir`
>
> lines for? When do these lines make sense?
>
> By the way, I'm waiting for your comment regarding
> https://sourceware.org/ml/systemtap/2017-q3/msg00230.html .
>
> ----------------------------------------
> >From 0223d184146c1fa3c2828fdf6ab25f75dad3e67c Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <[hidden email]>
> Date: Mon, 23 Oct 2017 15:39:06 +0900
> Subject: [PATCH] java stapbm: handle auto deletion of old files
>
> Linux has /proc/sys/kernel/random/boot_id interface which is determined
> at boot time. Since this ID is unlikely same for each boot, we can use
> this ID for detecting whether a system has restarted or not.
>
> Linux has starttime field in /proc/$pid/stat interface which is determined
> at process creation time. Since this stamp is unlikely same when PID is
> reused, we can use this ID for detecting whether PID is reused or not.
> (Be careful when parsing that field, for comm field might include spaces
> and/or parentheses.)
>
> Thus, change the lock file from `hostname`-${target_pid}-bm to
> ${boot_id}-${target_pid}-${start_time}-bm and automatically delete old
> lock files when system restart or PID reuse is detected when a Byteman
> agent is installed.
>
> We can delete rule files from filesystem as soon as we hold a file
> descriptor of the rule file, for we can specify /proc/$pid/fd/ interface
> for rule files which will be automatically disappear when byteman-submit
> completes. In case stapbm was unexpectedly terminated between creating
> and deleting a rule file, change the rule file from `hostname`-$$.btm to
> ${boot_id}-${target_pid}-${start_time}-$$.btm so that old rule files
> will be automatically deleted as with old lock files.
>
> Signed-off-by: Tetsuo Handa <[hidden email]>
> ---
>  java/stapbm.in | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/java/stapbm.in b/java/stapbm.in
> index f483d20..e520828 100755
> --- a/java/stapbm.in
> +++ b/java/stapbm.in
> @@ -146,7 +146,9 @@ fi
>  # JVM is ready for further bytemanning without a prior setup step,
>  # and include in it the designated byteman agent listening-port number.
>  #
> -byteman_installed_portfile=$flagdir/`hostname`-${target_pid}-bm
> +read boot_id < /proc/sys/kernel/random/boot_id
> +start_time=`awk -F ')' ' { split($NF, array, " "); print array[20]; } ' /proc/${target_pid}/stat`
> +byteman_installed_portfile=$flagdir/${boot_id}-${target_pid}-${start_time}-bm
>
>  exec 200>>$byteman_installed_portfile # open/create lock file
>  flock -x 200  # exclusive-lock it
> @@ -188,10 +190,9 @@ else
>      if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
>          echo "Byteman agent installed for java pid $target_pid, port $bmport"
>      fi
> -    # XXX: Erase file to keep it from sticking around indefinitely,
> -    # in case process ends, machine reboots, pid gets reused
> -    # XXX: consider explicit notification to stapbm via process("java").begin/end ?
> -    # ... or else: liveness-check below
> +    # Erase file to keep it from sticking around indefinitely,
> +    # in case process ends, machine reboots, pid gets reused.
> +    find -- $flagdir/ -maxdepth 1 -type f \( -name '*-bm' -o -name '*.btm' \) \( ! -name ${boot_id}'-*' -o \( -name ${boot_id}'-'${target_pid}'-*' ! -name ${boot_id}'-'${target_pid}'-'${start_time}'-*' \) \) -print0 | xargs -0r rm -f --
>  fi
>  exec 200>&-   # close file & release flock
>
> @@ -245,13 +246,14 @@ function echo_bytemanrule()
>
>
>  # Generate the byteman rule file on-the-fly
> -btmfile=$flagdir/`hostname`-$$.btm
> -echo_bytemanrule > $btmfile
> -trap 'rm -f $btmfile' 0 1 2 3 4 5 9 15
> +btmfile=$flagdir/${boot_id}-${target_pid}-${start_time}-$$.btm
> +exec 201>$btmfile # open/create rule file
> +rm -f $btmfile
> +echo_bytemanrule >&201
>
>  if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
>      echo "Byteman rule file:"
> -    cat $btmfile
> +    cat /proc/$$/fd/201
>  fi
>
>  if [ $mode = "uninstall" ]; then
> @@ -261,7 +263,7 @@ else
>  fi
>
>  if [ "$SYSTEMTAP_VERBOSE" != "0" ]; then
> -    echo java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd $btmfile
> +    echo java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd /proc/$$/fd/201
>  fi
>
> -exec java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd $btmfile
> +exec java -classpath ${BYTEMAN_SUBMIT_JAR}:${BYTEMAN_JAR}:${HELPERSDT_JAR} org.jboss.byteman.agent.submit.Submit -p $bmport $bmcmd /proc/$$/fd/201
> --
> 1.8.3.1



--
David Smith
Principal Software Engineer
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: java stapbm: handle auto deletion of old files

Tetsuo Handa
David Smith wrote:

> Tetsuo,
>
> I'm sorry that we haven't responded to your emails about systemtap's
> java support. Unfortunately, the guy who worked on this has moved to
> another project, and no one has really stepped up to take over java
> support. Luckily, not many problems have been reported with it
> (besides the things you've mentioned).
>
> I've touched the java support a couple of times, but haven't spent the
> time to really understand it to the level your patches require.

Regarding "handle auto deletion of old files" patch, it does not require
java knowledge. It utilizes how Linux kernel works.

>
> I'm not really sure how to proceed here, just wanted you to know that
> you weren't being completely ignored.

Thank you for responding.

>
> If someone had the time, it might not be a bad idea to take a step
> back and look at the current state of java debugging and see if
> byteman is still the right answer here. One of the things that have
> always bugged me about the current systemtap java support is the lack
> of support for listing java class/methods
> (<https://sourceware.org/bugzilla/show_bug.cgi?id=15773>).

Even if we stop using byteman in future, I think that this "handle auto
deletion of old files" patch is worth applying, for it prevents random
failure which will become more and more likely to occur as stapbm is
executed for more and more times.