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 2013/08/27 15:29:05 UTC
Re: svn commit: r1099094 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c
On Tue, May 3, 2011 at 7:39 PM, <ph...@apache.org> wrote:
> Author: philip
> Date: Tue May 3 15:39:55 2011
> New Revision: 1099094
>
> URL: http://svn.apache.org/viewvc?rev=1099094&view=rev
> Log:
> Revert part of r1099044. Files still get closed explicitly, to
> avoid problems with too many open files, but the files will
> exist on disk for longer.
>
> * subversion/libsvn_ra_serf/commit.c
> (apply_textdelta): Go back to svn_io_file_del_on_pool_cleanup.
>
Hi Philip,
Do you remember why you reverted delete on close approach? Did you
find any problems? Because currently file_baton pool is not cleared
until close_edit and temporary files remain on disk until commit
finishes.
--
Ivan Zhakov
Re: svn commit: r1099094 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c
Posted by Philip Martin <ph...@wandisco.com>.
Ben Reser <be...@reser.org> writes:
> On 8/28/13 7:35 AM, Ivan Zhakov wrote:
>> Bert refers to r712674 [1] -- 4 years old change in APR, so it should
>> not be issue now.
>>
>> @Bert: Do you remember why temporary file could be not removed on file close?
>>
>> [1] http://svn.apache.org/viewvc?view=revision&revision=r712674
>
> Unless we've raised our requirement of APR to not allow versions that
> have this issue we should leave any workarounds we have for it in. We
> end up with a lot of users running on old Linux distros that use
> whatever APR is provided by the distros. Heck our own CentOS buildbot
> is one of these cases, it's using APR/APR-Util 1.2.7 which was released
> April 14th, 2006.
>
> If you want to avoid the workarounds for newer APR versions that you
> detect at runtime I'd support that.
>
> I'd support us raising our APR version requirement in Subversion 1.9.x
> to APR 1.3.x, not sure if that helps in this case.
The Subversion change (r1099094) was to revert svn_io_file_del_on_close
back to svn_io_file_del_on_pool_cleanup. The APR change (r712674) was a
fix for APR_DELONCLOSE so I suppose that svn_io_file_del_on_close might
not work properly with old APR.
I don't recall the exact details of what, if anything, failed. I don't
know whether Bert was worried about some specific ra_serf problem with
using svn_io_file_del_on_close or whether it was just a general
observation that there was an APR bug. There are other places in the
current Subversion source code that use svn_io_file_del_on_close so we
don't appear to have a blanket ban on using it.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: svn commit: r1099094 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c
Posted by Ben Reser <be...@reser.org>.
On 8/28/13 7:35 AM, Ivan Zhakov wrote:
> Bert refers to r712674 [1] -- 4 years old change in APR, so it should
> not be issue now.
>
> @Bert: Do you remember why temporary file could be not removed on file close?
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=r712674
Unless we've raised our requirement of APR to not allow versions that
have this issue we should leave any workarounds we have for it in. We
end up with a lot of users running on old Linux distros that use
whatever APR is provided by the distros. Heck our own CentOS buildbot
is one of these cases, it's using APR/APR-Util 1.2.7 which was released
April 14th, 2006.
If you want to avoid the workarounds for newer APR versions that you
detect at runtime I'd support that.
I'd support us raising our APR version requirement in Subversion 1.9.x
to APR 1.3.x, not sure if that helps in this case.
Re: svn commit: r1099094 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Aug 27, 2013 at 6:45 PM, Philip Martin
<ph...@wandisco.com> wrote:
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
>> On Tue, May 3, 2011 at 7:39 PM, <ph...@apache.org> wrote:
>>> Author: philip
>>> Date: Tue May 3 15:39:55 2011
>>> New Revision: 1099094
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1099094&view=rev
>>> Log:
>>> Revert part of r1099044. Files still get closed explicitly, to
>>> avoid problems with too many open files, but the files will
>>> exist on disk for longer.
>>>
>>> * subversion/libsvn_ra_serf/commit.c
>>> (apply_textdelta): Go back to svn_io_file_del_on_pool_cleanup.
>>>
>> Hi Philip,
>>
>> Do you remember why you reverted delete on close approach? Did you
>> find any problems? Because currently file_baton pool is not cleared
>> until close_edit and temporary files remain on disk until commit
>> finishes.
>
> I don't recall the details, there was some discussion on IRC:
>
> http://colabti.org/irclogger/irclogger_log/svn-dev?date=2011-05-03#l123
>
> Bert suggested that r1099044 could cause files not to be deleted at
> all.
>
Bert refers to r712674 [1] -- 4 years old change in APR, so it should
not be issue now.
@Bert: Do you remember why temporary file could be not removed on file close?
[1] http://svn.apache.org/viewvc?view=revision&revision=r712674
--
Ivan Zhakov
Re: svn commit: r1099094 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c
Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:
> On Tue, May 3, 2011 at 7:39 PM, <ph...@apache.org> wrote:
>> Author: philip
>> Date: Tue May 3 15:39:55 2011
>> New Revision: 1099094
>>
>> URL: http://svn.apache.org/viewvc?rev=1099094&view=rev
>> Log:
>> Revert part of r1099044. Files still get closed explicitly, to
>> avoid problems with too many open files, but the files will
>> exist on disk for longer.
>>
>> * subversion/libsvn_ra_serf/commit.c
>> (apply_textdelta): Go back to svn_io_file_del_on_pool_cleanup.
>>
> Hi Philip,
>
> Do you remember why you reverted delete on close approach? Did you
> find any problems? Because currently file_baton pool is not cleared
> until close_edit and temporary files remain on disk until commit
> finishes.
I don't recall the details, there was some discussion on IRC:
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2011-05-03#l123
Bert suggested that r1099044 could cause files not to be deleted at
all.
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*