[PATCH] make flapping smbcontrol test more informative

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Tue Feb 27 00:54:41 UTC 2018


On 27/02/18 11:38, Jeremy Allison via samba-technical wrote:
> On Tue, Feb 27, 2018 at 07:11:18AM +1300, Gary Lockyer via samba-technical wrote:
>> Ouch this was added for the pre-fork process model work and it was
>> intended to catch gross failures in the start up.
>>
>> So it could just be limited to just the prefork test environments,
>> currently only addc_no_ntlm.
>>
>> My preference though would be to get it working for the standard model
>> as well, but I'm unsure how to do this. Any thoughts, suggestions.
> 
> If you've got a TOC/TOU race it's difficult to see
> how this test can survive in its current form.
> 
> To make it reliable you must eliminate the race,
> and irpc_all_servers() is only ever going to
> get you a 'snapshot' of processes available to
> ping.

Yes, though in this case, we could probably do this:

         this_is_a_forky_environment = whatever()         
         we_have_seen_a_pingable_ldap_server = False

         processes = self.msg_ctx.irpc_all_servers()
         for p in processes:
             for id in p.ids:
                 if p.name != "samba":
                     try: 
                         self.check_run("%s %d %s" % (COMMAND, id.pid, PING),
                                        msg="trying to ping %s" % p.name)
                     except BlackboxProcessError as e:
                         if (this_is_a_forky_environment and
                             p.name == 'ldap_server'):
                             continue
                         else:
                             raise

                     if p.name == 'ldap_server':
                         we_have_seen_a_pingable_ldap_server = True

         self.assertTrue(we_have_seen_a_pingable_ldap_server,
                         "no long-lived ldap_server was found")

because we only need to know that there is one permanent ldap server.

Another way would be to rename the child ldap_servers as they fork, to
(say) 'ldap_server-transient', then ignore all of those. But it
doesn't seem worth it.

Douglas

> 
>> On 25/02/18 22:35, Ralph Böhme via samba-technical wrote:
>>> On Sun, Feb 25, 2018 at 09:07:06PM +1300, Douglas Bagnall via samba-technical wrote:
>>>> And then I thought: with multi-process fork-on-demand LDAP, we should
>>>> expect a race between listing the processes and pinging them. They 
>>>> disappear all the time.
>>>>
>>>>         processes = self.msg_ctx.irpc_all_servers()
>>>>         for p in processes:
>>>>             for id in p.ids:
>>>>                 if p.name != "samba":
>>>>                     self.check_run("%s %d %s" % (COMMAND, id.pid, PING),
>>>>                                    msg="trying to ping %s" % p.name)
>>>>
>>>> so I was wrong here:
>>>>
>>>>>>> This is a canary, I suspect -- the test itself is sound (sort of),
>>>>
>>>> the test is not sound.
>>>
>>> hehe, ouch! Good catch! :)
>>>
>>> -slow
>>>
>>
> 
> 
> 
> 




More information about the samba-technical mailing list