'for' cycle

applehalf · 11-18-2015, 09:31 AM

Hello, my friends.
I made a little script with 'for' cycle.
On different linux servers that script rounds different number of times.
Is it real to identify how many times was asked command during script was running?

Script:

Code:

time for j in `sudo multipath -ll | grep 'MY_STORAGE_NAME' | awk '{print $1}'`;do echo $(echo $j; echo $(for i in `sudo multipath -ll|grep 'MY_STORAGE_NAME'|awk '{print $2}'|awk -F \( '{print$2}'|awk -F \) '{print $1}'`;do echo $(sudo multipath -a $i -ll |grep sd |awk '{print $3}';sudo multipath -a $i -ll | grep 'MY_STORAGE_NAME'|awk '{print $1}');done |awk '{for(k=1; k<=NF; k++) print $k,$NF}'|grep sd | awk '{if($2=="'"$j"'") print $0;}'|awk '{print $1}'));done

How many times command multipath was running if it is N disks from storage?

simple output is:
DISK1 sdb sdw sdg sdab sdl sdag sdq sdal
DISK2 sdc sdx sdh sdac sdm sdah sdr sdam
DISK3 sda sdv sdf sdaa sdk sdaf sdp sdak
DISK4 sdd sdy sdi sdad sdn sdai sds sdan
DISK5 sde sdz sdj sdae sdo sdaj sdt sdao

command 'time sudo multipath -ll' performed for 0.1s
script performed for more than 3s
commands 'awk' and 'grep' It does not make any significant change on lead time of script.

grail · 11-18-2015, 09:49 AM

Please use [code][/code] tags around code and data.

As for the code, my god what a nightmare. If this is run on any server I would be extremely grateful to not be the admin. This is extremely un-maintainable code.

Apart from that, I am not sure I understand what your question is? The timings will depend on the individual machines, so really how many times a particular command can only really be measured by yourself.

Maybe you could add some additional details on what it is you want or need?

suicidaleggroll · 11-18-2015, 10:08 AM

Quote:

Originally Posted by grail

As for the code, my god what a nightmare.

My thoughts exactly

rtmistler · 11-18-2015, 12:13 PM

The point of writing code of any type is to organize it in a logical fashion, make it readable, reusable, and maintainable. What you've written appears to be none of those.

What exactly are you trying to accomplish with your efforts? In summary English, not code explanations? I'm asking because many times there are a variety of ways to do things and you may be unsuccessfully trying to do something where there is a better set of ways to do so.

berndbausch · 11-18-2015, 04:03 PM

Quote:

Originally Posted by applehalf

How many times command multipath was running if it is N disks from storage?

I'd say N*N times at least, which seems to be confirmed by your calculation, but I have given up dissecting your program somewhere in the middle.
Please use code tags (the button with the "#" sign on it) and indentation when posting programs.

I don't know what your program is supposed to achieve, but there must be a simpler way.

chrism01 · 11-18-2015, 06:27 PM

Link to code tags howto https://www.linuxquestions.org/quest...do=bbcode#code

applehalf · 11-19-2015, 02:18 AM

I understand that scare you this piece of code, but the code is entirely worse.
I want to recieve information about which disk from storage has at least one path's await more than 10 miliseconds.
So all code now is:

Code:

echo $(for j in `sudo multipath -ll | grep 'MY_STORAGE_NAME'| awk '{print $1}'`;do echo $(echo $j; echo $(for i in `sudo multipath -ll|grep 'MY_STORAGE_NAME'|awk '{print $2}'|awk -F \( '{print$2}'|awk -F \) '{print $1}'`;do echo $(sudo multipath -a $i -ll |grep sd |awk '{print $3}';sudo multipath -a $i -ll | grep 'MY_STORAGE_NAME'|awk '{print $1}');done |awk '{for(k=1; k<=NF; k++) print $k,$NF}'|grep sd | awk '{if($2=="'"$j"'") print $0;}'|awk '{print $1}'))|grep -E `echo \'$(iostat -kNx 1 2 |grep sd|tail -\`iostat -kNx |grep sd|wc -l\`| awk '{if ($10>10) print $1}'|awk 'NR > 1{print line"[[:space:]]|"}{line=$0;}END{print $0"[[:space:]]"}')\'|sed s/\ //g`|awk '{print $1}';done)

Maybe it is much easer way to get that information?
That script performed more than 5 seconds.

grail · 11-19-2015, 02:47 AM

I have no idea about multipath command, so can you give us an example output from:

Code:

multipath -ll
# and
multipath -a var -ll

Because all of those useless greps can go in one hit and I am positive you would not need all of those awks, and although I may have missed it as again this code is a nightmare, I could not find where the $j variable
from the first loop was ever used.

berndbausch · 11-19-2015, 03:07 AM

Quote:

Originally Posted by applehalf

I understand that scare you this piece of code, but the code is entirely worse.
I want to recieve information about which disk from storage has at least one path's await more than 10 miliseconds.
So all code now is:

Code:

echo $(for j in `sudo multipath -ll | grep 'MY_STORAGE_NAME'| awk '{print $1}'`;do echo $(echo $j; echo $(for i in `sudo multipath -ll|grep 'MY_STORAGE_NAME'|awk '{print $2}'|awk -F \( '{print$2}'|awk -F \) '{print $1}'`;do echo $(sudo multipath -a $i -ll |grep sd |awk '{print $3}';sudo multipath -a $i -ll | grep 'MY_STORAGE_NAME'|awk '{print $1}');done |awk '{for(k=1; k<=NF; k++) print $k,$NF}'|grep sd | awk '{if($2=="'"$j"'") print $0;}'|awk '{print $1}'))|grep -E `echo \'$(iostat -kNx 1 2 |grep sd|tail -\`iostat -kNx |grep sd|wc -l\`| awk '{if ($10>10) print $1}'|awk 'NR > 1{print line"[[:space:]]|"}{line=$0;}END{print $0"[[:space:]]"}')\'|sed s/\ //g`|awk '{print $1}';done)

Maybe it is much easer way to get that information?
That script performed more than 5 seconds.

Congratulations!
Perhaps it performs better (at least human reviewers perform better) if it's not all on one line?
And indented to reveal the structure better?

As I said, I think this thing has at least quadratic complexity, so it's not surprising the time goes up fast.

What is it supposed to do?

EDIT: I think this is what it is supposed to do:

Quote:

Originally Posted by applehalf

information about which disk from storage has at least one path's await more than 10 miliseconds

I don't quite understand though. Do you mean there are wait times on paths? How do you measure them?

EDIT2: Oh, you use iostat's await column. Well, you run iostat -kNx 1 2, which takes two seconds already. And a multitude of multipath commands in nested loops - no wonder it ends up being 5 secs.

Can you show us the output of a single multipath -ll command? I have a feeling this script has potential for radical simplification.

applehalf · 11-19-2015, 04:44 AM

'iostat -kNx 1 2, which takes two seconds already'
It's wrong. This is the output two times in 1 second. First output is statistic for long time, second output for last 1 second:
the output of 'multipath -ll':

DISK1 (UUID_OF_DISK1) dm-1 MY_STORAGE_NAME
size=5.0G features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:1 sdb 8:16 active ready running
|- 3:0:6:1 sdw 65:96 active ready running
|- 2:0:20:1 sdg 8:96 active ready running
|- 3:0:13:1 sdab 65:176 active ready running
|- 2:0:21:1 sdl 8:176 active ready running
|- 3:0:27:1 sdag 66:0 active ready running
|- 2:0:22:1 sdq 65:0 active ready running
`- 3:0:29:1 sdal 66:80 active ready running
DISK2 (UUID_OF_DISK2) dm-2 MY_STORAGE_NAME
size=500G features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:2 sdc 8:32 active ready running
|- 3:0:6:2 sdx 65:112 active ready running
|- 2:0:20:2 sdh 8:112 active ready running
|- 3:0:13:2 sdac 65:192 active ready running
|- 2:0:21:2 sdm 8:192 active ready running
|- 3:0:27:2 sdah 66:16 active ready running
|- 2:0:22:2 sdr 65:16 active ready running
`- 3:0:29:2 sdam 66:96 active ready running
DISK3 (UUID_OF_DISK3) dm-0 MY_STORAGE_NAME
size=5.0G features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:0 sda 8:0 active ready running
|- 3:0:6:0 sdv 65:80 active ready running
|- 2:0:20:0 sdf 8:80 active ready running
|- 3:0:13:0 sdaa 65:160 active ready running
|- 2:0:21:0 sdk 8:160 active ready running
|- 3:0:27:0 sdaf 65:240 active ready running
|- 2:0:22:0 sdp 8:240 active ready running
`- 3:0:29:0 sdak 66:64 active ready running
DISK4 (UUID_OF_DISK4) dm-3 MY_STORAGE_NAME
size=4.0T features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:3 sdd 8:48 active ready running
|- 3:0:6:3 sdy 65:128 active ready running
|- 2:0:20:3 sdi 8:128 active ready running
|- 3:0:13:3 sdad 65:208 active ready running
|- 2:0:21:3 sdn 8:208 active ready running
|- 3:0:27:3 sdai 66:32 active ready running
|- 2:0:22:3 sds 65:32 active ready running
`- 3:0:29:3 sdan 66:112 active ready running
DISK5 (UUID_OF_DISK5) dm-4 MY_STORAGE_NAME
size=4.0T features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:4 sde 8:64 active ready running
|- 3:0:6:4 sdz 65:144 active ready running
|- 2:0:20:4 sdj 8:144 active ready running
|- 3:0:13:4 sdae 65:224 active ready running
|- 2:0:21:4 sdo 8:224 active ready running
|- 3:0:27:4 sdaj 66:48 active ready running
|- 2:0:22:4 sdt 65:48 active ready running
`- 3:0:29:4 sdao 66:128 active ready running

the result of comand 'multipath -a UUID_OF_DISK1 -ll' will be:
DISK1 (UUID_OF_DISK1) dm-1 MY_STORAGE_NAME
size=5.0G features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:18:1 sdb 8:16 active ready running
|- 3:0:6:1 sdw 65:96 active ready running
|- 2:0:20:1 sdg 8:96 active ready running
|- 3:0:13:1 sdab 65:176 active ready running
|- 2:0:21:1 sdl 8:176 active ready running
|- 3:0:27:1 sdag 66:0 active ready running
|- 2:0:22:1 sdq 65:0 active ready running
`- 3:0:29:1 sdal 66:80 active ready running

Does anyone can see variable of my script, which will be much simplier?

grail · 11-19-2015, 07:45 AM

As said previously, please use code tags for data as well as code, it will maintain the formatting which may be important to some commands being used.

Can i also ask, is there some sort of sadistic pleasure in mixing up both `` and $(). My suggestion would be to stick with $() as these can be nested if needed.

applehalf · 11-19-2015, 08:52 AM

I modified all `` to $()

Code:

echo $(for j in $(sudo multipath -ll | grep MY_STORAGE_NAME| awk '{print $1}')
	   do echo $(echo $j
				 echo $(for i in $(sudo multipath -ll|
								   grep MY_STORAGE_NAME|awk '{print $2}'|awk -F \( '{print$2}'|awk -F \) '{print $1}')
						do echo $(sudo multipath -a $i -ll |grep sd |awk '{print $3}'
								  sudo multipath -a $i -ll | grep MY_STORAGE_NAME|awk '{print $1}')
						done |
				 awk '{for(k=1; k<=NF; k++) print $k,$NF}'|grep sd | awk '{if($2=="'"$j"'") print $0;}'|awk '{print $1}'))|
				 grep -E $(echo \'$(iostat -kNx 1 2 |grep sd|tail -$(iostat -kNx |grep sd|wc -l)|
				                   awk '{if($10>10) print $1}'|
								   awk 'NR > 1{print line"[[:space:]]|"}{line=$0;}END{print $0"[[:space:]]"}')\'|
				 sed s/\ //g)|
	   awk '{print $1}';done)

I think it is more readable.

berndbausch · 11-19-2015, 09:07 AM

Quote:

Originally Posted by applehalf

I think it is more readable.

Yes but it's easier to start from scratch.

So you want a report that

maps DISK1/2/3 etc to its paths and
points out those paths that have a high "await" value in iostat

For the first task, something like (I am paraphrasing, you will have to provide what's missing)

Code:

echo "Multipath $DISK"
multipath -ll $DISK | grep running | cut -d' ' -f3

You can write that to a file I will name report1; then, to accomplish task 2, something like

Code:

iostat -kNx 1 2 $(grep -v "^Multipath" report1) | awk '$10>10 { print "device",$1,"await",$10 }' >report2

Note: Not tested.
The iostat will still take two seconds, but the rest will be much faster. And slightly easier to maintain than your code.
Of course, if you want the names of the multipaths in report 2, more work is required.

grail · 11-19-2015, 09:46 AM

I am curious as to which stick someone beat you with that you had to echo everything? All of your commands return data, hence the $() will return the same data, then using echo as an extra handler
to print it to screen seems a bit over the top.

Something else I found quite strange was the necessity to reprint the same data further inside the loops:

Code:

sudo multipath -ll | grep MY_STORAGE_NAME| awk '{print $1}'

sudo multipath -a $i -ll | grep MY_STORAGE_NAME|awk '{print $1}'

Although the first prints all names, such as DISK1, the second prints the exact same information that you retrieved at the start of the script.
Also, the simpler form would be:

Code:

sudo multipath -ll | awk '/MY_STORAGE_NAME/{print $1}'

And then there is this monstrosity:

Code:

sudo multipath -ll | grep MY_STORAGE_NAME|awk '{print $2}'|awk -F \( '{print$2}'|awk -F \) '{print $1}'

sudo multipath -ll | awk '/MY_STORAGE_NAME/{print gensub(/[()]/,"","g",$2)}'

As you can see, with a little thought you can reduce most of your code to much simpler and more understandable code.

berndbausch has pointed out some alternatives and I would agree that if you indicate the data you are starting with and what the desired output is there would be a much simpler method.

applehalf · 11-20-2015, 05:39 AM

awk '/MY_STORAGE_NAME/{print gensub(/[()]/,"","g")}' is very nice decision.
Ok once more time.
For example command iostat -kNx result is:
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await svctm util
sdc 0.00 0.00 0.00 0.00 0.00 0.00 10.77 0.00 1.80 1.78 0.00
sdt 0.00 0.00 0.00 0.00 0.00 0.00 10.98 0.00 0.56 0.54 0.00
sdg 0.00 0.00 0.00 0.00 0.00 0.00 10.56 0.00 1.19 1.18 0.00
sdx 0.00 0.00 0.00 0.00 0.00 0.00 11.12 0.00 20.51 0.51 0.00
sdk 0.00 0.00 0.00 0.00 0.00 0.00 11.31 0.00 0.73 0.73 0.00
sdab 0.00 0.00 0.00 0.00 0.00 0.00 11.00 0.00 30.66 0.64 0.00
sdo 0.00 0.00 0.00 0.00 0.00 0.00 11.38 0.00 0.87 0.86 0.00
sdaf 0.00 0.00 0.00 0.00 0.00 0.00 10.52 0.00 0.78 0.78 0.00

DISK5 (UUID_OF_DISK5) dm-1 FUJITSU,ETERNUS_DX8000
size=4.0T features='1 queue_if_no_path' hwhandler='0' wp=rw
`-+- policy='round-robin 0' prio=130 status=active
|- 2:0:15:2 sdc 8:32 active ready running
|- 3:0:12:2 sdt 65:48 active ready running
|- 2:0:16:2 sdg 8:96 active ready running
|- 3:0:15:2 sdx 65:112 active ready running
|- 2:0:17:2 sdk 8:160 active ready running
|- 3:0:20:2 sdab 65:176 active ready running
|- 2:0:18:2 sdo 8:224 active ready running
`- 3:0:21:2 sdaf 65:240 active ready running

I see that 2 pathes of one disk have await more than 10, so I want to recieve information about DISK5 (but only once, because if I have 20 disks with 16 paths each, and 10 disks have await much then 10 in all pathes, then I want to recieve 10 rows instead of 160).