Re: issues about the systemtap log file name

Re: issues about the systemtap log file name

[Frank Ch. Eigler]

Hi -

> currently we can use '-s' option to specify the maximum size of output
> file and the maximum number of output files i.e.,
> 
> > stap -F -o test.output -S 128,10 test.stp
> 
> The issue here is the output file names will be from test.output.0 to
> test.output.INT_MAX, that is not friendly if we continuesly run
> systemtap.
> 
> So I give a suggestion that, if the 'N' of '-S size[,N]' is set, the
> output file name should be from
> output_filename.0 to output_filename.(N-1) and then rotate begin with
> output_filename.0.

That makes perfect sense.

> The attachment is my patch to fix this issue. hope it could help.
> I can't find how to submit a patch to your open source git tree, so I
> just email to you.

(The README file points to HACKING, which points to several ways.)

In this rotatingly-named file case, is there any need to remove
old files?  Can we just drop that part, and let stap incidentally
truncate the output-$N files is it goes?  If so, the code could 
be simplified even more.


- FChE

Re: issues about the systemtap log file name

[Yafang Shao]

On Sat, Jun 2, 2018 at 3:27 AM, Frank Ch. Eigler <fche@redhat.com> wrote:
> Hi -
>
>> currently we can use '-s' option to specify the maximum size of output
>> file and the maximum number of output files i.e.,
>>
>> > stap -F -o test.output -S 128,10 test.stp
>>
>> The issue here is the output file names will be from test.output.0 to
>> test.output.INT_MAX, that is not friendly if we continuesly run
>> systemtap.
>>
>> So I give a suggestion that, if the 'N' of '-S size[,N]' is set, the
>> output file name should be from
>> output_filename.0 to output_filename.(N-1) and then rotate begin with
>> output_filename.0.
>
> That makes perfect sense.
>
>> The attachment is my patch to fix this issue. hope it could help.
>> I can't find how to submit a patch to your open source git tree, so I
>> just email to you.
>
> (The README file points to HACKING, which points to several ways.)
>

Thanks :)

> In this rotatingly-named file case, is there any need to remove
> old files?  Can we just drop that part, and let stap incidentally
> truncate the output-$N files is it goes?  If so, the code could
> be simplified even more.
>

Agreed. That's reasonable.


Thanks
Yafang

Re: issues about the systemtap log file name

[Yafang Shao]

On Sat, Jun 2, 2018 at 9:35 PM, Yafang Shao <laoar.shao@gmail.com> wrote:
> On Sat, Jun 2, 2018 at 3:27 AM, Frank Ch. Eigler <fche@redhat.com> wrote:
>> Hi -
>>
>>> currently we can use '-s' option to specify the maximum size of output
>>> file and the maximum number of output files i.e.,
>>>
>>> > stap -F -o test.output -S 128,10 test.stp
>>>
>>> The issue here is the output file names will be from test.output.0 to
>>> test.output.INT_MAX, that is not friendly if we continuesly run
>>> systemtap.
>>>
>>> So I give a suggestion that, if the 'N' of '-S size[,N]' is set, the
>>> output file name should be from
>>> output_filename.0 to output_filename.(N-1) and then rotate begin with
>>> output_filename.0.
>>
>> That makes perfect sense.
>>
>>> The attachment is my patch to fix this issue. hope it could help.
>>> I can't find how to submit a patch to your open source git tree, so I
>>> just email to you.
>>
>> (The README file points to HACKING, which points to several ways.)
>>
>
> Thanks :)
>
>> In this rotatingly-named file case, is there any need to remove
>> old files?  Can we just drop that part, and let stap incidentally
>> truncate the output-$N files is it goes?  If so, the code could
>> be simplified even more.
>>
>

Hi Frank,

When I think over should we remove the old files or not again, I think
that we'd better remove the old file and create a new one.
Because we always use the file creation event to judge that we stop
writing the current file and begin to write to the next file.

I will submit a new patch with commit log.

Thanks
Yafang

Re: issues about the systemtap log file name

[Yafang Shao]

On Sat, Jun 2, 2018 at 3:27 AM, Frank Ch. Eigler <fche@redhat.com> wrote:
> Hi -
>
>> currently we can use '-s' option to specify the maximum size of output
>> file and the maximum number of output files i.e.,
>>
>> > stap -F -o test.output -S 128,10 test.stp
>>
>> The issue here is the output file names will be from test.output.0 to
>> test.output.INT_MAX, that is not friendly if we continuesly run
>> systemtap.
>>
>> So I give a suggestion that, if the 'N' of '-S size[,N]' is set, the
>> output file name should be from
>> output_filename.0 to output_filename.(N-1) and then rotate begin with
>> output_filename.0.
>
> That makes perfect sense.
>
>> The attachment is my patch to fix this issue. hope it could help.
>> I can't find how to submit a patch to your open source git tree, so I
>> just email to you.
>
> (The README file points to HACKING, which points to several ways.)
>
> In this rotatingly-named file case, is there any need to remove
> old files?  Can we just drop that part, and let stap incidentally
> truncate the output-$N files is it goes?  If so, the code could
> be simplified even more.
>
>

Hi Frank,

What about bellow change ?
We don't remove the old file any more, and just truncate it.

Hope you could give any response : )

---
 staprun/relay.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/staprun/relay.c b/staprun/relay.c
index c87bef4..fdb3001 100644
--- a/staprun/relay.c
+++ b/staprun/relay.c
@@ -71,7 +71,7 @@ time_t read_backlog(int cpu, int fnum)
        return time_backlog[cpu][fnum & BACKLOG_MASK];
 }

-static int open_outfile(int fnum, int cpu, int remove_file)
+static int open_outfile(int fnum, int cpu)
 {
        char buf[PATH_MAX];
        time_t t;
@@ -81,17 +81,8 @@ static int open_outfile(int fnum, int cpu, int remove_file)
        }

        time(&t);
-       if (fnum_max) {
-               if (remove_file) {
-                        /* remove oldest file */
-                       if (make_outfile_name(buf, PATH_MAX, fnum - fnum_max,
-                                cpu, read_backlog(cpu, fnum - fnum_max),
-                                bulkmode) < 0)
-                               return -1;
-                       remove(buf); /* don't care */
-               }
+       if (fnum_max)
                write_backlog(cpu, fnum, t);
-       }

        if (make_outfile_name(buf, PATH_MAX, fnum, cpu, t, bulkmode) < 0)
                return -1;
@@ -105,17 +96,17 @@ static int open_outfile(int fnum, int cpu, int remove_file)

 static int switch_outfile(int cpu, int *fnum)
 {
-       int remove_file = 0;
-
        dbug(3, "thread %d switching file\n", cpu);
        close(out_fd[cpu]);
        *fnum += 1;
        if (fnum_max && *fnum >= fnum_max)
-               remove_file = 1;
-       if (open_outfile(*fnum, cpu, remove_file) < 0) {
+               *fnum = 0;
+
+       if (open_outfile(*fnum, cpu) < 0) {
                perr("Couldn't open file for cpu %d, exiting.", cpu);
                return -1;
        }
+
        return 0;
 }

@@ -364,7 +355,7 @@ int init_relayfs(void)
                for (i = 0; i < ncpus; i++) {
                        if (init_backlog(avail_cpus[i]) < 0)
                                return -1;
-                       if (open_outfile(0, avail_cpus[i], 0) < 0)
+                       if (open_outfile(0, avail_cpus[i]) < 0)
                                return -1;
                }
        } else if (bulkmode) {



Thanks
Yafang