[Twisted-Python] a possible solution for ticket 5562

Hi All A few months ago, I reported a bug about IOCP. Last night I spent several hours on its implementation and finally found out a possible solution for that. when sending some small chunks data continuously, the buffer will pile them up and send to IOCP; however there is a SEND_LIMIT (128K) that means only 128K will be handled. Now the problem is when we try to trigger the next writing, IOCP will raise ERROR_IO_PENDING immediately and then connection Lost. So I got a idea: if the size of data is larger than SEND_LIMIT, we can wait a little bit time for the next writing instead of do it immediately. in twisted\internet\iocpreactor\abstract.py there is a method def _cbWrite(self, rc, bytes, evt): if self._handleWrite(rc, bytes, evt): self.doWrite() now I modified a bit, def _cbWrite(self, rc, bytes, evt): if self._handleWrite(rc, bytes, evt): if len(evt.buff) < self.SEND_LIMIT: self.doWrite() else: self.reactor.callLater(0.8,self.doWrite) 0.8 is a silly trial but I have no idea what is the right number for this place. After this modification, previous problematic scripts can pass. Maybe the better solution is to find a way to poll the complete port status when read/write will be recovered from IO PENDING. Simply wait a little is risky. Regards gelin yan

This looks like the kind of thing that could involve using Deferred as part of solution. Instead of callLater(0.8,doWrite), design the mechanism to wire up event-source to fire the deferred and make doWrite be the callback. On Oct 20, 2012, at 8:29:10AM, gelin yan wrote:

I'm top-posting just for consistency in this thread, but in the future please comment inline with trimming :). It's easier to follow for future readers; if they find a post stand-alone in some web archive, the question will come before the answer. Anyhow; Mike, you're definitely closer to the answer; a callLater is unnecessary, the callback should be called on demand. But, given that a queue of write operations is just that - a queue - we don't need a Deferred for every write; the callback to the write operation can just pick up the next queued item. -g On Oct 20, 2012, at 9:07 AM, Mike Winter <miwinter@cisco.com> wrote:

On Sun, Oct 21, 2012 at 3:35 AM, Glyph <glyph@twistedmatrix.com> wrote:
Hi All Thanks for reply. According to MSDN (link here: http://msdn.microsoft.com/en-us/library/windows/desktop/ms683209(v=vs.85).as... ) "A pending operation is indicated when the function that started the operation returns *FALSE*, and the *GetLastError*<http://msdn.microsoft.com/en-us/library/windows/desktop/ms679360(v=vs.85).as...> function returns *ERROR_IO_PENDING*. When an I/O operation is pending, the function that started the operation resets the *hEvent* member of the *OVERLAPPED* structure to the nonsignaled state. Then when the pending operation has been completed, the system sets the event object to the signaled state." If we can know when event object is in the signaled state we definitely can use a queue directly. Any idea? Regards gelin yan

On Oct 20, 2012, at 7:56 PM, gelin yan <dynamicgl@gmail.com> wrote:
"A pending operation is indicated when the function that started the operation returns FALSE, and the GetLastError function returns ERROR_IO_PENDING. When an I/O operation is pending, the function that started the operation resets the hEvent member of the OVERLAPPED structure to the nonsignaled state. Then when the pending operation has been completed, the system sets the event object to the signaled state."
If we can know when event object is in the signaled state we definitely can use a queue directly. Any idea?
Sounds like you're at the point where you should just try doing an implementation, and if it works and passes all the tests you can think of for it, submit it for code review. Much more speculation without testing wouldn't be useful. Thanks for working on this! -glyph

On Mon, Oct 22, 2012 at 12:06 PM, Glyph <glyph@twistedmatrix.com> wrote:
Hi All Sorry for coming late. The day before yesterday, I dig out what happen in that code. Actually, ERROR_IO_PENDING isn't the root of problem. The problem is doWrite method might be triggered twice instead of once; in particular, when trying to send a large chunks data whose size is bigger then SEND_LIMIT, it will always happen. When doWrite being invoked twice, it means the same buffer data will be sent twice. It is for sure that PB is unable to deserialize these data and finally it raise an exception, now, we can see connection lost. The solution is to make sure doWrite calling in order; hence I introduce a new field named _doWriteCalling to detect whether doWrite call is completed. (due to doWrite always post an event to IOCP, so once _cbWrite get called, it means we can schedule another doWrite). If _cbWrite isn't called, we simply re-schedule the doWrite by reactor.callLater. I attach a modified abstract.py here. You may put it in Path\twisted\internet\iocpreactor and give it a try. Regards gelin yan

On Oct 24, 2012, at 8:48 PM, gelin yan <dynamicgl@gmail.com> wrote:
I attach a modified abstract.py here. You may put it in Path\twisted\internet\iocpreactor and give it a try.
Hi Gelin, Can you generate and attach a patch to the ticket, as per the instructions on <http://twistedmatrix.com/trac/wiki/TwistedDevelopment#SubmittingaPatch>, so that this work does not get lost? If you can write some test cases, you may want to also submit the ticket for review. Thanks! -glyph

On Oct 24, 2012, at 11:03 PM, gelin yan <dynamicgl@gmail.com> wrote:
Thanks for mentioning that. I never submitted any patch before so probably I need to take some time to investigate how to do it.
No problem. It should be fairly easy. If you have any trouble don't hesitate to ask questions. -glyph

This looks like the kind of thing that could involve using Deferred as part of solution. Instead of callLater(0.8,doWrite), design the mechanism to wire up event-source to fire the deferred and make doWrite be the callback. On Oct 20, 2012, at 8:29:10AM, gelin yan wrote:

I'm top-posting just for consistency in this thread, but in the future please comment inline with trimming :). It's easier to follow for future readers; if they find a post stand-alone in some web archive, the question will come before the answer. Anyhow; Mike, you're definitely closer to the answer; a callLater is unnecessary, the callback should be called on demand. But, given that a queue of write operations is just that - a queue - we don't need a Deferred for every write; the callback to the write operation can just pick up the next queued item. -g On Oct 20, 2012, at 9:07 AM, Mike Winter <miwinter@cisco.com> wrote:

On Sun, Oct 21, 2012 at 3:35 AM, Glyph <glyph@twistedmatrix.com> wrote:
Hi All Thanks for reply. According to MSDN (link here: http://msdn.microsoft.com/en-us/library/windows/desktop/ms683209(v=vs.85).as... ) "A pending operation is indicated when the function that started the operation returns *FALSE*, and the *GetLastError*<http://msdn.microsoft.com/en-us/library/windows/desktop/ms679360(v=vs.85).as...> function returns *ERROR_IO_PENDING*. When an I/O operation is pending, the function that started the operation resets the *hEvent* member of the *OVERLAPPED* structure to the nonsignaled state. Then when the pending operation has been completed, the system sets the event object to the signaled state." If we can know when event object is in the signaled state we definitely can use a queue directly. Any idea? Regards gelin yan

On Oct 20, 2012, at 7:56 PM, gelin yan <dynamicgl@gmail.com> wrote:
"A pending operation is indicated when the function that started the operation returns FALSE, and the GetLastError function returns ERROR_IO_PENDING. When an I/O operation is pending, the function that started the operation resets the hEvent member of the OVERLAPPED structure to the nonsignaled state. Then when the pending operation has been completed, the system sets the event object to the signaled state."
If we can know when event object is in the signaled state we definitely can use a queue directly. Any idea?
Sounds like you're at the point where you should just try doing an implementation, and if it works and passes all the tests you can think of for it, submit it for code review. Much more speculation without testing wouldn't be useful. Thanks for working on this! -glyph

On Mon, Oct 22, 2012 at 12:06 PM, Glyph <glyph@twistedmatrix.com> wrote:
Hi All Sorry for coming late. The day before yesterday, I dig out what happen in that code. Actually, ERROR_IO_PENDING isn't the root of problem. The problem is doWrite method might be triggered twice instead of once; in particular, when trying to send a large chunks data whose size is bigger then SEND_LIMIT, it will always happen. When doWrite being invoked twice, it means the same buffer data will be sent twice. It is for sure that PB is unable to deserialize these data and finally it raise an exception, now, we can see connection lost. The solution is to make sure doWrite calling in order; hence I introduce a new field named _doWriteCalling to detect whether doWrite call is completed. (due to doWrite always post an event to IOCP, so once _cbWrite get called, it means we can schedule another doWrite). If _cbWrite isn't called, we simply re-schedule the doWrite by reactor.callLater. I attach a modified abstract.py here. You may put it in Path\twisted\internet\iocpreactor and give it a try. Regards gelin yan

On Oct 24, 2012, at 8:48 PM, gelin yan <dynamicgl@gmail.com> wrote:
I attach a modified abstract.py here. You may put it in Path\twisted\internet\iocpreactor and give it a try.
Hi Gelin, Can you generate and attach a patch to the ticket, as per the instructions on <http://twistedmatrix.com/trac/wiki/TwistedDevelopment#SubmittingaPatch>, so that this work does not get lost? If you can write some test cases, you may want to also submit the ticket for review. Thanks! -glyph

On Oct 24, 2012, at 11:03 PM, gelin yan <dynamicgl@gmail.com> wrote:
Thanks for mentioning that. I never submitted any patch before so probably I need to take some time to investigate how to do it.
No problem. It should be fairly easy. If you have any trouble don't hesitate to ask questions. -glyph
participants (3)
-
gelin yan
-
Glyph
-
Mike Winter