xenserver / xscontainer

Support for Docker and Container Management
BSD 2-Clause "Simplified" License
25 stars 14 forks source link

xscontainer: ignore SIG_PIPE #51

Closed naichuans closed 5 years ago

naichuans commented 5 years ago

During the dundeedockerlinux test, there are many SIG_PIPE throw out because pipe broken, which will cause xscontainer-monitor exit. Ignore the single to avoid the failure.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 45.111% when pulling deae3969b8b6908410429c274373b4a8e8e3a9d0 on naichuans:ignore_SIG_PIPE into 9ebb074a4bee68ccef9224b597ac74c34c41d51f on xenserver:master.

robertbreker commented 5 years ago

Thanks for this @naichuans.

My understanding is that:

Why do you do this, instead of handling the IOErrors locally (and failing the request) in https://github.com/xenserver/xscontainer/blob/master/src/xscontainer/remote_helper/ssh.py?

naichuans commented 5 years ago

Thanks for this @naichuans.

My understanding is that:

  • python's default handler of SIGPIPE just throws IOErrors.-
  • With your change signal(SIGPIPE, SIG_IGN), python is not throwing IOErrors anymore, when SIPPIPE occurs. Is this correct?

Why do you do this, instead of handling the IOErrors locally (and failing the request) in https://github.com/xenserver/xscontainer/blob/master/src/xscontainer/remote_helper/ssh.py?

Hi, @robertbreker . You are right, I will ignore SIG_PIPE in the code. We can catch the IOErrors, but there is nothing we can do. It is more like a performance issue. Maybe we could add some error logs?

robertbreker commented 5 years ago

So if SIG_PIPE is ignored in the code (with this PR), are the IOErrors still thrown? I think we do need the IOerrors, so we log something and don't assume a request was submitted, when it wasn't due to SIG_PIPE.

naichuans commented 5 years ago

Hi, @robertbreker, I'm afraid there is no IOError throw out during the test. We just received a SIG_PIPE and exit. Could we add a SIG_PIPE process function with some logs?

naichuans commented 5 years ago

Hi, @robertbreker , any suggestion about that?

naichuans commented 5 years ago

@robertbreker , sorry, maybe I misunderstood your meaning. How about catch the signal and throw out an IOError exception?

robertbreker commented 5 years ago

I think catching the signal and throwing out an IOError exception, would be the perfect solution.