From 3de8aa530b812f8e577270c99da16a0d6db0e31c Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Tue, 31 Jan 2012 22:44:00 -0300 Subject: [PATCH] More fixes as per Baron's review. --- bin/pt-diskstats | 55 ++++++++++++++++++++++++++------------------ lib/Diskstats.pm | 5 +--- lib/DiskstatsMenu.pm | 49 ++++++++++++++++++++++++--------------- 3 files changed, 65 insertions(+), 44 deletions(-) diff --git a/bin/pt-diskstats b/bin/pt-diskstats index 36377a11..2ed3c36b 100755 --- a/bin/pt-diskstats +++ b/bin/pt-diskstats @@ -2370,10 +2370,7 @@ sub print_deltas { if ( $self->automatic_headers() && !$self->isa("DiskstatsGroupByAll") ) { - $header_method = ref($header_method) - ? $header_method - : "force_print_header"; - $self->$header_method( $header, "#ts", "device" ); + $self->force_print_header( $header, "#ts", "device" ); } } } @@ -2894,8 +2891,6 @@ my %actions = ( 'D' => \&group_by, 'S' => \&group_by, 'i' => \&hide_inactive_disks, - 'd' => get_new_value_for( "refresh_interval", - "Enter a new refresh interval in seconds: " ), 'z' => get_new_value_for( "sample_time", "Enter a new interval between samples in seconds: " ), 'c' => get_new_regex_for( "columns_regex", @@ -2970,6 +2965,7 @@ sub run_interactive { gather_while => sub { getppid() }, samples_to_gather => $o->get('iterations'), filename => $filename, + sample_interval => $o->get('interval'), ); if ( $filename ) { unlink $filename unless $o->get('save-samples'); @@ -3031,7 +3027,7 @@ sub run_interactive { my $run = 1; MAIN_LOOP: while ($run) { - my $refresh_interval = $o->get('refresh-interval'); + my $refresh_interval = $o->get('interval'); my $time = scalar Time::HiRes::gettimeofday(); my $sleep = $refresh_interval - fmod( $time, $refresh_interval ); @@ -3101,21 +3097,30 @@ sub read_command_timeout { sub gather_samples { my (%args) = @_; my $samples = 0; - my $fh; + my $sample_interval = $args{sample_interval}; + my @fhs; if ( my $filename = $args{filename} ) { - open $fh, ">>", $filename + open my $fh, ">>", $filename or die "Cannot open $filename for appending: $OS_ERROR"; - } - else { - $fh = \*STDOUT; + push @fhs, $fh; } - $fh->autoflush(1); + push @fhs, \*STDOUT; + + for my $fh ( @fhs ) { + $fh->autoflush(1); + } + + Time::HiRes::sleep( $sample_interval + - fmod( scalar(Time::HiRes::gettimeofday()), + $sample_interval)); GATHER_DATA: while ( $args{gather_while}->() ) { - my $sleep = 1 - fmod( scalar(Time::HiRes::gettimeofday()), 1 ); + my $time_of_day = scalar(Time::HiRes::gettimeofday()); + my $sleep = $sample_interval + - fmod( $time_of_day, $sample_interval ); Time::HiRes::sleep($sleep); open my $diskstats_fh, "<", "/proc/diskstats" @@ -3124,7 +3129,9 @@ sub gather_samples { my @to_print = timestamp(); push @to_print, <$diskstats_fh>; - print { $fh } @to_print; + for my $fh ( @fhs ) { + print { $fh } @to_print; + } close $diskstats_fh or die $OS_ERROR; $samples++; @@ -3133,7 +3140,10 @@ sub gather_samples { last GATHER_DATA; } } - close $fh or die $OS_ERROR; + pop @fhs; # STDOUT + for my $fh ( @fhs ) { + close $fh or die $OS_ERROR; + } return; } @@ -3203,7 +3213,7 @@ sub help { my $column_re = $args{OptionParser}->get('columns-regex'); my $device_re = $args{OptionParser}->get('devices-regex'); my $interval = $obj->sample_time() || '(none)'; - my $disp_int = $args{OptionParser}->get('refresh-interval'); + my $disp_int = $args{OptionParser}->get('interval'); my $inact_disk = $obj->show_inactive() ? 'no' : 'yes'; for my $re ( $column_re, $device_re ) { @@ -3223,6 +3233,7 @@ sub help { q) Quit the program ------------------- Press any key to continue ----------------------- HELP + pause(%args); return; } @@ -3319,9 +3330,8 @@ sub pause { } sub timestamp { - my ( $seconds, $microseconds ) = Time::HiRes::gettimeofday(); - return sprintf( "TS %d.%d %s\n", $seconds, - $microseconds*1000, Transformers::ts($seconds) ); + my ($s, $m) = Time::HiRes::gettimeofday(); + return sprintf( "TS %d.%09d %s\n", $s, $m*1000, Transformers::ts( $s ) ); } sub _d { @@ -3364,7 +3374,7 @@ sub main { $o->get_opts(); # --sample-time only applies to --group-by sample. - if ( PTDEBUG && $o->get('group-by') !~ m/sample/i ) { + if ( PTDEBUG && $o->get('group-by') !~ m/sample/i && $o->get('sample-time') ) { _d("Possibly useless use of --sample-time without --group-by sample"); } @@ -3678,11 +3688,12 @@ type: int When in interactive mode, stop after N samples. Run forever by default. -=item --refresh-interval +=item --interval type: int; default: 1 When in interactive mode, wait N seconds before printing to the screen. +Also, how often the tool should sample /proc/diskstats. =item --show-inactive diff --git a/lib/Diskstats.pm b/lib/Diskstats.pm index 8259e811..1df9088e 100644 --- a/lib/Diskstats.pm +++ b/lib/Diskstats.pm @@ -966,10 +966,7 @@ sub print_deltas { if ( $self->automatic_headers() && !$self->isa("DiskstatsGroupByAll") ) { - $header_method = ref($header_method) - ? $header_method - : "force_print_header"; - $self->$header_method( $header, "#ts", "device" ); + $self->force_print_header( $header, "#ts", "device" ); } } } diff --git a/lib/DiskstatsMenu.pm b/lib/DiskstatsMenu.pm index 735d4811..7521de81 100644 --- a/lib/DiskstatsMenu.pm +++ b/lib/DiskstatsMenu.pm @@ -46,8 +46,6 @@ my %actions = ( 'D' => \&group_by, 'S' => \&group_by, 'i' => \&hide_inactive_disks, - 'd' => get_new_value_for( "refresh_interval", - "Enter a new refresh interval in seconds: " ), 'z' => get_new_value_for( "sample_time", "Enter a new interval between samples in seconds: " ), 'c' => get_new_regex_for( "columns_regex", @@ -132,6 +130,7 @@ sub run_interactive { gather_while => sub { getppid() }, samples_to_gather => $o->get('iterations'), filename => $filename, + sample_interval => $o->get('interval'), ); if ( $filename ) { unlink $filename unless $o->get('save-samples'); @@ -198,7 +197,7 @@ sub run_interactive { my $run = 1; MAIN_LOOP: while ($run) { - my $refresh_interval = $o->get('refresh-interval'); + my $refresh_interval = $o->get('interval'); my $time = scalar Time::HiRes::gettimeofday(); my $sleep = $refresh_interval - fmod( $time, $refresh_interval ); @@ -286,21 +285,31 @@ sub read_command_timeout { sub gather_samples { my (%args) = @_; my $samples = 0; - my $fh; + my $sample_interval = $args{sample_interval}; + my @fhs; if ( my $filename = $args{filename} ) { - open $fh, ">>", $filename + open my $fh, ">>", $filename or die "Cannot open $filename for appending: $OS_ERROR"; - } - else { - $fh = \*STDOUT; + push @fhs, $fh; } - $fh->autoflush(1); + push @fhs, \*STDOUT; + + for my $fh ( @fhs ) { + $fh->autoflush(1); + } + + # Wait until the next tick of the clock + Time::HiRes::sleep( $sample_interval + - fmod( scalar(Time::HiRes::gettimeofday()), + $sample_interval)); GATHER_DATA: while ( $args{gather_while}->() ) { - my $sleep = 1 - fmod( scalar(Time::HiRes::gettimeofday()), 1 ); + my $time_of_day = scalar(Time::HiRes::gettimeofday()); + my $sleep = $sample_interval + - fmod( $time_of_day, $sample_interval ); Time::HiRes::sleep($sleep); open my $diskstats_fh, "<", "/proc/diskstats" @@ -309,9 +318,11 @@ sub gather_samples { my @to_print = timestamp(); push @to_print, <$diskstats_fh>; - # Lovely little method from IO::Handle: turns on autoflush, - # prints, and then restores the original autoflush state. - print { $fh } @to_print; + for my $fh ( @fhs ) { + # Lovely little method from IO::Handle: turns on autoflush, + # prints, and then restores the original autoflush state. + print { $fh } @to_print; + } close $diskstats_fh or die $OS_ERROR; $samples++; @@ -320,7 +331,10 @@ sub gather_samples { last GATHER_DATA; } } - close $fh or die $OS_ERROR; + pop @fhs; # STDOUT + for my $fh ( @fhs ) { + close $fh or die $OS_ERROR; + } return; } @@ -396,7 +410,7 @@ sub help { my $column_re = $args{OptionParser}->get('columns-regex'); my $device_re = $args{OptionParser}->get('devices-regex'); my $interval = $obj->sample_time() || '(none)'; - my $disp_int = $args{OptionParser}->get('refresh-interval'); + my $disp_int = $args{OptionParser}->get('interval'); my $inact_disk = $obj->show_inactive() ? 'no' : 'yes'; for my $re ( $column_re, $device_re ) { @@ -520,9 +534,8 @@ sub pause { sub timestamp { # TS timestamp.nanoseconds ISO8601-timestamp - my ( $seconds, $microseconds ) = Time::HiRes::gettimeofday(); - return sprintf( "TS %d.%d %s\n", $seconds, - $microseconds*1000, Transformers::ts($seconds) ); + my ($s, $m) = Time::HiRes::gettimeofday(); + return sprintf( "TS %d.%09d %s\n", $s, $m*1000, Transformers::ts( $s ) ); } sub _d {