12:06 p.m.
Nir Soffer has uploaded a new change for review.
Change subject: protocoldetector: Fix random double close
......................................................................
protocoldetector: Fix random double close
Previously we stopped the reactor before closing the acceptor. This
works most of the time, but once in a while, it fails with this error:
Traceback (most recent call last):
File "vdsm/tests/protocoldetectorTests.py", line 113, in tearDown
self.acceptor.stop()
File "vdsm/lib/vdsm/protocoldetector.py", line 211, in stop
self._acceptor.close()
File "/usr/lib64/python2.7/asyncore.py", line 407, in close
self.del_channel()
File "vdsm/lib/yajsonrpc/betterAsyncore.py", line 137, in del_channel
asyncore.dispatcher.del_channel(self, map)
File "/usr/lib64/python2.7/asyncore.py", line 292, in del_channel
del map[fd]
KeyError: 63
The flow of events leading to this error is:
1. Stooping reactor, signaling the reactor thread via eventfd
2. Context switch to the reactor thread
3. Reactor find it it was closed and close all dispatchers
4. Context switch to the original thread
5. Closing acceptor, fd is not in asyncore map
This patch fixes this issue by closing the acceptor before stopping the
reactor.
Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/protocoldetector.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/65187/1
diff --git a/lib/vdsm/protocoldetector.py b/lib/vdsm/protocoldetector.py
index 196a2ab..e505aa7 100644
--- a/lib/vdsm/protocoldetector.py
+++ b/lib/vdsm/protocoldetector.py
@@ -207,8 +207,8 @@
def stop(self):
self.log.debug("Stopping Acceptor")
- self._reactor.stop()
self._acceptor.close()
+ self._reactor.stop()
class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
12:06 p.m.
gerrit-hooks has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
2:48 a.m.
Piotr Kliczewski has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
PS1, Line 210: self._acceptor.close()
this is run when vdsm is being shutdown. I do not see any value by calling acceptor close
because reactor stop would close all the dispatchers including this one.
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
4:46 p.m.
Nir Soffer has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
Line 206: self._handlers.append(detector)
Line 207:
Line 208: def stop(self):
Line 209: self.log.debug("Stopping Acceptor")
Line 210: self._acceptor.close()
this is run when vdsm is being shutdown. I do not see any value by
calling
But this patch did not add acceptor.close(), I'm just fixing the order
so we don't get random double close. This issue is showing in the ci from time to time
and we should fix it.
This is the minimal fix required to fix this issue.
Line 211: self._reactor.stop()
Line 212:
Line 213:
Line 214: class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
4:32 p.m.
Nir Soffer has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1: Verified+1
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
5:15 a.m.
Piotr Kliczewski has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
Line 206: self._handlers.append(detector)
Line 207:
Line 208: def stop(self):
Line 209: self.log.debug("Stopping Acceptor")
Line 210: self._acceptor.close()
But this patch did not add acceptor.close(), I'm just fixing the
order so w
Minimal is to remove acceptor close line.
Line 211: self._reactor.stop()
Line 212:
Line 213:
Line 214: class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
3:41 p.m.
Nir Soffer has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
Line 206: self._handlers.append(detector)
Line 207:
Line 208: def stop(self):
Line 209: self.log.debug("Stopping Acceptor")
Line 210: self._acceptor.close()
Minimal is to remove acceptor close line.
No, this creates
broken stop method that stop the reactor instead of the acceptor.
Line 211: self._reactor.stop()
Line 212:
Line 213:
Line 214: class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
2:14 a.m.
Piotr Kliczewski has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
Line 206: self._handlers.append(detector)
Line 207:
Line 208: def stop(self):
Line 209: self.log.debug("Stopping Acceptor")
Line 210: self._acceptor.close()
No, this creates broken stop method that stop the reactor instead of
the ac
what is your definition of broken because it seems I have a different one.
Line 211: self._reactor.stop()
Line 212:
Line 213:
Line 214: class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
6:03 p.m.
Nir Soffer has posted comments on this change.
Change subject: protocoldetector: Fix random double close
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65187/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:
Line 206: self._handlers.append(detector)
Line 207:
Line 208: def stop(self):
Line 209: self.log.debug("Stopping Acceptor")
Line 210: self._acceptor.close()
what is your definition of broken because it seems I have a different
one.
Here is another failure:
22:59:10 ======================================================================
22:59:10 ERROR: test_detect_uppercase(False) (protocoldetectorTests.AcceptorTests)
22:59:10 ----------------------------------------------------------------------
22:59:10 Traceback (most recent call last):
22:59:10 File
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/tests/protocoldetectorTests.py",
line 113, in tearDown
22:59:10 self.acceptor.stop()
22:59:10 File
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/lib/vdsm/protocoldetector.py",
line 211, in stop
22:59:10 self._acceptor.close()
22:59:10 File "/usr/lib64/python2.7/asyncore.py", line 407, in close
22:59:10 self.del_channel()
22:59:10 File
"/home/jenkins/workspace/vdsm_master_check-patch-fc24-x86_64/vdsm/lib/yajsonrpc/betterAsyncore.py",
line 137, in del_channel
22:59:10 asyncore.dispatcher.del_channel(self, map)
22:59:10 File "/usr/lib64/python2.7/asyncore.py", line 292, in del_channel
22:59:10 del map[fd]
22:59:10 KeyError: 63
22:59:10 -------------------- >> begin captured logging <<
--------------------
22:59:10 2016-10-12 22:56:33,013 INFO (MainThread) [vds.MultiProtocolAcceptor]
Listening at 127.0.0.1:39101 (protocoldetector:178)
22:59:10 2016-10-12 22:56:33,014 DEBUG (MainThread) [vds.MultiProtocolAcceptor] Adding
detector <protocoldetectorTests.Echo object at 0x7f2c4a5262d0>
(protocoldetector:205)
22:59:10 2016-10-12 22:56:33,014 DEBUG (MainThread) [vds.MultiProtocolAcceptor] Adding
detector <protocoldetectorTests.Uppercase object at 0x7f2c4a526590>
(protocoldetector:205)
22:59:10 2016-10-12 22:56:33,016 INFO (Thread-182) [ProtocolDetector.AcceptorImpl]
Accepting connection from 127.0.0.1:33770 (protocoldetector:71)
22:59:10 2016-10-12 22:56:33,016 DEBUG (Thread-182) [ProtocolDetector.Detector] Using
required_size=9 (protocoldetector:84)
22:59:10 2016-10-12 22:56:33,017 INFO (Thread-182) [ProtocolDetector.Detector]
Detected protocol uppercase from 127.0.0.1:33770 (protocoldetector:120)
22:59:10 2016-10-12 22:56:33,068 DEBUG (MainThread) [vds.MultiProtocolAcceptor]
Stopping Acceptor (protocoldetector:209)
22:59:10 --------------------- >> end captured logging <<
---------------------
Piotr, we need to fix this code soon.
Line 211: self._reactor.stop()
Line 212:
Line 213:
Line 214: class _CannotDetectProtocol(Exception):
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
12:14 p.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: protocoldetector: Fix random double close
......................................................................
protocoldetector: Fix random double close
Previously we stopped the reactor before closing the acceptor. This
works most of the time, but once in a while, it fails with this error:
Traceback (most recent call last):
File "vdsm/tests/protocoldetectorTests.py", line 113, in tearDown
self.acceptor.stop()
File "vdsm/lib/vdsm/protocoldetector.py", line 211, in stop
self._acceptor.close()
File "/usr/lib64/python2.7/asyncore.py", line 407, in close
self.del_channel()
File "vdsm/lib/yajsonrpc/betterAsyncore.py", line 137, in del_channel
asyncore.dispatcher.del_channel(self, map)
File "/usr/lib64/python2.7/asyncore.py", line 292, in del_channel
del map[fd]
KeyError: 63
The flow of events leading to this error is:
1. Stooping reactor, signaling the reactor thread via eventfd
2. Context switch to the reactor thread
3. Reactor find it it was closed and close all dispatchers
4. Context switch to the original thread
5. Closing acceptor, fd is not in asyncore map
This patch fixes this issue by closing the acceptor before stopping the
reactor.
Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/protocoldetector.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Piotr Kliczewski: Looks good to me, approved
Nir Soffer: Verified
Jenkins CI: Passed CI tests
--
To view, visit https://gerrit.ovirt.org/65187
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0cec41c4baebcc620b70e19e62febed5dc9c542d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
2474
days inactive
2553
days old
vdsm-patches@lists.fedorahosted.org
9 comments
4 participants
participants (4)
-
automation＠ovirt.org -
Code Review -
Nir Soffer -
Piotr Kliczewski