On Thu, Mar 11, 2010 at 02:45:38AM +0200, Marian Marinov wrote:
> # HG changeset patch
> # User Marian Marinov <[email protected]>
> # Date 1268266846 -7200
> # Branch mysql-ms
> # Node ID 31ea5ae9c8e773edb89648cf482907ff3028d456
> # Parent  7d5becb0cbd1bf5b3558dd85dede99d7f3ef2a3c
> Medium: RA: mysql: fixed the 'while read k v' problem
> 
> Fixed all places where I used while read k v for getting the values.
> Now I simply change the IFS(input field separator) and get everything line by 
> line.

But there is absolutely no need for that,
it makes life more difficult without any reason.

The values returned by those mysql commands do not contain white-space 
characters,
they are in a perfectly defined order, with -B they are tab separated,
and -N leaves off the column names.

so please simply go the set -- $(mysql) route.
please.

> diff -r 7d5becb0cbd1 -r 31ea5ae9c8e7 heartbeat/mysql
> --- a/heartbeat/mysql Tue Mar 02 14:43:57 2010 +0100
> +++ b/heartbeat/mysql Thu Mar 11 02:20:46 2010 +0200
> @@ -337,14 +337,12 @@
>      master_port=''
>      slave_sql=''
>      slave_io=''
> -    mysql \
> -        --socket=$OCF_RESKEY_socket -O connect_timeout=1 \
> -        -e 'SHOW SLAVE STATUS\G'|grep -P 'Running|Master_[UHP]'|while read k 
> v; do
> -        if [ "$k" = 'Master_Host:' ]; then master_host="$v"; fi
> -        if [ "$k" = 'Master_User:' ]; then master_user="$v"; fi
> -        if [ "$k" = 'Master_Port:' ]; then master_port="$v"; fi
> -        if [ "$k" = 'Slave_IO_Running:' ]; then slave_io="$v"; fi
> -        if [ "$k" = 'Slave_SQL_Running:' ]; then slave_sql="$v"; fi
> +    for i in $(mysql -S $OCF_RESKEY_socket -O connect_timeout=1 -e 'SHOW 
> SLAVE STATUS\G'|grep -P 'Running|Master_[UHP]'); do

Don't do "grep -P", there are _many_ greps out there not supporting
perl-regexp, and you don't even need it here. if you don't want to type
backslashes, -E would be enough (and is more widely supported), or even
-e Running -e "Master_[UHP]".

> +     if ( echo $i | grep 'Master_Host:' > /dev/null ); then 
> master_host=$(echo $i|sed 's/^.*: //'); fi

as the way you try it to do i only contains _either_ the column label
_or_ the value, this all does not work.

>       if [ -z "$master_file" ] || [ -z "$master_pos" ]; then
> -             ocf_log err "unable to find master file or master position"
> -             return $OCF_ERR_GENERIC;
> +             ocf_log warn "unable to find master file or master position"
> +             return $OCF_SUCCESS;

You may consider to reduce your master score each time you cannot reach
the master from monitor by an arbitrary amount,
because it _may_ be that some other slave can still reach the master.
If the master then dies later, and the last time this slave updated
its score was a high score, and the other slaves that have still been
able to reach the master have been lagging behind, and thus had a
slightly lower score, you'd promote a slave that has not been updated
for some time.

But this is all a bit tricky, with these score updates only every
monitoring intervall, you cannot possibly get it "right", you can only
document the expected behaviour in the various failure scenarios.

>       fi
>  
>       local_log=''
>       local_pos=''
>       exec_pos=''
> -     mysql --socket=$OCF_RESKEY_socket -O connect_timeout=1 \
> -             -e 'SHOW SLAVE STATUS\G'|grep Master_Log|while read k v; do
> -             if [ "$k" = 'Master_Log_File:' ]; then local_log="$v"; fi
> -             if [ "$k" = 'Read_Master_Log_Pos:' ]; then local_pos="$v"; fi
> -             if [ "$k" = 'Exec_Master_Log_Pos:' ]; then exec_pos="$v"; fi
> +     IFS=$'\n'
> +     for i in $(mysql -S $OCF_RESKEY_socket -O connect_timeout=1 -e 'SHOW 
> SLAVE STATUS\G'|grep Master_Log); do

Maybe we can reduce the number of same queries in one RA run,
by "caching" their output in some shell variable?
I did not check the exact control flow, though.

> +     if ( echo $i | grep 'Master_Log_File:' > /dev/null ); then 
> local_log=$(echo $i|sed 's/^.*: //'); fi
> +         if ( echo $i | grep 'Read_Master_Log_Pos:' > /dev/null ); then 
> local_pos=$(echo $i|sed 's/^.*: //'); fi
> +         if ( echo $i | grep 'Exec_Master_Log_Pos:' > /dev/null ); then 
> exec_pos=$(echo $i|sed 's/^.*: //'); break; fi
>       done
> +     IFS="$oldIFS"
>  
>       if [ -z "$local_log" ] || [ -z "$local_pos" ] || [ -z "$exec_pos" ]; 
> then
> -             ocf_log err "unable to get slave status values"
> -             return $OCF_ERR_GENERIC
> +             ocf_log warn "unable to get slave status values"
> +             return $OCF_SUCCESS

Hm. If you are unable to get the _slave_ status,
I'd suggest that is an OCF_ERR_GENERIC. This is the instance you are
monitoring. It should answer, and it is supposed to know where it is.

>       fi
>  
>       # no preference for very old slaves
>       if [ "$master_file" != "$local_log" ]; then
>               ocf_run $CRM_MASTER -v 0
> -             return 1;
> +             return $OCF_SUCCESS;

what is the rational behind this?

>       fi

-- 
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to