You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2014/06/26 14:14:18 UTC

Re: [serf-dev] Plan to release serf 1.4.0 (+ TODO list)

On 22 June 2014 14:13, Lieven Govaerts <lg...@mobsol.be> wrote:
> Hi,
>
>
> I propose we start planning a serf 1.4 release.
>
> Serf trunk has some features that will be used in the upcoming
> Subversion 1.9, so I'd like to get 1.4 out when the Subversion devs
> start releasing 1.9 release candidates. (When is that planned?)  Also,
> I want to start working on HTTP 2.0 support on trunk. This is probably
> going to take me a couple of months and will require some API changes,
> I'd like to get the 1.4 release out of the way first.
>
> The idea is to create a 1.4.x branch soon, let's say next week Sunday,
> June 30.
June 30 is very tight schedule for me: I'm busy with Subversion 1.9.x
stuff and also
have several things to resolve in serf 1.4.x (see below). Could you
move branch time to
week later at least?

> We can keep backporting changes to that branch for a while,
> but I'd like to release as soon as the API is stable (see TODO list).
> AFAIC that can be by the end of next month.
>
[..]
>
> Fixes:
> --------
> r2382: Fix problems with NTLM authentication when accessing local
> server using alias hostname (Windows)
While the problem is important, Michael raised some concerns about my fix:
https://groups.google.com/forum/#!topic/serf-dev/yFPCis1zNNI

These concerns have to be resolved before branch: probably we need add some
configuration option to change reverse lookup behavior.

[..]

> TODO:
> =====
> Blocking the release:
> ---------------------------
> * Finalize get_remaining(): Ivan has added this function to the bucket
> API, but it's only implemented for some buckets.
As far I remember it's implemented for all buckets where length is known.

> I don't think the new API is in use (don't see it in svn trunk). Does it still serve a
> purpose?
This still useful for Subversion. It's currently unused Subversion
only because serf-trunk has version 2.0.0, not 1.4.0

[...]

> * Logging: there are some TODO's concerning logging. Logging of raw
> messages is not readable, debugging symbols that need to be removed,
> negative performance impact of config lookup in socket_writev.
I'm still want to review logging changes because personally I would
like to have numbered indexes for configuration options instead of
strings. To avoid use of apr_hash for better performance.

>
> * set the version to 1.4: I'll do that on the 1.4.x branch.
>
> Issues I think we should fix in 1.4:
> ---------------------------------------------
> * Fix cancellation of failed requests when an authn callback is set
> (r2360). See the discussion and proposal to fix this:
> https://groups.google.com/d/msg/serf-dev/3_HLwF2lVsY/bcR-fatxGB0J
>
It still in my todo list :(

> * issue 144: svn commit hangs over https+kerberos. It's not clear to
> me yet what causes this issue, but getting serf in an endless loop is
> an issue we should fix. Not blocking 1.4, I doubt a solution will
> require API changes.
>
> What about...
> ------------------
> * SSL session sharing? Ivan's work in r1982 to enable reuse of SSL
> sessions was reverted to make place for a better solution. The
> performance benefits of this work were really good, any interest to
> implement the proposed better solution?
> (see https://groups.google.com/d/msg/serf-dev/W6vNiUlMDDw/t1Pnr0Zq-aQJ)
>
Unfortunetaly I don't have enough time to work on this now, because
I've a lot of things to do this summer. So I think we could defer it
to serf 1.5.0.

> * OCSP stapling support: Justin posted a patch to add OCSP stapling
> support to serf's OpenSSL layer. We can discuss what benefits it will
> actually bring, but AFAIC besides some more code to support there's no
> harm in adding it, and people will use it if it's available (e.g. the
> place I'm working now requires either CRL or OCSP (stapling) support)
> See https://groups.google.com/d/msg/serf-dev/rbFtTyE9E_w/l5lWaBIBDE0J .
>
It looks like nice feature to have. I didn't tested proposed patch yet.


-- 
Ivan Zhakov

Re: [serf-dev] Plan to release serf 1.4.0 (+ TODO list)

Posted by Lieven Govaerts <lg...@mobsol.be>.
On Thu, Jun 26, 2014 at 2:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 22 June 2014 14:13, Lieven Govaerts <lg...@mobsol.be> wrote:
>> Hi,
>>
>>
>> I propose we start planning a serf 1.4 release.
>>
>> Serf trunk has some features that will be used in the upcoming
>> Subversion 1.9, so I'd like to get 1.4 out when the Subversion devs
>> start releasing 1.9 release candidates. (When is that planned?)  Also,
>> I want to start working on HTTP 2.0 support on trunk. This is probably
>> going to take me a couple of months and will require some API changes,
>> I'd like to get the 1.4 release out of the way first.
>>
>> The idea is to create a 1.4.x branch soon, let's say next week Sunday,
>> June 30.
> June 30 is very tight schedule for me: I'm busy with Subversion 1.9.x
> stuff and also
> have several things to resolve in serf 1.4.x (see below). Could you
> move branch time to
> week later at least?
Ok.

>
>> We can keep backporting changes to that branch for a while,
>> but I'd like to release as soon as the API is stable (see TODO list).
>> AFAIC that can be by the end of next month.
>>
> [..]
>>
>> Fixes:
>> --------
>> r2382: Fix problems with NTLM authentication when accessing local
>> server using alias hostname (Windows)
> While the problem is important, Michael raised some concerns about my fix:
> https://groups.google.com/forum/#!topic/serf-dev/yFPCis1zNNI
>
> These concerns have to be resolved before branch: probably we need add some
> configuration option to change reverse lookup behavior.
>
> [..]
>
>> TODO:
>> =====
>> Blocking the release:
>> ---------------------------
>> * Finalize get_remaining(): Ivan has added this function to the bucket
>> API, but it's only implemented for some buckets.
> As far I remember it's implemented for all buckets where length is known.
>
>> I don't think the new API is in use (don't see it in svn trunk). Does it still serve a
>> purpose?
> This still useful for Subversion. It's currently unused Subversion
> only because serf-trunk has version 2.0.0, not 1.4.0
>
I don't understand what you're saying here. Are you saying that if we
include the get_remaining API in serf 1.4.0, that you'll make svn 1.9
use it?
If there are no short term plans to start using it, I'd remove it from
the 1.4 release and keep it on trunk for 1.5 or later.

> [...]
>
>> * Logging: there are some TODO's concerning logging. Logging of raw
>> messages is not readable, debugging symbols that need to be removed,
>> negative performance impact of config lookup in socket_writev.
> I'm still want to review logging changes because personally I would
> like to have numbered indexes for configuration options instead of
> strings. To avoid use of apr_hash for better performance.

Ok, the lookup is indeed quite expensive.

>>
>> * set the version to 1.4: I'll do that on the 1.4.x branch.
>>
>> Issues I think we should fix in 1.4:
>> ---------------------------------------------
>> * Fix cancellation of failed requests when an authn callback is set
>> (r2360). See the discussion and proposal to fix this:
>> https://groups.google.com/d/msg/serf-dev/3_HLwF2lVsY/bcR-fatxGB0J
>>
> It still in my todo list :(
I'll see if I can have some time in the coming week to work on my proposal.

>
>> * issue 144: svn commit hangs over https+kerberos. It's not clear to
>> me yet what causes this issue, but getting serf in an endless loop is
>> an issue we should fix. Not blocking 1.4, I doubt a solution will
>> require API changes.
>>
>> What about...
>> ------------------
>> * SSL session sharing? Ivan's work in r1982 to enable reuse of SSL
>> sessions was reverted to make place for a better solution. The
>> performance benefits of this work were really good, any interest to
>> implement the proposed better solution?
>> (see https://groups.google.com/d/msg/serf-dev/W6vNiUlMDDw/t1Pnr0Zq-aQJ)
>>
> Unfortunetaly I don't have enough time to work on this now, because
> I've a lot of things to do this summer. So I think we could defer it
> to serf 1.5.0.

Ok.

>
>> * OCSP stapling support: Justin posted a patch to add OCSP stapling
>> support to serf's OpenSSL layer. We can discuss what benefits it will
>> actually bring, but AFAIC besides some more code to support there's no
>> harm in adding it, and people will use it if it's available (e.g. the
>> place I'm working now requires either CRL or OCSP (stapling) support)
>> See https://groups.google.com/d/msg/serf-dev/rbFtTyE9E_w/l5lWaBIBDE0J .
>>
> It looks like nice feature to have. I didn't tested proposed patch yet.
>
>
thanks,

Lieven

> --
> Ivan Zhakov
>
> --
> You received this message because you are subscribed to the Google Groups "Serf Development List" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to serf-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to serf-dev@googlegroups.com.
> Visit this group at http://groups.google.com/group/serf-dev.
> For more options, visit https://groups.google.com/d/optout.

Re: [serf-dev] Plan to release serf 1.4.0 (+ TODO list)

Posted by Lieven Govaerts <lg...@mobsol.be>.
On Mon, Aug 31, 2015 at 3:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 8 September 2014 at 13:03, Lieven Govaerts <lg...@mobsol.be> wrote:
>> On Mon, Sep 8, 2014 at 11:57 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> On 7 September 2014 12:32, Lieven Govaerts <lg...@mobsol.be> wrote:
>>>> On Mon, Sep 1, 2014 at 7:08 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>>> On 19 August 2014 12:39, Lieven Govaerts <lg...@mobsol.be> wrote:
>>>>>> On Thu, Jun 26, 2014 at 1:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> ..
>>>>
>>>>> Do we really need this feature?
>>>>
>>>> An application needs to know about the serf_config_t objects anyhow,
>>>> so it seemed useful to me to also offer its benefits to the
>>>> application.
>>>>
>>>> Why does the application need to know about serf_config_t?
>>>> The serf_config_t baton is passed by the context loop to the request
>>>> bucket and response bucket. Any bucket in the request/response chains
>>>> needs to pass this baton to the next bucket, so that the new logging
>>>> feature works correctly.
>>>> An application that implements its own buckets has to implement the
>>>> set_config() method to pass the serf_config_t baton to any wrapped
>>>> buckets.
>>>>
>>>> What are the benefits to the application?
>>>> First, by giving application buckets access to information stored by
>>>> serf (host name, port & ip addresses). An application can use this to
>>>> implements its own logging.
>>>> Second, by allowing the application to store its own key/value pairs
>>>> to pass around between buckets. Of course, the application can provide
>>>> any information to its own buckets by modifying the bucket
>>>> constructor, a luxury we don't have in serf due to backwards
>>>> compatibility guarantees. So what's a benefit for serf might not be as
>>>> useful for applications.
>>>>
>>> I understand why application need access to serf_config_t, but I still
>>> do not see reasons to support storing application data in
>>> svn_config_t. In opposite I see two arguments to use serf_config_t
>>> only for serf data:
>>> 1. No compatibility problems in future
>>> 2. We can have simple and very efficient store using pre-allocated
>>> since we know keys count
>>>
>>
>> Fair enough, I don't have good arguments pro this feature.
> Lieven, sorry for delayed reply.
>
> Could you please update serf_config_t documentation (or code?) before
> 1.4.0 to avoid API promise?

It's on my TODO list, thanks for the reminder!
Lieven

>
> --
> Ivan Zhakov

Re: [serf-dev] Plan to release serf 1.4.0 (+ TODO list)

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 8 September 2014 at 13:03, Lieven Govaerts <lg...@mobsol.be> wrote:
> On Mon, Sep 8, 2014 at 11:57 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On 7 September 2014 12:32, Lieven Govaerts <lg...@mobsol.be> wrote:
>>> On Mon, Sep 1, 2014 at 7:08 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> On 19 August 2014 12:39, Lieven Govaerts <lg...@mobsol.be> wrote:
>>>>> On Thu, Jun 26, 2014 at 1:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> ..
>>>
>>>> Do we really need this feature?
>>>
>>> An application needs to know about the serf_config_t objects anyhow,
>>> so it seemed useful to me to also offer its benefits to the
>>> application.
>>>
>>> Why does the application need to know about serf_config_t?
>>> The serf_config_t baton is passed by the context loop to the request
>>> bucket and response bucket. Any bucket in the request/response chains
>>> needs to pass this baton to the next bucket, so that the new logging
>>> feature works correctly.
>>> An application that implements its own buckets has to implement the
>>> set_config() method to pass the serf_config_t baton to any wrapped
>>> buckets.
>>>
>>> What are the benefits to the application?
>>> First, by giving application buckets access to information stored by
>>> serf (host name, port & ip addresses). An application can use this to
>>> implements its own logging.
>>> Second, by allowing the application to store its own key/value pairs
>>> to pass around between buckets. Of course, the application can provide
>>> any information to its own buckets by modifying the bucket
>>> constructor, a luxury we don't have in serf due to backwards
>>> compatibility guarantees. So what's a benefit for serf might not be as
>>> useful for applications.
>>>
>> I understand why application need access to serf_config_t, but I still
>> do not see reasons to support storing application data in
>> svn_config_t. In opposite I see two arguments to use serf_config_t
>> only for serf data:
>> 1. No compatibility problems in future
>> 2. We can have simple and very efficient store using pre-allocated
>> since we know keys count
>>
>
> Fair enough, I don't have good arguments pro this feature.
Lieven, sorry for delayed reply.

Could you please update serf_config_t documentation (or code?) before
1.4.0 to avoid API promise?

-- 
Ivan Zhakov

Re: [serf-dev] Plan to release serf 1.4.0 (+ TODO list)

Posted by Lieven Govaerts <lg...@mobsol.be>.
On Thu, Jun 26, 2014 at 1:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 22 June 2014 14:13, Lieven Govaerts <lg...@mobsol.be> wrote:
>> Hi,
>>
>>
>> I propose we start planning a serf 1.4 release.
>>
>> Serf trunk has some features that will be used in the upcoming
>> Subversion 1.9, so I'd like to get 1.4 out when the Subversion devs
>> start releasing 1.9 release candidates. (When is that planned?)  Also,
>> I want to start working on HTTP 2.0 support on trunk. This is probably
>> going to take me a couple of months and will require some API changes,
>> I'd like to get the 1.4 release out of the way first.
>>
>> The idea is to create a 1.4.x branch soon, let's say next week Sunday,
>> June 30.
> June 30 is very tight schedule for me: I'm busy with Subversion 1.9.x
> stuff and also
> have several things to resolve in serf 1.4.x (see below). Could you
> move branch time to
> week later at least?
>
>> We can keep backporting changes to that branch for a while,
>> but I'd like to release as soon as the API is stable (see TODO list).
>> AFAIC that can be by the end of next month.
>>
> [..]
>>
>> Fixes:
>> --------
>> r2382: Fix problems with NTLM authentication when accessing local
>> server using alias hostname (Windows)
> While the problem is important, Michael raised some concerns about my fix:
> https://groups.google.com/forum/#!topic/serf-dev/yFPCis1zNNI
>
> These concerns have to be resolved before branch: probably we need add some
> configuration option to change reverse lookup behavior.
>
> [..]
>
>> TODO:
>> =====
>> Blocking the release:
>> ---------------------------
>> * Finalize get_remaining(): Ivan has added this function to the bucket
>> API, but it's only implemented for some buckets.
> As far I remember it's implemented for all buckets where length is known.
>
>> I don't think the new API is in use (don't see it in svn trunk). Does it still serve a
>> purpose?
> This still useful for Subversion. It's currently unused Subversion
> only because serf-trunk has version 2.0.0, not 1.4.0
>
> [...]
>
>> * Logging: there are some TODO's concerning logging. Logging of raw
>> messages is not readable, debugging symbols that need to be removed,
>> negative performance impact of config lookup in socket_writev.

> I'm still want to review logging changes because personally I would
> like to have numbered indexes for configuration options instead of
> strings. To avoid use of apr_hash for better performance.

@Ivan: r2416 eliminates those hash tables that are actually slow, the
ones where the configuration key/value pair are stored in. I've
replaced them by a linked list. Since we only have one or two keys per
category lookup is now a lot faster.
We could have used an array here, but since the config store is part
of the public API and applications are allowed to define their own
keys, we 1) need to partition serf internal and application keys and
2) it's better to not assume that the application dev will use 1, 2,
3... as key numbers. Hence, a linked list is the better option.

I've kept the hash tables that store the per connection/per host
config objects. These are only rarely accessed and are best used with
string keys so there's no problem in using them.

Lieven


>>
>> * set the version to 1.4: I'll do that on the 1.4.x branch.
>>
>> Issues I think we should fix in 1.4:
>> ---------------------------------------------
>> * Fix cancellation of failed requests when an authn callback is set
>> (r2360). See the discussion and proposal to fix this:
>> https://groups.google.com/d/msg/serf-dev/3_HLwF2lVsY/bcR-fatxGB0J
>>
> It still in my todo list :(
>
>> * issue 144: svn commit hangs over https+kerberos. It's not clear to
>> me yet what causes this issue, but getting serf in an endless loop is
>> an issue we should fix. Not blocking 1.4, I doubt a solution will
>> require API changes.
>>
>> What about...
>> ------------------
>> * SSL session sharing? Ivan's work in r1982 to enable reuse of SSL
>> sessions was reverted to make place for a better solution. The
>> performance benefits of this work were really good, any interest to
>> implement the proposed better solution?
>> (see https://groups.google.com/d/msg/serf-dev/W6vNiUlMDDw/t1Pnr0Zq-aQJ)
>>
> Unfortunetaly I don't have enough time to work on this now, because
> I've a lot of things to do this summer. So I think we could defer it
> to serf 1.5.0.
>
>> * OCSP stapling support: Justin posted a patch to add OCSP stapling
>> support to serf's OpenSSL layer. We can discuss what benefits it will
>> actually bring, but AFAIC besides some more code to support there's no
>> harm in adding it, and people will use it if it's available (e.g. the
>> place I'm working now requires either CRL or OCSP (stapling) support)
>> See https://groups.google.com/d/msg/serf-dev/rbFtTyE9E_w/l5lWaBIBDE0J .
>>
> It looks like nice feature to have. I didn't tested proposed patch yet.
>
>
> --
> Ivan Zhakov
>
> --
> You received this message because you are subscribed to the Google Groups "Serf Development List" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to serf-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to serf-dev@googlegroups.com.
> Visit this group at http://groups.google.com/group/serf-dev.
> For more options, visit https://groups.google.com/d/optout.