scroogie / pdsh

Automatically exported from code.google.com/p/pdsh
0 stars 0 forks source link

Undesirable change in 2.28 when there is no host output. #54

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
We recently upgraded from 2.23 to 2.28 and a change that was introduced broke 
our use case.  Before, this was the expected result and output of a command 
that had no output from the host:

[root@wtm-32vm1 ~]# pdsh -V
pdsh-2.23 (+readline)
rcmd modules: mrsh
misc modules: (none)
[root@wtm-32vm1 ~]# pdsh -t 120 -S -Rmrsh -w wtm-16vm1,wtm-16vm2,wtm-16vm7 -l 
root cd ..
[root@wtm-32vm1 ~]#

Now, we are getting this output:

[root@wtm-16vm1 ~]# pdsh -V
pdsh-2.28 (+readline)
rcmd modules: ssh,rsh,mrsh (default: mrsh)
misc modules: (none)
[root@wtm-16vm1 ~]# pdsh -t 120 -S -Rrsh -w wtm-16vm1,wtm-16vm2,wtm-16vm7 -l 
root cd ..
wtm-16vm2: wtm-16vm1: wtm-16vm7: [root@wtm-16vm1 ~]#

I do not know the code well enough to know when/where this change happened.  I 
did see this commit:

c37f0b533b93c0d6825df83dc3035ec54a1ce3e8 Use a dynamic buffer for output to 
avoid truncation

Maybe the length of the buffer is not being checked correctly, and it being 
printed, even when empty?

If you need me to do testing, please let me know: this is a blocker for us 
upgrading.

Original issue reported on code.google.com by jos...@whamcloud.com on 11 Feb 2013 at 9:38

GoogleCodeExporter commented 8 years ago
Thanks, this is a good find. It will be a severe bug for us as well because
running pdsh where there is output from only some nodes is a common idiom for
us.

Do you know if this issue only happens with rsh? (Have you tried mrsh/ssh/exec)

mark

Original comment by mark.gro...@gmail.com on 11 Feb 2013 at 9:58

GoogleCodeExporter commented 8 years ago
Ah, the -S option appears to be required to recreate this problem:

grondo@hype355:~/git/pdsh.git/src/pdsh$ pdsh -S -w hype[210-213] cd ..
hype210: hype212: hype211: hype213: grondo@hype355:~/git/pdsh.git/src/pdsh$ 
grondo@hype355:~/git/pdsh.git/src/pdsh$ pdsh -w hype[210-213] cd ..

and I cannot recreate the problem while using the 'exec' module (which is 
unfortunate
for writing a testcase, but another good clue here)

To collect return codes from remote processes the rsh and mrsh modules use a 
special
string embedded at the beginning of a line. If there is otherwise no output, 
then
when this string is removed it leaves an empty line sans newline, so we get the
unwanted behavior.

I'll try to find exactly when this broke.

Original comment by mark.gro...@gmail.com on 11 Feb 2013 at 10:15

GoogleCodeExporter commented 8 years ago
So far, only tested with mrsh. Do not know if it happens with rsh or ssh.

Original comment by jos...@whamcloud.com on 11 Feb 2013 at 10:50

GoogleCodeExporter commented 8 years ago
Yes, git bisect verified your guess above that commit 
c37f0b533b93c0d6825df83dc3035ec54a1ce3e8 was the culprit.
Now comparing that change with preexisting code, I do see
that an if (strlen(buf) > 0) was dropped.

Thanks, this will be fixed in pdsh-2.29

Original comment by mark.gro...@gmail.com on 11 Feb 2013 at 11:16

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 60b39484c3c9.

Original comment by mark.gro...@gmail.com on 11 Feb 2013 at 11:20

GoogleCodeExporter commented 8 years ago
Joshua:

If you are willing can you verify the fix applied in revision 60b39484c3c9?

Original comment by mark.gro...@gmail.com on 11 Feb 2013 at 11:26

GoogleCodeExporter commented 8 years ago
Just built and tested on el5 {i686|x86_64} and el6 {i686|x86_64}. Did not test 
suse11, but I can't imagine why it would be any different. The behavior is as 
expected.

How soon do you think this will be made into a release?

Original comment by jos...@whamcloud.com on 12 Feb 2013 at 2:20

GoogleCodeExporter commented 8 years ago
pdsh-2.29 should be out within the next day or so.

Original comment by mark.gro...@gmail.com on 13 Feb 2013 at 12:18

GoogleCodeExporter commented 8 years ago
I've uploaded pdsh-2.29

http://pdsh.googlecode.com/files/pdsh-2.29.tar.bz2

Original comment by mark.gro...@gmail.com on 13 Feb 2013 at 6:06