Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revive KQueue Reactor #1918

Closed
twisted-trac opened this issue Jul 12, 2006 · 88 comments
Closed

Revive KQueue Reactor #1918

twisted-trac opened this issue Jul 12, 2006 · 88 comments

Comments

@twisted-trac
Copy link

dialtone's avatar dialtone reported
Trac ID trac#1918
Type enhancement
Created 2006-07-12 15:21:44Z
Branch https://github.com/twisted/twisted/tree/kqueue-1918

foom has been working a new version of pykqueue that works better than version 1.3 and is distributed and maintained by arg here: www.python-hpio.net.

Under this new circumstances reviving the kqueue reactor should be a good aim to work for.

Attachments:

  • kqueue.diff (15422 bytes) - added by dialtone on 2006-07-12 15:22:34Z - Working KQueue reactor patch
  • kqueue_1.1.diff (13239 bytes) - added by dialtone on 2006-07-13 16:48:40Z - New version
  • kqreactor_patch.diff (4070 bytes) - added by psykidellic on 2009-09-01 18:45:35Z - Patch for kqreactor using select26 module.
  • issue_1918.patch (6737 bytes) - added by psykidellic on 2010-04-05 21:20:58Z - kqueue using the Python select module.
  • twisted_errors.txt (45771 bytes) - added by psykidellic on 2010-04-05 21:33:13Z - Cases those failed.
  • new_kqueue.log (662618 bytes) - added by oberstet on 2011-11-04 15:46:21Z - new kqueue trial
  • select.log (659538 bytes) - added by oberstet on 2011-11-04 15:46:43Z - select trial (on same system)
  • select_vs_new_kqueue_diff.log (6450 bytes) - added by oberstet on 2011-11-04 15:48:18Z - diff select.log new_kqueue.log > select_vs_new_kqueue_diff.log
  • kqueue.patch (10648 bytes) - added by oberstet on 2011-11-07 17:15:16Z - select.kqueue based reactor
  • kqueue.log (723379 bytes) - added by oberstet on 2011-11-07 17:15:49Z - trial -r kqueue --rterrors twisted > kqueue.log
  • kqueue.2.patch (12370 bytes) - added by oberstet on 2011-11-09 14:00:46Z - Revised patch after review
  • kqueue.3.patch (14671 bytes) - added by oberstet on 2011-11-13 12:23:45Z - Revised patch : daemonization, correct news file
  • kqueue.4.patch (16895 bytes) - added by oberstet on 2011-11-15 12:30:36Z - Revised patch: docstring, IReactorDaemonize
  • kqueue.5.patch (21687 bytes) - added by oberstet on 2011-11-23 14:07:30Z - Revised patch: unit tests
  • kqueue.6.patch (21403 bytes) - added by oberstet on 2012-01-19 16:15:12Z - revised for review comments of exarkun
  • kqueue.7.patch (21411 bytes) - added by oberstet on 2012-01-19 16:39:53Z - issue 5.: use new style class
Searchable metadata
trac-id__1918 1918
type__enhancement enhancement
reporter__dialtone dialtone
priority__normal normal
milestone__ 
branch__branches_kqueue_1918 branches/kqueue-1918
branch_author__oberstet__exarkun__therve oberstet, exarkun, therve
status__closed closed
resolution__fixed fixed
component__core core
keywords__ 
time__1152717704000000 1152717704000000
changetime__1337450143000000 1337450143000000
version__None None
owner__oberstet oberstet
cc__therve cc__thijs cc__psykidellic cc__oberstet
@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

I don't like platform.usingKQueue. Also, I'd like to see all tests pass on FreeBSD, or else an explanation as to why they don't.

@twisted-trac
Copy link
Author

dialtone's avatar dialtone commented

Removed usingKQueue.

This patch was tested against MacOSX 10.4.7 and FreeBSD 6.1-stable.

stdio and PTY tests unfortunately still fail and need more testing. Probably _doReadOrWrite method needs some extra corner cases to deal with stdio and processes errors since kqueue/kevent deal with those with small but decisive differences.

@twisted-trac
Copy link
Author

glyph's avatar @glyph set owner to dialtone

@twisted-trac
Copy link
Author

therve's avatar @therve commented

python-hpio.net is down for a while now, can you provide the latest tarball? The best would probably to ship it with Twisted, if license is compatible.

@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

The latest source is here:
http://twistedmatrix.com/trac/browser/sandbox/foom/pykqueue

As far as I know, the thing on python-hpio was unchanged from this.

License is BSD.

@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

Also, there is no way that kqueue will work with PTYs (or any "device" kind of fd) on osx < 10.5 (which hasn't been released yet). This is a limitation in the OS. Poll doesn't work for device fds either, btw.

Just to calibrate expectations here...

@twisted-trac
Copy link
Author

dreid's avatar @dreid commented

This patch no longer applies cleanly to trunk.

@twisted-trac
Copy link
Author

therve's avatar @therve commented

Note that the libevent reactor (at #1930) is supposed to support kqueue, but I didn't have the opportunity to test.

@twisted-trac
Copy link
Author

therve's avatar @therve commented

I have started some things in the kqreactor-1918 branch.

@twisted-trac
Copy link
Author

therve's avatar @therve commented

(In [23105]) Branching to 'kqreactor-1918-2'

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

While trying to port dialtone's inotify to Mac, seems I need to use kqueue(). The kqreactor in Twisted uses kqsyscall. The Python module for it seems only to be maintained with Python 1.5. Also, it does not compile on my Leapord even with the patch provided by Itamar at http://twistedmatrix.com/documents/8.2.0/api/twisted.internet.kqreactor.html.

Meanwhile, Python 2.6 is bringing kqueue() support in its select module. A backport of it for 2.5 is available at: http://pypi.python.org/pypi/select26. Using the module and patching up kqreactor.py with the attached diff, I was able to run:

kqreactor.install().

I would really appreciate if somebody can look into the patch and let me know if this is the right way to proceed.

I need folderwatcher support for Mac at my work so I am willing to spend time to get this thing working so any thoughts, comments are welcome.

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

One thing to note.

Following the example at:
http://julipedia.blogspot.com/2004/10/example-of-kqueue.html (which
works perfectly as it tells all events), I tried to port it to Python:

http://www.bpaste.net/show/25/

Not sure where I am wrong but the poller only returns one event
ever. Adding new text, deleting the file does not return any event.

Looks like it might be related to the bug at: http://bugs.python.org/issue5910 but I tried to implement that patch in the current select26 without success.

I will try to contact the original author cheimes to see if he can help me out.

@twisted-trac
Copy link
Author

glyph's avatar @glyph set owner to @glyph
@glyph set status to assigned

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

#3114 also talks about kqueue.

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Hi

Here is my first attempt to use the Python 2.6 select module and Christian Heimes backport to update the kqueue reactor.

Initially, I had some issues with test cases related to timerservice but after exarkun pointed out at IRC, the timeout should be in seconds rather than milliseconds.

Though I am still seeing errors when I run the testcases and they primarily seemed to be grouped in processes or pty/tty which seems to follow dialtone and jknight observations.

I am attaching a patch file which is applicable to trunk at revision: r28802.

Let me know what else I can do?

@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

Are you testing on OSX? What version?

I guess all the tests which use devices should be marked as known failures on OSX/kqueuereactor, since apple seems in no hurry to fix it...

Is the patch for select26 in your patch only required for the select26 backport? That is: does the reactor work properly against released python 2.6's select module?

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Wow. That was quick.

Yes. I am testing on Mac OS X v10.5 now. I have a friend who has 10.6 so I can ask him to test it on Snow Leapord also. I guess, I can install FreeBSD on my virtual box and run the test cases.

As an addition, I am also attaching the error log of the complete test run.

The following tests seem to be taking longer (when using kqueue rather than the default select) but eventually they succeeded.

test_outputWithErrorIgnored ...
HalfCloseTestCase
testCloseWriteCloser ...

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Replying to jknight:

Is the patch for select26 in your patch only required for the select26 backport? That is: does the reactor work properly against released python 2.6's select module?

This works on both. It first tries to import select26 module and if it fails tries to import the select module.

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Ooops, my bad. I should not have removed the review keyword.

@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

"This works on both" -- well, it doesn't work on an unpatched select26 -- it requires a patched copy to work properly. So I was wondering if it worked on an out-of-the-box python 2.6. Anyways, I looked at the source for selectmodule.c in python 2.6: it looks like only Python 2.6.5 or later will work, because that's the first version to include the fix for:
#5910: fix kqueue for calls with more than one event.

On earlier versions of Python 2.6, I suppose users will need to install the patched select26 module also. kqueuereactor should probably check that the version of python it's running against is acceptable before letting you use it.

As for the failing tests, the test_process and test_stdio failures are just because the kqueue syscall in OSX is broken for ptys: those failures are expected. But do retest on FreeBSD to make sure it works there.

I don't think the half-close tests in test_tcp should be failing, however. That /probably/ indicates an actual bug in kqueuereactor.

Also, as a note on the process here: if you think there are still things you need to work on, removing the review keyword yourself is an entirely appropriate thing to do.

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun set owner to psykidellic
@exarkun set status to new

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

My bad about the interpretation of your question. Yes, you need to apply the patch for select26 module.

Is there any standard way to check versions when you code in Twisted? I can put those checks.

I guess the next step is to look into the non test_process/test_stdio test cases to make sure there are no errors in kqreactor.

I will also try to install FreeBSD on my VirtualBox to run the testcases.

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Alright. I am kind of stuck now. An extra eye would be much appreciated.

http://www.bpaste.net/show/5236/ - I have pasted the output of trial run with -e and the resulting test.log.

Basically, in the testcases: testCloseWriteCloser and testWriteCloseNotification, the method readConnectionLost method is never called.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to psykidellic:

http://www.bpaste.net/show/5236/ - I have pasted the output of trial run with -e and the resulting test.log.

In the future, could you attach logs like this to tickets as attachments? Pastebins tend to expire things pretty quickly, so when someone goes to diagnose it, it's OK.

Basically, in the testcases: testCloseWriteCloser and testWriteCloseNotification, the method readConnectionLost method is never called.

You mentioned on IRC that this was run under Leopard. Any chance you could test it on a more recent MacOS, or a recent FreeBSD, and see if the results differ? Thanks!

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

You mentioned on IRC that this was run under Leopard. Any chance you could test it on a more recent MacOS, or a recent FreeBSD, and see if the results differ? Thanks!

We have a Snow Leopard slave and a FreeBSD slave. Someone should put the code in a branch and run it on our buildbot.

@twisted-trac
Copy link
Author

jyknight's avatar @jyknight commented

So I tried select.kqueue on python on my mac running OSX 10.6.2. It still doesn't support PTYs (or /dev/null), and furthermore, it crashed my mac with a kernel panic when I exited the python process.

I had run something like this at an interactive python prompt:

import select, os
kq = select.kqueue()
devnull = os.open("/dev/null", O_WRONLY)
kq.control([select.kevent(devnull, select.KQ_FILTER_WRITE, select.KQ_EV_ADD)], 1, 0)

and it returned a KQ_EV_ERROR with EINVAL error code, as it always has...and kernel panic'd on control-D. I didn't just run the above, though, it was some interactive fiddling, so I don't know exactly what set of things caused the panic. I haven't tried (and am not going to try) to reproduce it.

It's nearly certainly kqueue's fault, though.

Anyhow, given that behavior, it doesn't seem like we should try to support kqueue reactor on OSX (nor even attempt to run it on the buildslave). It's clear Apple doesn't really test kqueue at all, nor care if it works (it's been broken since it was added, in 10.3 or so). They're probably too busy making sure that people can't run software on the iPhone to get around to fixing kernel panics in OSX.

And BTW, poll on OSX is broken just as badly as kqueue...but apparently python on OSX is built without poll support, so you can't even try that. :)

Reviving kqueuereactor for freebsd to use is still a good idea, though.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to jknight:

So I tried select.kqueue on python on my mac running OSX 10.6.2. It still doesn't support PTYs (or /dev/null), and furthermore, it crashed my mac with a kernel panic when I exited the python process.

I had run something like this at an interactive python prompt:

Unfortunately I can't reproduce this particular panic :-. Maybe this would be helpful, if you experience it again? How to log a kernel panic.

and it returned a KQ_EV_ERROR with EINVAL error code, as it always has...and kernel panic'd on control-D. I didn't just run the above, though, it was some interactive fiddling, so I don't know exactly what set of things caused the panic. I haven't tried (and am not going to try) to reproduce it.

It's nearly certainly kqueue's fault, though.

Anyhow, given that behavior, it doesn't seem like we should try to support kqueue reactor on OSX (nor even attempt to run it on the buildslave). It's clear Apple doesn't really test kqueue at all, nor care if it works (it's been broken since it was added, in 10.3 or so). They're probably too busy making sure that people can't run software on the iPhone to get around to fixing kernel panics in OSX.

If you can reproduce this kernel panic, report it at bugreport.apple.com. I assure you someone will be interested :).

Calendar Server uses kqueue, albeit not with /dev/null or PTYs, so I'm personally motivated to help out with this where I can.

@twisted-trac
Copy link
Author

psykidellic's avatar psykidellic commented

Wow. So many replies. I thought I would be notified if I am in the CC list. Anyway, we had a mid-release coming up so I had to divert resources to office work and could not reply here.

Status update:

-- I was able to sneak in couple of hours over the weekdays to run do couple of testruns. Due to my oversight I completely missed that branch 1918-2 already had the patch from dialtone. After correction from exarkun, I ran the test again using Python 2.5 and select26 backport on my laptop. Then I went ahead and ported the changes to the trunk but unfortunately some extra test cases (related to process fails) even though the same test case dont fail in 1918-2 branch.

I am not sure what might be the reason. If somebody with more knowledge can help me out, it would be much appreciated. I see the same comment from dreid so I am not sure what changes broke those tests.

As for FreeBSD, I had VirtualBox and after some fiddling around and Googling, it seems VirtualBox does not really support FreeBSD out of the box. Thankfully though I was able to get one of the extra licenses for VMWare Fusion from office. According to the docs: http://www.freebsd.org/doc/en/books/handbook/virtualization-guest.html, VMWare Fusion should support FreeBSD. I am installing it as I write this message.

Regarding the kernel panic, even I was unable to reproduce the error. I had a kernel panic only once when I was trying to run the test suite over SSH but that probably was bad mixup of code/branches so I am not even sure of the steps to reproduce the error.

I will upload my test report soon as they are on my laptop.

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

(In [29762]) Branching to 'kqreactor-1918-3'

@twisted-trac
Copy link
Author

zectbumo's avatar zectbumo commented

I have confirmed that the unit tests fail on the PTY test for both OSX 10.6.4 (Python 2.6) and FreeBSD 6.1 (Python 2.7).

BTW, I had to change a line in reactormixins.py from "kqueuereactor" to "kqreactor" so I could use the command trial -r kqueue

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented
> > 2. twisted/test/test_twistd.py
> >   1. FakeNonDaemonizingReactor should be new-style 
> I don't understand

FakeNonDaemonizingReactor just needs to subclass object (as all newly introduced classes in Twisted should, so we stop accumulating new "classic" classes).

How do I find all PTY related tests?

One trick for this might be to just make spawnProcess raise a bogus exception when someone tries to use a pty on os x. This should make all such tests fail quickly, and then we can decide how to handle the resulting list of tests (hopefully they'll appear in a few clusters so they don't all need to be handled individually).

In any case: why is a Mac builder for Twisted kqueue needed?

OS X is the only BSD-like platform we have a slave for. We have an offer of some hosting resources for a FreeBSD slave, but someone with FreeBSD experience needs to set it up and perhaps offer a minimal level of maintenance (making sure it stays running, fixing any platform-specific issues that might arise that interfere with the slave itself (although helping out with platform-specific issues in the test suite is also very helpful :), etc). Am I right in guessing that if you're not too interested in OS X, you are interested in FreeBSD instead?

An alternative to figuring out all the OS X/PTY test failures would be to only set up the FreeBSD slave and only claim kqueue is supported on FreeBSD. Someone else with more interest in OS X can deal with getting a clean test suite run on OS X later.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented
  1. : ah, ok. now I see. done: kqueue.7.patch

  2. : yes, you are right;) I don't care about OSX, only FBSD. Instead of investing time into osx, I'd rather provide resources for a FreeBSD slave. Can be time or hosting. Could do the latter also. Is there any document that describes whats needed for a slave and how it works? How do I get started?

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to oberstet:

  1. : ah, ok. now I see. done: kqueue.7.patch

  2. : yes, you are right;) I don't care about OSX, only FBSD. Instead of investing time into osx, I'd rather provide resources for a FreeBSD slave. Can be time or hosting. Could do the latter also. Is there any document that describes whats needed for a slave and how it works? How do I get started?

I care about OS X, but I don't care a whole lot about OSX+kqueue+spawnProcess; there are enough other ways to get code which requires some subset of those features to work. Ideally we could just add a slave to our existing builder but have some conditional test skips for PTY-based tests.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

(oops. I meant OSX+kqueue+spawnProcess+PTYs. Actually i do care about OSX+kqueue+spawnProcess ;-))

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Replying to glyph:

Replying to oberstet:

  1. : ah, ok. now I see. done: kqueue.7.patch

  2. : yes, you are right;) I don't care about OSX, only FBSD. Instead of investing time into osx, I'd rather provide resources for a FreeBSD slave. Can be time or hosting. Could do the latter also. Is there any document that describes whats needed for a slave and how it works? How do I get started?

I care about OS X, but I don't care a whole lot about OSX+kqueue+spawnProcess; there are enough other ways to get code which requires some subset of those features to work. Ideally we could just add a slave to our existing builder but have some conditional test skips for PTY-based tests.

Do I get this right? :

  1. currently there is neither a OS X nor a FreeBSD slave
  2. ideally there should be both of those
  3. the OS X slave requires skipping PTY tests for kqueue. The detection of unsupported PTY could be done by simply detecting the OS X platform, or - IMHO better - using feature detection (see my comment # 64)

As said, I would help with the FreeBSD slave.

''
Is 3. then still required to have the patch accepted, or could that be done in another ticket (and by someone actually interested in the OS X platform ;) ??''

I don't wanna sound silly/frustrated, but my motivation of doing 3. approaches zero ...

P.S.: any docs for builder slaves?

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

Replying to oberstet:

Replying to glyph:

I care about OS X, but I don't care a whole lot about OSX+kqueue+spawnProcess; there are enough other ways to get code which requires some subset of those features to work. Ideally we could just add a slave to our existing builder but have some conditional test skips for PTY-based tests.

Do I get this right? :

  1. currently there is neither a OS X nor a FreeBSD slave

There is an OS X slave, but it isn't testing kqueue reactor.

  1. ideally there should be both of those

Since OS X mostly supports kqueue, yes - ideally, at some point, a kqueue-based reactor would be available on both OS X and FreeBSD (who knows, perhaps even on some other BSDs as well :).

  1. the OS X slave requires skipping PTY tests for kqueue. The detection of unsupported PTY could be done by simply detecting the OS X platform, or - IMHO better - using feature detection (see my comment # 64)

Yes. As long as tests are failing (consistently), we don't consider a platform supported. Relatedly, we don't introduce changes that make tests fail (consistently) on a supported platform. We do testing on unsupported platforms, but they get less attention.

As said, I would help with the FreeBSD slave.

''
Is 3. then still required to have the patch accepted, or could that be done in another ticket (and by someone actually interested in the OS X platform ;) ??''

OS X issues can absolutely be addressed in another ticket. Just adding kqueue support for FreeBSD is a major feature addition, and is worthwhile independent of any OS X issues.

I don't wanna sound silly/frustrated, but my motivation of doing 3. approaches zero ...

I understand. Thanks very much for sticking with it for this long. :)

P.S.: any docs for builder slaves?

Yep - http://twistedmatrix.com/trac/wiki/ContinuousIntegration

I'm trying to dig up credentials for an existing FreeBSD machine we should have access to. If I succeed, and you're interested in setting up/maintaining a slave on it, I can email you the details.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Replying to exarkun:

P.S.: any docs for builder slaves?

Yep - http://twistedmatrix.com/trac/wiki/ContinuousIntegration

Ah, ok. BuildBot. Used that before ..

I'm trying to dig up credentials for an existing FreeBSD machine we should have access to. If I succeed, and you're interested in setting up/maintaining a slave on it, I can email you the details.

Thanks! Either that, or I can host a virtualized one on machine in a datacenter.

So, more important for me: can you give me (tobias.oberstein@...) a slave name/password and configure that on the build master?

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

ok, full email: tobias.oberstein at tavendo.de

another thing: I would setup a slave running FreeBSD 8.2 i386 .. so slave name could be something like freebsd82_i386 ..

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to exarkun:

Replying to oberstet:

Replying to glyph:

Is 3. then still required to have the patch accepted, or could that be done in another ticket (and by someone actually interested in the OS X platform ;) ??''

OS X issues can absolutely be addressed in another ticket. Just adding kqueue support for FreeBSD is a major feature addition, and is worthwhile independent of any OS X issues.

Just to reinforce this, if no tests fail on the existing OS X builder I'm perfectly happy for that work to happen on a different ticket. I'm sorry if my earlier comment gave the impression otherwise.

Also, I'd be happy to take this on, so if you wouldn't mind describing things in detail on that ticket (maybe copy some of comment 63, to remind me) and assign it to me, please do so.

I don't wanna sound silly/frustrated, but my motivation of doing 3. approaches zero ...

I understand. Thanks very much for sticking with it for this long. :)

Ditto. This has been a significant chunk of work, and a feature I'm personally happy to see land, so your perseverance is noted and very much appreciated.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Replying to glyph:

Replying to exarkun:

Replying to oberstet:

Replying to glyph:

Is 3. then still required to have the patch accepted, or could that be done in another ticket (and by someone actually interested in the OS X platform ;) ??''

OS X issues can absolutely be addressed in another ticket. Just adding kqueue support for FreeBSD is a major feature addition, and is worthwhile independent of any OS X issues.

Just to reinforce this, if no tests fail on the existing OS X builder I'm perfectly happy for that work to happen on a different ticket. I'm sorry if my earlier comment gave the impression otherwise.

I am trying to verify if "no tests fail on the existing OS X builder".

When I do the following on OS X (using trunk with the patch applied)

trial -r select --rterrors twisted

I get 11 errors in total. Here is the breakdown:

E1:
1 failed test case:

twisted.trial.test.test_loader

=> Tries to create a directory under the Twisted installation directory .. permission denied.

This is unrelated to the patch.

E2:
2 failed test cases:

twisted.test.test_enterprise
twisted.test.test_reflector

=> "from twisted.enterprise.adbapi import _safe" fails .. "cannot import name _safe"

This is unrelated to the patch.

E3:

1 failed test case:

TCPClientTestsBuilder_KQueueReactor
test_protocolGarbageAfterLostConnection

 "Can't stop a reactor that isn't running"

E4:

7 failed test cases:
PTYProcessTestsBuilder_KQueueReactor
test_openFileDescriptors
...

So as far as I can see only E3 and E4 are kqueue related.

However, I am wondering why those cases are even run for kqueue although I started trial with option "-r select".

How does a BuildBot slave run the trial? Similar to how I did it manually?

Anyway, would you agree that fixing E3 and E4 (that is make those tests be skipped on OS X) should make the patch finally acceptable?

PS: I'll create a new ticket ("Skip PTY tests on OS X when running kqueue") when this patch is done and it's clear whats being left to do for OS X.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

For completeness, here is the breakdown for FreeBSD when running the same

trial -r select --rterrors twisted

E1 and E2 are the same.

Further, the following cases fail/err:

twisted.internet.test.test_posixprocess.FileDescriptorTests.test_expectedFDs
twisted.internet.test.test_process.PTYProcessTestsBuilder_KQueueReactor.test_openFileDescriptors
twisted.internet.test.test_process.PTYProcessTestsBuilder_PollReactor.test_openFileDescriptors
twisted.internet.test.test_process.PTYProcessTestsBuilder_SelectReactor.test_openFileDescriptors
twisted.internet.test.test_process.ProcessTestsBuilder_KQueueReactor.test_openFileDescriptors
twisted.internet.test.test_process.ProcessTestsBuilder_PollReactor.test_openFileDescriptors
twisted.internet.test.test_process.ProcessTestsBuilder_SelectReactor.test_openFileDescriptors

This is due to platform restrictions (related to listing the open FDs of a process), but unrelated to the patch.

To make FreeBSD a "supported" platform, I'd create a separate ticket for those and fix them.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to oberstet:

This is due to platform restrictions (related to listing the open FDs of a process), but unrelated to the patch.

To make FreeBSD a "supported" platform, I'd create a separate ticket for those and fix them.

This looks suspiciously similar to #4881. Are you using current trunk?

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to oberstet:

Replying to glyph:

Replying to exarkun:

Replying to oberstet:

Replying to glyph:

Is 3. then still required to have the patch accepted, or could that be done in another ticket (and by someone actually interested in the OS X platform ;) ??''

OS X issues can absolutely be addressed in another ticket. Just adding kqueue support for FreeBSD is a major feature addition, and is worthwhile independent of any OS X issues.

Just to reinforce this, if no tests fail on the existing OS X builder I'm perfectly happy for that work to happen on a different ticket. I'm sorry if my earlier comment gave the impression otherwise.

I am trying to verify if "no tests fail on the existing OS X builder".

When I do the following on OS X (using trunk with the patch applied)

E2:
2 failed test cases:

twisted.test.test_enterprise
twisted.test.test_reflector

=> "from twisted.enterprise.adbapi import _safe" fails .. "cannot import name _safe"

This is unrelated to the patch.

There's no such module in current trunk: http://twistedmatrix.com/trac/browser/trunk/twisted/test/test_enterprise.py

Did you fail to clean up PYCs maybe?

E3:

1 failed test case:

TCPClientTestsBuilder_KQueueReactor
test_protocolGarbageAfterLostConnection

 "Can't stop a reactor that isn't running"

This looks suspicious, like it might actually be related to the patch. If this is going to be run by the existing buildbot it might need fixing.

So as far as I can see only E3 and E4 are kqueue related.

However, I am wondering why those cases are even run for kqueue although I started trial with option "-r select".

These are the reactor-builder tests. They test all the reactors using tests which build the reactor, then run very specific tests against that created reactor, then clean it up. What -r select says to trial is to use the select reactor as the global reactor, i.e. the one that gets spun up if you return a Deferred from your test case.

Generally, the reactor builder tests are a lot better, as they demonstrate more isolated failures, and we are moving to test all reactors in this way as much as possible.

How does a BuildBot slave run the trial? Similar to how I did it manually?

Pretty close, but if you want to know exactly, here's the builder: http://buildbot.twistedmatrix.com/builders/osx10.6-py2.6-select

Anyway, would you agree that fixing E3 and E4 (that is make those tests be skipped on OS X) should make the patch finally acceptable?

As long as there's an accompanying ticket to fix it later :). E3 looks like a potentially real problem, but given that this is new functionality, skipping it on OS X for now and fixing it separately is OK.

PS: I'll create a new ticket ("Skip PTY tests on OS X when running kqueue") when this patch is done and it's clear whats being left to do for OS X.

Sounds good, although any failing reactor builder tests need to be skipped before landing in order to not break the existing builder, as noted above.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Replying to glyph:

Replying to oberstet:

This is due to platform restrictions (related to listing the open FDs of a process), but unrelated to the patch.

To make FreeBSD a "supported" platform, I'd create a separate ticket for those and fix them.

This looks suspiciously similar to [#4881](#4881). Are you using current trunk?

Yep, I did use trunk.

And yes, the ticket is exactly about this issue.

In the meantime (with the help of exarkun), I've setup a FreeBSD build slave.

After mounting

mount -t fdescfs null /dev/fd

those cases also succeed:

http://buildbot.twistedmatrix.com/builders/freebsd-8.2-i386/builds/3

Is #4881 already merged on trunk? If yes, then the skipping of the open FD tests when "fdescfs" is NOT mounted doesn't seem to work.

But I will automount the fdescfs on the slave anyway .. now that I know how to.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to oberstet:

Replying to glyph:

Replying to oberstet:

This is due to platform restrictions (related to listing the open FDs of a process), but unrelated to the patch.

To make FreeBSD a "supported" platform, I'd create a separate ticket for those and fix them.

This looks suspiciously similar to [[#4881](https://github.com//issues/4881)](#4881). Are you using current trunk?

Yep, I did use trunk.

Make sure to update, clean up PYCs, etc, to deal with any local configuration issues.

And yes, the ticket is exactly about this issue.

In the meantime (with the help of exarkun), I've setup a FreeBSD build slave.

After mounting

mount -t fdescfs null /dev/fd

those cases also succeed:

http://buildbot.twistedmatrix.com/builders/freebsd-8.2-i386/builds/3

OK, great.

Is #4881 already merged on trunk? If yes, then the skipping of the open FD tests when "fdescfs" is NOT mounted doesn't seem to work.

It was merged to trunk here: http://twistedmatrix.com/trac/ticket/4881#comment:24

It sounds like you're saying the fix didn't actually work. When fdescfs is mounted, even earlier versions of Twisted would pass tests on FreeBSD.

But I will automount the fdescfs on the slave anyway .. now that I know how to.

That does seem like a reasonable solution as far as this ticket is concerned, but possibly you should re-open #4881 for further investigation.

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

That does seem like a reasonable solution as far as this ticket is concerned, but possibly you should re-open #4881 for further investigation.

Or file a new ticket. I think only reverts should re-open tickets.

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

(In [33458]) Create feature branch for #1918 ("revive kqueue reactor").

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Ok, the new branch "branches/kqueue-1918" now contains the above "kqueue.7.patch".

I'd like to discuss the results of running trial against the branch on OS X and FreeBSD. Need some help;)

OS X:

http://buildbot.twistedmatrix.com/builders/osx10.6-py2.6-select/builds/1791

It has 8 errors. 7 of those are PTY issues. As already discussed, those need to be skipped on OS X. I will open a new ticket for that when the branch is merged.

The 1 other error is in:

twisted.internet.test.test_tcp.TCPClientTestsBuilder_CFReactor.test_protocolGarbageAfterLostConnection

I suspect CF = Core Foundation? How is that related to kqueue .. what is happening. I'm a bit puzzled by that one.

FreeBSD:

http://buildbot.twistedmatrix.com/builders/freebsd-8.2-i386/builds/5

It has 8 erros and 1 failure.

Those are issues I have not seen in my testing before, I guess because the buildslave I've setup has everything (but GTK) installed, and thus skips less than my manual testing before.

All 9 issue are in

twisted.test.test_stdio.StandardInputOutputTestCase

and as far as I could analyze, they are related to handling of "temp" files produced during the test cases.

I have no clue why those pop up when running kqueue, but not when running select.

Any hints on that one would be great also .. whats going on?

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun set owner to @oberstet

As already discussed, those need to be skipped on OS X.

A simple way to do this for now might be to remove kqueue reactor from the ReactorBuilder._reactors list on OS X. There's already some platform-specific inspection code there, should it should be an easy change.

The 1 other error is in:

twisted.internet.test.test_tcp.TCPClientTestsBuilder_CFReactor.test_protocolGarbageAfterLostConnection

I suspect CF = Core Foundation? How is that related to kqueue .. what is happening. I'm a bit puzzled by that one.

CF is Core Foundation, yes. I'm not sure what's going on here. Possibly state from one of the failing KQueue reactor tests is interfering. I would try to get rid of the rest of the failures and see if this one remains. If this one stops failing, it would then also be interesting to investigate how the KQueue failures were causing this one to fail (but that doesn't need to block this ticket).

All 9 issue are in

twisted.test.test_stdio.StandardInputOutputTestCase

These may be environment setup issues, caused by a bug in the unit tests, resulting in the wrong version of Twisted being used in the child process. I would investigate what PYTHONPATH ends up being set to in the launched process and see if that leads anywhere. I suspect this is the problem because:

  1. The PYTHONPATH initialization code in StandardInputOutputTestCase._spawnProcess looks complicated and perhaps even buggy (constructing "/foo/bar:" as a value sometimes).
  2. One of the tests fails with output including "ImportError: No module named kqsyscall"

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

Replying to exarkun:

All 9 issue are in

twisted.test.test_stdio.StandardInputOutputTestCase

These may be environment setup issues, caused by a bug in the unit tests, resulting in the wrong version of Twisted being used in the child process. I would investigate what PYTHONPATH ends up being set to in the launched process and see if that leads anywhere. I suspect this is the problem because:

  1. The PYTHONPATH initialization code in StandardInputOutputTestCase._spawnProcess looks complicated and perhaps even buggy (constructing "/foo/bar:" as a value sometimes).
  2. One of the tests fails with output including "ImportError: No module named kqsyscall"

Thanks! You were right .. problem is module search path is not correct.

The PYTHONPATH of the spawned process does point to the locally checked out code tree (and thus, to the new kqueue), but the PYTHONPATH ends up behind the one pointing to the system installed Twisted within sys.path.

I can get the cases working by doing something like this:

[oberstet@txbuild ~/Twisted-1918]$ svn diff
Index: twisted/test/stdio_test_consumer.py
===================================================================
--- twisted/test/stdio_test_consumer.py (revision 33468)
+++ twisted/test/stdio_test_consumer.py (working copy)
@@ -8,7 +8,8 @@
 that process transports implement IConsumer properly.
 """

-import sys
+import sys, os
+sys.path = os.environ.get("PYTHONPATH", "").split(":") + sys.path

 from twisted.python import log, reflect
 from twisted.internet import stdio, protocol
[oberstet@txbuild ~/Twisted-1918]$

Is is acceptable to do that for all 9 files? Or do you prefer something different?

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet commented

As discussed on IRC with exarkun, I did:

svn cp bin/_preamble.py twisted/test

and added

import _preamble

to the twisted/test/stdio_test_* files.

This solves the problem. FreeBSD now runs the whole trial without any error/unexpected failure:

http://buildbot.twistedmatrix.com/builders/freebsd-8.2-i386/builds/8

I want to note that at least 2 more files contain roughly similar code like _preamble.py:

./twisted/test/process_twisted.py

./twisted/conch/test/test_conch.py

a) The code is not the same, and it is inferior (i.e. one will only work when the local Twisted base directory is actually called "Twisted").

b) The code is replicated.

Thus, I think we should clean that up also. But probably not in this ticket. This ticket is already touching a couple of places. Agreed?

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet set owner to @exarkun

Ok, the OS X platform also succeeds, after doing like suggested by exarkun in #7401.

I have run the full set of builders using force-builds.py:

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/kqueue-1918

Is there anything more to do for final merge?

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

(In [33478]) Cosmetic fixes

refs #1918

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun set owner to @oberstet

I made a few further changes, very minor; one is linked above, the others were r33479 and r33480.

Latest build results look fairly good (I think that's as good as Windows gets at the moment).

This looks good to me to merge now. Since this is your first merge, maybe it would be good if you found someone on IRC to walk you through it, or at least confirm your ideas of how it works if you have some already. :)

Thanks very much for your work on this!

@twisted-trac
Copy link
Author

zectbumo's avatar zectbumo commented

Replying to zectbumo:
It says I was removed from the CC but I still get emails. Will someone please remove me from the CC?

@twisted-trac
Copy link
Author

oberstet's avatar @oberstet set status to closed

(In [33481]) Merge kqueue-1918: Revive kqueue reactor

Author: oberstet
Reviewer: exarkun, glyph
Fixes: #1918

A kqueue()/kevent() based implementation of the Twisted main loop.

This implementation depends on Python 2.6 or higher which has kqueue support
built in the select module.

Note, that you should use Python 2.6.5 or higher, since previous implementations
of select.kqueue had U{http://bugs.python.org/issue5910} not yet fixed.

Note, that on OS X, PTYs will (currently) NOT work due to platform restrictions.

@twisted-trac
Copy link
Author

building team's avatar building team commented

I cant get this to work with Python 2.6.5

I'm using Linux 64bit, Python 2.6.5.

The following script, causes my CPUs to go 100% after 10 seconds.

If I use gtk.main() instead of reactor.run(), there will be no problem, so I wonder if it is caused by the new version of pykqueue or a bug of twisted?

#!/usr/bin/python
#import gtk
from twisted.internet import gtk2reactor
gtk2reactor.install()
from twisted.internet import reactor
import subprocess
subprocess.Popen(['sleep', '10'])
#gtk.main()
reactor.run()

/Århus


(In [33481]) Merge kqueue-1918: Revive kqueue reactor

Author: oberstet Reviewer: exarkun, glyph Fixes: #1918

A kqueue()/kevent() based implementation of the Twisted main loop.

This implementation depends on Python 2.6 or higher which has kqueue support built in the select module.

Note, that you should use Python 2.6.5 or higher, since previous implementations of select.kqueue had U{ http://bugs.python.org/issue5910} not yet fixed.

Note, that on OS X, PTYs will (currently) NOT work due to platform restrictions.


Replying to glyph:

Replying to oberstet:

Replying to glyph:

Replying to oberstet:

This is due to platform restrictions (related to listing the open FDs of a process), but unrelated to the patch.

To make FreeBSD a "supported" platform, I'd create a separate ticket for those and fix them.

This looks suspiciously similar to [[#4881](https://github.com//issues/4881)](#4881). Are you using current trunk?

Yep, I did use trunk.

Make sure to update, clean up PYCs, etc, to deal with any local configuration issues.

And yes, the ticket is exactly about this issue.

In the meantime (with the help of exarkun), I've setup a FreeBSD build slave.

After mounting

mount -t fdescfs null /dev/fd

those cases also succeed:

http://buildbot.twistedmatrix.com/builders/freebsd-8.2-i386/builds/3 Teambuilding århus

OK, great.

Is #4881 already merged on trunk? If yes, then the skipping of the open FD tests when "fdescfs" is NOT mounted doesn't seem to work.

It was merged to trunk here: http://twistedmatrix.com/trac/ticket/4881#comment:24 Teambuilding

It sounds like you're saying the fix didn't actually work. When fdescfs is mounted, even earlier versions of Twisted would pass tests on FreeBSD.

But I will automount the fdescfs on the slave anyway .. now that I know how to.

That does seem like a reasonable solution as far as this ticket is concerned, but possibly you should re-open #4881 for further investigation.

@twisted-trac
Copy link
Author

exarkun's avatar @exarkun commented

I cant get this to work with Python 2.6.5

Please use the mailing list or the IRC channel for support.

@twisted-trac
Copy link
Author

glyph's avatar @glyph commented

Replying to building team:

I'm using Linux 64bit, (...) I wonder if it is caused by the new version of pykqueue or a bug of twisted?

For posterity, I just want to make it clear that this question doesn't really make sense, since kqueue is a feature of BSD-family OSes (FreeBSD, NetBSD, Mac OS X) and can't be used on Linux. Also, the kqueue reactor wouldn't be used in conjunction with the GTK reactor; they're different event-loop APIs.

As exarkun already suggested, if you don't know what your issue actually relates to, ask the question on the mailing list or IRC first and we'll attempt to steer you to the correct bug (or a new bug).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant