Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Branko Čibej
On 22.05.2015 10:50, Philip Martin wrote: Branko Čibej br...@wandisco.com writes: FWIW, I think in this case revving the API without deprecating the old one is justified. Another option would be to invent a different API name for the flushing stream, e.g., svn_stream_from_aprfile_flushed or

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Branko Čibej
On 22.05.2015 00:09, Philip Martin wrote: Ivan Zhakov i...@visualsvn.com writes: We considered adding FLUSH_TO_DISK flag to svn_stream_from_apr_file(), but decided to commit simple fix for now: 1. The current FSFS code uses explicit flush and this commit makes code consistent with other

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Branko Čibej br...@wandisco.com writes: FWIW, I think in this case revving the API without deprecating the old one is justified. Another option would be to invent a different API name for the flushing stream, e.g., svn_stream_from_aprfile_flushed or some such. This way we'd also avoid the

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Branko Čibej br...@wandisco.com writes: On 22.05.2015 10:50, Philip Martin wrote: Another approach would be to have a function to enable flushing directly on the stream created by svn_stream_from_aprfile2() without creating a new stream. This is probably the smallest/simplest patch. After

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Ivan Zhakov i...@visualsvn.com writes: On 22 May 2015 at 11:50, Philip Martin philip.mar...@wandisco.com wrote: Another approach would be to have a function to enable flushing directly on the stream created by svn_stream_from_aprfile2() without creating a new stream. This is probably the

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Branko Čibej
I mean we could leave the current 1.8.x backport proposal unchanged. Alternatively, tweaking your patch to make the new API private for 1.8.x is fine; probably even better as it's likely to make future backports easier. On 22 May 2015 12:46 pm, Philip Martin philip.mar...@wandisco.com wrote:

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Ivan Zhakov
On 22 May 2015 at 09:40, Branko Čibej br...@wandisco.com wrote: On 22.05.2015 00:09, Philip Martin wrote: Ivan Zhakov i...@visualsvn.com writes: We considered adding FLUSH_TO_DISK flag to svn_stream_from_apr_file(), but decided to commit simple fix for now: 1. The current FSFS code uses

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Ivan Zhakov i...@visualsvn.com writes: Yes, this patch looks easier to review, the only problem that it's incomplete. I'm attaching minimal working patch with svn_stream_from_file3() against trunk@r1680818. I like this patch better. It puts the flush into 2 extra places, 4 in total, in FSFS,

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Ivan Zhakov
On 22 May 2015 at 11:50, Philip Martin philip.mar...@wandisco.com wrote: Branko Čibej br...@wandisco.com writes: FWIW, I think in this case revving the API without deprecating the old one is justified. Another option would be to invent a different API name for the flushing stream, e.g.,

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Evgeny Kotkov
Philip Martin philip.mar...@wandisco.com writes: I like this patch better. It puts the flush into 2 extra places, 4 in total, in FSFS, and the corresponding 4 places in FSX. For 1.8 we would make the new API private: svn_stream__flush_to_disk_on_close. Index: subversion/include/svn_io.h

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Ivan Zhakov i...@visualsvn.com writes: I intentionally kept packing code unchanged: it seems that now we don't use hardware flush when packing repository: so we need apply fix to all pack code, which is separate issue IMO. Yes! I can confirm that with strace: we write pack/manifest files for

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

2015-05-22 Thread Philip Martin
Evgeny Kotkov evgeny.kot...@visualsvn.com writes: Or is it just a gut feeling that we should be using streams here? We have been gradually moving our file-based code to stream-based code for years. -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*

Re: Segfault in Perl bindings when commit touches a large number of files

2015-05-22 Thread Philip Martin
Philip Martin philip.mar...@wandisco.com writes: So, following the approach of r876245, would something like this do the trick? It would help if I built the correct tree. No, that is not enough, the regression tests fail. -- Philip

Re: Segfault in Perl bindings when commit touches a large number of files

2015-05-22 Thread Roderich Schupp
On Fri, May 22, 2015 at 6:07 PM, Philip Martin phi...@codematters.co.uk wrote: It would help if I built the correct tree. No, that is not enough, the regression tests fail. No surprise here: (1) Your patch simply means: ignore the result_digest, but also produce no return value for it. But

Re: Segfault in Perl bindings when commit touches a large number of files

2015-05-22 Thread Roderich Schupp
On Fri, May 22, 2015 at 6:25 PM, Roderich Schupp roderich.sch...@gmail.com wrote: But I'm not entirely convinced that the bug is really in the construction of the magical md5sum. Maybe git-svn is to blame, perhaps a problem with the lifetime of the pools it uses... I just modified the Swig