On 14 November 2015 at 12:06, Bert Huijben <b...@qqmail.nl> wrote: >> -----Original Message----- >> From: Ivan Zhakov [mailto:i...@visualsvn.com] >> Sent: woensdag 11 november 2015 18:01 >> To: Bert Huijben <b...@qqmail.nl> >> Cc: rhuij...@apache.org; dev@serf.apache.org >> Subject: Re: svn commit: r1713489 - serf_bucket_readline() >> >> On 11 November 2015 at 19:51, Bert Huijben <b...@qqmail.nl> wrote: >> >> -----Original Message----- >> >> From: Ivan Zhakov [mailto:i...@visualsvn.com] >> >> Sent: woensdag 11 november 2015 13:22 >> >> To: Bert Huijben <b...@qqmail.nl> >> >> Cc: rhuij...@apache.org; dev@serf.apache.org >> >> Subject: Re: svn commit: r1713489 - serf_bucket_readline() >> >> >> >> On 11 November 2015 at 01:44, Bert Huijben <b...@qqmail.nl> wrote: >> >> >> -----Original Message----- >> >> >> From: Bert Huijben [mailto:b...@qqmail.nl] >> >> >> Sent: dinsdag 10 november 2015 23:37 >> >> >> To: 'Ivan Zhakov' <i...@visualsvn.com>; rhuij...@apache.org >> >> >> Cc: dev@serf.apache.org >> >> >> Subject: RE: svn commit: r1713489 - in /serf/trunk: >> >> buckets/event_buckets.c >> >> >> outgoing.c serf_private.h >> >> >> > -----Original Message----- >> >> >> > From: Ivan Zhakov [mailto:i...@visualsvn.com] >> >> >> > Sent: dinsdag 10 november 2015 20:43 >> >> >> > To: rhuij...@apache.org >> >> >> > Cc: dev@serf.apache.org >> >> >> > Subject: Re: svn commit: r1713489 - in /serf/trunk: >> >> >> buckets/event_buckets.c >> >> >> > outgoing.c serf_private.h >> >> >> > >> >> >> > On 9 November 2015 at 20:49, <rhuij...@apache.org> wrote: >> >> >> > > Author: rhuijben >> >> >> > > Date: Mon Nov 9 17:49:59 2015 >> >> >> > > New Revision: 1713489 >> >> >> > > >> >> >> > > URL: http://svn.apache.org/viewvc?rev=1713489&view=rev >> >> >> > > Log: >> >> >> > > Replace the track bucket in the request writing with a tiny bit >> >> >> > > more >> >> >> > > advanced event bucket that tracks both writing done and >> destroyed. >> >> The >> >> >> > > timings of these callbacks will allow simplifying some logic >> introduced >> >> >> > > in r1712776. >> >> >> > > >> >> >> > > For now declare the event bucket as a private type. >> >> >> > > >> >> >> > > * buckets/event_buckets.c >> >> >> > > New file. >> >> >> > > >> >> >> > > * outgoing.c >> >> >> > > (request_writing_done): New function. >> >> >> > > (request_writing_finished): Tweak to implement >> >> >> > serf_bucket_event_callback_t. >> >> >> > > (write_to_connection): Add event bucket directly after the >> request >> >> >> > bucket, >> >> >> > > instead of an aggregate when the writing is done. >> >> >> > > >> >> >> > Hi Bert, >> >> >> > >> >> >> > What do you think about alternative design for event buckets: make >> >> >> > event bucket wrap any other bucket (request bucket in our case)? I >> >> >> > think it will be more flexible since we could add callback to be >> >> >> > called before reading from wrapped bucket to track pending >> request. >> >> >> >> >> >> That might work, but would have different characteristics unless you >> do >> >> >> more special things. >> >> > >> >> > There is one more problem with all this wrapping: All layers need to >> >> somehow support all read methods. >> >> > >> >> > Wrapping works for all methods except: readline. >> >> > >> >> How is event bucket situation different from barrier bucket for example? >> > >> > The event bucket currently doesn't forward a single read method, while >> the barrier forwards all? >> > >> I understand that currently event bucket doesn't forward read and >> other methods. But my suggestion was to make event bucket work >> differently and wrap existing bucket. > > I'm starting to think that we should switch to wrapping for the destroy > event, but > perhaps for a different reason than stated before: The current code depends on > implementation details of the aggregate bucket. > > There are certain cases where it might destroy buckets (or might have > destroyed) > out of order and when you wrap the bucket you know exactly. Not depending on > the knowledge how this bucket works makes this more generic... and better able > to handle other aggregation buckets. I thought about the same reason, but missed it in my original email. Sorry.
-- Ivan Zhakov
