Re: [Maria-developers] 0d05c9f8699: Add mytop to Client component for rpm package

2020-10-13 Thread Jean Weisbuch 
I think the version number could be bumped to clearly not that there are 
major changes and that it's not a MariaDB specific version.


On the commit, you have used tabs instead of spaces on the rest of the 
code).



On 10/13/20 2:39 PM, Anel Husakovic wrote:

Hi Serg,
Added new commit addressing your review, thanks:
https://github.com/MariaDB/server/commit/660374b542639642e825079fd065333f7531cde9

On Thu, Oct 8, 2020 at 3:24 PM Sergei Golubchik > wrote:


Hi, Anel!

About the diff to 1.9, I tend to agree that our mytop is a superset.
I did have some comments when looking at the diff, couldn't help it :)
Here they are:

> diff --git a/scripts/mytop.sh b/../mytop-1.9.1-4
> index 321b79537fc..056b7a9a806 100644
> --- b/../mytop-1.9.1-4
> +++ a/scripts/mytop.sh
> @@ -1,13 +1,13 @@
> -#!/usr/bin/perl -w
> +#!/usr/bin/env perl

I thought we're getting rid of "env perl" ?


Yap, added in patch

>  #
> -# $Id: mytop,v 1.91 2012/01/18 16:49:12 mgrennan Exp $
> +# $Id: mytop,v 1.99-maria6 2019/10/22 14:53:51 jweisbuch Exp $
>
>  =pod
>
>  =head1 NAME
>
> -mytop - display MySQL server performance info like `top'
> +mytop - display MariaDB server performance info like `top'

may be "MariaDB/MySQL" here and below?


Added.

>
>  =cut
>
> @@ -242,11 +253,15 @@ my $dsn;
>
>  ## Socket takes precedence.
>
> -$dsn
="DBI:mysql:database=$config{db};mysql_read_default_group=mytop;";
> +if (eval {DBI->install_driver("MariaDB")}) {
> +  $dsn =
"DBI:MariaDB:database=$config{db};mariadb_read_default_group=mytop;";
> +} else {
> +  $dsn =
"DBI:mysql:database=$config{db};mysql_read_default_group=mytop;";
> +}
>
>  if ($config{socket} and -S $config{socket})
>  {
> -    $dsn .= "mysql_socket=$config{socket}";
> +    $dsn .= "mariadb_socket=$config{socket}";

if you support both drivers, you need to use the correct option here

Added.

>  }
>  else
>  {
> @@ -1027,10 +918,15 @@ sub GetData()
>              }
>          }
>
> -        open L, " -        my $l = ;
> -        close L;
> -        chomp $l;
> +        my $l;
> +        if (-e "/proc/loadavg")
> +        {
> +            ## To avoid warnings if the OS is not Linux
> +            open (my $fh, "<", "/proc/loadavg");
> +            ## Only the first 3 values are interresting

rr?

> +            $l = join(" ", (split /\s+/, <$fh>)[0..2]);
> +            close $fh;
> +        }
>
>          $last_time = $now_time;
>
> @@ -1960,11 +1694,15 @@ sub PrintHelp()
>    S - change slow query hightlighting
>    t - switch to thread view (default)
>    u - show only a specific user
> +  V - show variables
>    : - enter a command (not yet implemented)
>    ! - Skip an error that has stopped replications (at your own
risk)
> +  L - show full queries (do not strip to terminal width)
> +  w - adjust the User and DB columns width
> +  a - toggle the progress column
>
> -${GREEN}http://jeremy.zawodny.com/mysql/mytop/${RESET}

> -${GREEN}http://www.mysqlfanboy.com/mytop-3/${RESET}

> +Base version from
${GREEN}http://www.mysqlfanboy.com/mytop-3${RESET}


if it's mytop-3, perhaps it should have the version 3.x
and not 1.99-maria6 ?


The mytop-3 is just a url. The same url has 1.9.1 version as the 
latest one. 1.99 is from PR #215 that the author contributed (9 
iterations/commits in some time frame) and it is ok from my point of view.


> +This version comes as part of the ${GREEN}MariaDB${RESET}
distribution.
>  ];
>
>      print $help;


Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org 

___
Mailing list: https://launchpad.net/~maria-developers
Post to     : maria-developers@lists.launchpad.net

Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp

Re: [Maria-developers] 0d05c9f8699: Add mytop to Client component for rpm package

2020-10-13 Thread Anel Husakovic 
Hi Serg,
Added new commit addressing your review, thanks:
https://github.com/MariaDB/server/commit/660374b542639642e825079fd065333f7531cde9

On Thu, Oct 8, 2020 at 3:24 PM Sergei Golubchik  wrote:

> Hi, Anel!
>
> About the diff to 1.9, I tend to agree that our mytop is a superset.
> I did have some comments when looking at the diff, couldn't help it :)
> Here they are:
>
> > diff --git a/scripts/mytop.sh b/../mytop-1.9.1-4
> > index 321b79537fc..056b7a9a806 100644
> > --- b/../mytop-1.9.1-4
> > +++ a/scripts/mytop.sh
> > @@ -1,13 +1,13 @@
> > -#!/usr/bin/perl -w
> > +#!/usr/bin/env perl
>
> I thought we're getting rid of "env perl" ?
>

Yap, added in patch

> >  #
> > -# $Id: mytop,v 1.91 2012/01/18 16:49:12 mgrennan Exp $
> > +# $Id: mytop,v 1.99-maria6 2019/10/22 14:53:51 jweisbuch Exp $
> >
> >  =pod
> >
> >  =head1 NAME
> >
> > -mytop - display MySQL server performance info like `top'
> > +mytop - display MariaDB server performance info like `top'
>
> may be "MariaDB/MySQL" here and below?
>

Added.

> >
> >  =cut
> >
> > @@ -242,11 +253,15 @@ my $dsn;
> >
> >  ## Socket takes precedence.
> >
> > -$dsn ="DBI:mysql:database=$config{db};mysql_read_default_group=mytop;";
> > +if (eval {DBI->install_driver("MariaDB")}) {
> > +  $dsn =
> "DBI:MariaDB:database=$config{db};mariadb_read_default_group=mytop;";
> > +} else {
> > +  $dsn =
> "DBI:mysql:database=$config{db};mysql_read_default_group=mytop;";
> > +}
> >
> >  if ($config{socket} and -S $config{socket})
> >  {
> > -$dsn .= "mysql_socket=$config{socket}";
> > +$dsn .= "mariadb_socket=$config{socket}";
>
> if you support both drivers, you need to use the correct option here
>

Added.

> >  }
> >  else
> >  {
> > @@ -1027,10 +918,15 @@ sub GetData()
> >  }
> >  }
> >
> > -open L, " > -my $l = ;
> > -close L;
> > -chomp $l;
> > +my $l;
> > +if (-e "/proc/loadavg")
> > +{
> > +## To avoid warnings if the OS is not Linux
> > +open (my $fh, "<", "/proc/loadavg");
> > +## Only the first 3 values are interresting
>
> rr?
>
> > +$l = join(" ", (split /\s+/, <$fh>)[0..2]);
> > +close $fh;
> > +}
> >
> >  $last_time = $now_time;
> >
> > @@ -1960,11 +1694,15 @@ sub PrintHelp()
> >S - change slow query hightlighting
> >t - switch to thread view (default)
> >u - show only a specific user
> > +  V - show variables
> >: - enter a command (not yet implemented)
> >! - Skip an error that has stopped replications (at your own risk)
> > +  L - show full queries (do not strip to terminal width)
> > +  w - adjust the User and DB columns width
> > +  a - toggle the progress column
> >
> > -${GREEN}http://jeremy.zawodny.com/mysql/mytop/${RESET}
> > -${GREEN}http://www.mysqlfanboy.com/mytop-3/${RESET}
> > +Base version from ${GREEN}http://www.mysqlfanboy.com/mytop-3${RESET}
>
> if it's mytop-3, perhaps it should have the version 3.x
> and not 1.99-maria6 ?
>

The mytop-3 is just a url. The same url has 1.9.1 version as the latest
one. 1.99 is from PR #215 that the author contributed (9 iterations/commits
in some time frame) and it is ok from my point of view.

> +This version comes as part of the ${GREEN}MariaDB${RESET} distribution.
> >  ];
> >
> >  print $help;
>
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and secur...@mariadb.org
>
> ___
> Mailing list: https://launchpad.net/~maria-developers
> Post to : maria-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp