Avoinding file handle leak using the Python bindings & core.Stream
Hello, I've been working on an full text search plugin for Trac. At initial setup this indexes the entire Subversion repository by reading every node of every version. During testing we discovered that the indexer was running out of file handles, due to a file handle leak. As far as I can tell each core.Stream(fs.file_contents(.)) instance that was created and not subsequently .read() left an unclosed file handle. To work around this I have monkey patched a Stream.close() method that calls svn_stream_close, which is used in a try/finally block. The work-around has fixed our file-handle leak for, but I believe it points to a bug in the Subversion bindings for which I'll try and provide a patch. Before I file a bug I'd like to check I haven't misunderstood anything: 1. In the Python bindings core.Stream doesn't have a .close() method [a]. Is there any reason this might be intentional? 2. Disregarding Python, in the Subversion library is it required that every svn_stream_t created (by eg a call to svn_fs_file_contents) is explicitly closed, or is there some automatic clean-up/closure provided by the pool system? With thanks, Alex [a] http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/svn/core.py?revision=1303375&view=markup#l202 -- Alex Willmer | Developer 2 Trinity Park, Birmingham, B37 7ES | United Kingdom M: +44 7557 752744 al.will...@logica.com | www.logica.com Logica UK Ltd, registered in UK (registered number 947968) Registered Office: 250 Brook Drive, Green Park, Reading RG2 6UA, United Kingdom Think green - keep it on the screen. This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Re: Avoinding file handle leak using the Python bindings & core.Stream
Am 16.04.2012 11:29, schrieb Willmer, Alex (PTS): > I've been working on an full text search plugin for Trac. Nice! ;) > At initial setup this indexes the entire Subversion repository by > reading every node of every version. During testing we discovered > that the indexer was running out of file handles, due to a file > handle leak. As far as I can tell each core.Stream(fs.file_contents(.)) > instance that was created and not subsequently .read() left an > unclosed file handle. To work around this I have monkey patched a > Stream.close() method that calls svn_stream_close, which is used > in a try/finally block. Disclaimer: I'm not really familiar with the SVN/Python bindings. However, concerning Python in general, explicitly calling close() is the wrong way. Instead you should make sure files are closed by opening them in a with clause: with open(...) as f: ... # use f This will make sure that 'f' gets a C++-like scope and is automatically closed on exit of that scope, regardless whether the exit is due to an exception or a "normal" exit. You can easily add this behaviour by creating an __entry__/__exit__ function, look up "context manager" on the web. That said, when the core.Stream instance is garbage-collected (normally when it goes out of scope), it also releases its "self._stream" which can subsequently be garbage-collected. If this object in turn requires explicit cleanup of its internal resources, it should provide a __del__ method doing that. If it leaks this a file handle in the (unusual?) case that the content wasn't read, that is the place where the bug actually is. > The work-around has fixed our file-handle leak for, but I believe it > points to a bug in the Subversion bindings for which I'll try and > provide a patch. Before I file a bug I'd like to check I haven't > misunderstood anything: > 1. In the Python bindings core.Stream doesn't have a .close() > method [a]. Is there any reason this might be intentional? I guess that yes. The point is that the file interface only has read() and write(), but not close(). In other words, functions that are supposed to work with files and file-like types should only expect those two functions, not e.g. a close(). I can't comment on your second question, I'm not familiar enough with the API. Good luck! Uli ** Domino Laser GmbH, Fangdieckstraße 75a, 22547 Hamburg, Deutschland Geschäftsführer: Thorsten Föcking, Amtsgericht Hamburg HR B62 932 ** Visit our website at http://www.dominolaser.com ** Diese E-Mail einschließlich sämtlicher Anhänge ist nur für den Adressaten bestimmt und kann vertrauliche Informationen enthalten. Bitte benachrichtigen Sie den Absender umgehend, falls Sie nicht der beabsichtigte Empfänger sein sollten. Die E-Mail ist in diesem Fall zu löschen und darf weder gelesen, weitergeleitet, veröffentlicht oder anderweitig benutzt werden. E-Mails können durch Dritte gelesen werden und Viren sowie nichtautorisierte Änderungen enthalten. Domino Laser GmbH ist für diese Folgen nicht verantwortlich. **
'svn cleanup' ignores externals
Is it by design that 'svn cleanup' ignores externals and that there is no way to include them? I found it a bit inconsistent that 'working copy' has different meanings for 'svn update' and 'svn cleanup'. Kuno $>svn help update update (up): Bring changes from the repository into the working copy. $>svn help cleanup cleanup: Recursively clean up the working copy, removing locks, resuming $> svn up Fetching external item into '': svn: warning: W155037: Previous operation has not finished; run 'cleanup' if it was interrupted At revision 62460. svn: E205011: Failure occurred processing one or more externals definitions $> svn cleanup $> svn up Fetching external item into '': svn: warning: W155037: Previous operation has not finished; run 'cleanup' if it was interrupted At revision 62460. svn: E205011: Failure occurred processing one or more externals definitions
Re: 'svn cleanup' ignores externals
Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: > Is it by design that 'svn cleanup' ignores externals and that there is no way > to > include them? > You should be able to include them by naming them explicitly: % svn cleanup path/to/external/dir > I found it a bit inconsistent that 'working copy' has different meanings for > 'svn update' and 'svn cleanup'. > I think we can fix that in 1.8. We have --include-externals and --ignore-externals flags; we just need to decide whether to include externals by default or not. Daniel > Kuno > > > > $>svn help update > update (up): Bring changes from the repository into the working copy. > > $>svn help cleanup > cleanup: Recursively clean up the working copy, removing locks, resuming > > > > $> svn up > > Fetching external item into '': > svn: warning: W155037: Previous operation has not finished; run 'cleanup' if > it > was interrupted > > At revision 62460. > svn: E205011: Failure occurred processing one or more externals definitions > > $> svn cleanup > > $> svn up > > Fetching external item into '': > svn: warning: W155037: Previous operation has not finished; run 'cleanup' if > it > was interrupted > > At revision 62460. > svn: E205011: Failure occurred processing one or more externals definitions > >
RE: Feature request: inhibiting checkout for tags and branches
I don't have apxs and can't find a 64 bit version. Seems like I would also need Perl. Is there a Windows 64 pre-compiled version of mod_dontdothat? Paul Coulson Software Engineer Email:pcoul...@senscient.com A2 Arena Business Centre, Holy Rood Close Poole Dorset BH17 7FJ Tel:+44 (0) 1202 606473 Web:http:// -Original Message- From: Konstantin Kolinko [mailto:knst.koli...@gmail.com] Sent: 30 March 2012 13:06 To: Paul Coulson Cc: users@subversion.apache.org Subject: Re: Feature request: inhibiting checkout for tags and branches 2012/3/29 coolie : > I have an inherited repo structure that has many projects with their > own tags and branches folders. > Users check out the whole structure as there are common lib references > etc, but don't need to see the full contents of tags or branches > folders, which can be massive. I would like a property svn:inhibit > similar to svn:ignore set on the root folder that limits the checkout > depth to immediate (or empty) for a folder anywhere in the tree > matching the list. > > By setting as a property, then a new checkout will inherit this > property and limit the checkout depth for the named folders as required. > > Users can then checkout particular tag versions as normal if they > require past versions. > > IMHO this will make the behavior of tags more like what the SVN manual > says is just a "user friendly name for a revision". > You wouldn't want to check out all revisions on the trunk, so why > check out all revisions for tags by default? > > This would be a really cool improvement, as I see the problem has been > mentioned many times on this group. mod_dontdothat ? http://svn.apache.org/repos/asf/subversion/branches/1.7.x/tools/server-side/mod_dontdothat/README BTW, why are you sending your question to "subversion_us...@googlegroups.com" ? The proper address of this mailing list is users.at.subversion.apache.org, http://subversion.apache.org/mailing-lists.html Best regards, Konstantin Kolinko Senscient Ltd. is a limited company registered in England and Wales. Registered number:5014794. Registered office:Mountbatten House 1 Grosvenor Square Southampton SO15 2BZ Please note that Senscient Ltd. may monitor email traffic data and also the content of email for the purposes of security and staff training. This message contains confidential information and is intended only for the intended recipients. If you are not an intended recipient you should not disseminate, distribute or copy this e-mail. Please notify postmas...@senscient.com immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. Senscient Ltd. therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. If verification is required please request a hard-copy version.
RE: 'svn cleanup' ignores externals
> -Original Message- > From: Daniel Shahaf [mailto:danie...@elego.de] > Sent: 16 April 2012 12:49 > To: Kuno Meyer > Cc: users@subversion.apache.org > Subject: Re: 'svn cleanup' ignores externals > > Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: > > Is it by design that 'svn cleanup' ignores externals and > > that there is no way to include them? > > > > You should be able to include them by naming them explicitly: > > % svn cleanup path/to/external/dir > > > I found it a bit inconsistent that 'working copy' has > > different meanings for 'svn update' and 'svn cleanup'. > > > > I think we can fix that in 1.8. We have --include-externals and > --ignore-externals flags; we just need to decide whether to include > externals by default or not. > > Daniel I would vote for `--include-externals` as the default. I agree with the OP that this is a confusing issue. Alterntively the `run cleanup` message should specify to use `--include-externals` (if required) otherwise it looks like the suggested fix doesn't work and leaves you in a cycle. Just by tuppence. ~ mark c > > Kuno > > > > > > > > $>svn help update > > update (up): Bring changes from the repository into the > working copy. > > > > $>svn help cleanup > > cleanup: Recursively clean up the working copy, removing > locks, resuming > > > > > > > > $> svn up > > > > Fetching external item into '': > > svn: warning: W155037: Previous operation has not finished; > run 'cleanup' if it > > was interrupted > > > > At revision 62460. > > svn: E205011: Failure occurred processing one or more > externals definitions > > > > $> svn cleanup > > > > $> svn up > > > > Fetching external item into '': > > svn: warning: W155037: Previous operation has not finished; > run 'cleanup' if it > > was interrupted > > > > At revision 62460. > > svn: E205011: Failure occurred processing one or more > externals definitions > > > > >
AW: Subversion 1.6.18 released
Hi Stefan, Thanks for the new release! From the list of changes for 1.6.18 I learned that the server side performance of "log -g" was improved (r1152282). This sounds particularly interesting for our setup. Does a similar fix exist in 1.7.4, too? Regards, Thomas This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Torex (Torex Group of Companies). If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone. Please contact the sender if you believe you have received this email in error. Torex Retail Solutions GmbH Sitz der Gesellschaft: Berlin. HRB 102273B Amtsgericht Berlin Charlottenburg. UST.-ID: DE170817515. Steuer-Nr. 27/448/07028. WEEE-Reg.-Nr. DE 30664749. Geschaftsfuhrer: Stephen Callaghan, Martin Timmann. .
Re: Subversion 1.6.18 released
On Mon, Apr 16, 2012 at 8:52 AM, Becker, Thomas wrote: \ > Thanks for the new release! From the list of changes for 1.6.18 I > learned that the server side performance of "log -g" was improved > (r1152282). This sounds particularly interesting for our setup. Does a > similar fix exist in 1.7.4, too? That fix was made in July 2011 and is included in all of the 1.7.x releases. -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: Avoinding file handle leak using the Python bindings & core.Stream
Ulrich Eckhardt dominolaser.com> writes: > Am 16.04.2012 11:29, schrieb Willmer, Alex (PTS): > > To work around this I have monkey patched a > > Stream.close() method that calls svn_stream_close, which is used > > in a try/finally block. > > Disclaimer: I'm not really familiar with the SVN/Python bindings. > However, concerning Python in general, explicitly calling close() is the > wrong way. Instead you should make sure files are closed by opening them > in a with clause: > > with open(...) as f: > ... # use f Unfortunately I'm targeting Python 2.4, but always worth mentioned and I intend to include context manager support in any patch I submit. > That said, when the core.Stream instance is garbage-collected (normally > when it goes out of scope), it also releases its "self._stream" which > can subsequently be garbage-collected. If this object in turn requires > explicit cleanup of its internal resources, it should provide a __del__ > method doing that. If it leaks this a file handle in the (unusual?) case > that the content wasn't read, that is the place where the bug actually is. As I'm sure you're aware using __del__() for object clean-up is tricky and discouraged. However assume-cleanup-on-destruction seems to be how the bindings are used currently (at least by Trac). > > 1. In the Python bindings core.Stream doesn't have a .close() > > method [a]. Is there any reason this might be intentional? > > I guess that yes. The point is that the file interface only has read() > and write(), but not close(). In other words, functions that are > supposed to work with files and file-like types should only expect those > two functions, not e.g. a close(). I see that as the loose definition of "file like" biting us. I was just surprised to learn that the formalised io.IOBase abstract base class in 2.7/3.x has 14 methods/properties http://docs.python.org/library/io.html#io.IOBase.
Re: 'svn cleanup' ignores externals
Daniel Shahaf wrote on Mon, 16 Apr 2012 14:48:45 +0300: > Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: > > Is it by design that 'svn cleanup' ignores externals and that there is > no way to > > include them? > > > > You should be able to include them by naming them explicitly: > > % svn cleanup path/to/external/dir Thanks. This is what I did. However, since I am not the one who set up the "externals" layout of our source code repository, it took me some time to understand the way a working copy containing external references is organized. And then again, I realized that my custom update and rebuild script fails at garbage collecting unreferenced pristines in sub-WCs included by "externals" defnitions. But that's another story... Kuno -- NEU: FreePhone 3-fach-Flat mit kostenlosem Smartphone! Jetzt informieren: http://mobile.1und1.de/?ac=OM.PW.PW003K20328T7073a
Re: 'svn cleanup' ignores externals
Kuno Meyer wrote on Mon, Apr 16, 2012 at 16:51:29 +0200: > Daniel Shahaf wrote on Mon, 16 Apr 2012 14:48:45 +0300: > > Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: > > > Is it by design that 'svn cleanup' ignores externals and that there is > > no way to > > > include them? > > > > > > > You should be able to include them by naming them explicitly: > > > > % svn cleanup path/to/external/dir > > Thanks. This is what I did. > > However, since I am not the one who set up the "externals" layout of our > source code repository, it took me some time to understand the way a working > copy containing external references is organized. > > And then again, I realized that my custom update and rebuild script fails at > garbage collecting unreferenced pristines in sub-WCs included by "externals" > defnitions. But that's another story... That's a good point actually, since presently 'svn cleanup' is required to garbage-collect pristines, having it recurse to externals automatically makes even more sense. Has anyone filed a bug report about this yet? If not I will.. (so it's not forgotten, etc)
Implementations of an SVN/DAV aware proxy?
Hi, Anyone know if there is a workable SVN/DAV proxy that exists that can handle the protocol enough to make all the translations required between differing old/new URIs? Why? We're changing SVN servers from mod-dav-svn to SCM-Manager - and one minor fly in the ointment is the style of our old repo URI, which was of the form: https://host.../repositories/ Our test SCM server is serving on: https://host.../svn/ which is the form we prefer. Naively, I assumed I could create a virtual SVN instance with a simple Apache proxy rule (or rewrite) mapping /repositories/ to /svn/ (we are using Apache in front of SCM anyway). Sadly, the DAV protocol stuffs this up somewhat and it doesn't work. well, svn ls ... works, but svn co ... gets confused, and looking at it with Wireshark suggests some of the DAV/SVN protocol is carrying knowledge of /svn/ deeper in the protocol (ie not just headers). It's not the end of the world - worst case is I can write a perl script to change the URL in any given checkouts (and there are probably a hundrend SVN checkouts with knowledge of the old URI). But it made me curious as Google has not been my friend... Cheers, Tim -- Tim Watts Personal Blog: http://www.dionic.net/tim/
Re: 'svn cleanup' ignores externals
2012/4/16 Daniel Shahaf : > Kuno Meyer wrote on Mon, Apr 16, 2012 at 16:51:29 +0200: >> Daniel Shahaf wrote on Mon, 16 Apr 2012 14:48:45 +0300: >> > Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: >> > > Is it by design that 'svn cleanup' ignores externals and that there is >> > no way to >> > > include them? >> > > >> > >> > You should be able to include them by naming them explicitly: >> > >> > % svn cleanup path/to/external/dir >> >> Thanks. This is what I did. >> >> However, since I am not the one who set up the "externals" layout of our >> source code repository, it took me some time to understand the way a working >> copy containing external references is organized. >> >> And then again, I realized that my custom update and rebuild script fails at >> garbage collecting unreferenced pristines in sub-WCs included by "externals" >> defnitions. But that's another story... > > That's a good point actually, since presently 'svn cleanup' is required > to garbage-collect pristines, having it recurse to externals > automatically makes even more sense. > > Has anyone filed a bug report about this yet? If not I will.. > (so it's not forgotten, etc) There is http://subversion.tigris.org/issues/show_bug.cgi?id=2325 Maybe update its target milestone to be 1.8? Here is previous discussion, 6 months ago (dev@, 2011-10): http://subversion.markmail.org/thread/bogwwrvjxzwuuupf For reference, current thread: http://subversion.markmail.org/thread/koid6zy33s4zd3j3 Best regards, Konstantin Kolinko
Re: Implementations of an SVN/DAV aware proxy?
Tim Watts wrote: >[...] >We're changing SVN servers from mod-dav-svn to SCM-Manager - and one >minor fly in the ointment is the style of our old repo URI, which was of >the form: is there a reason you can't just use 'svn relocate' on the working copies? -- Lorenz
Re: 'svn cleanup' ignores externals
Thanks for the pointers, Konstantin. I've promoted the bug to 1.8-consider. (It's not a release blocker.) Konstantin Kolinko wrote on Tue, Apr 17, 2012 at 03:02:23 +0400: > 2012/4/16 Daniel Shahaf : > > Kuno Meyer wrote on Mon, Apr 16, 2012 at 16:51:29 +0200: > >> Daniel Shahaf wrote on Mon, 16 Apr 2012 14:48:45 +0300: > >> > Kuno Meyer wrote on Mon, Apr 16, 2012 at 10:31:33 +: > >> > > Is it by design that 'svn cleanup' ignores externals and that there is > >> > no way to > >> > > include them? > >> > > > >> > > >> > You should be able to include them by naming them explicitly: > >> > > >> > % svn cleanup path/to/external/dir > >> > >> Thanks. This is what I did. > >> > >> However, since I am not the one who set up the "externals" layout of our > >> source code repository, it took me some time to understand the way a > >> working copy containing external references is organized. > >> > >> And then again, I realized that my custom update and rebuild script fails > >> at garbage collecting unreferenced pristines in sub-WCs included by > >> "externals" defnitions. But that's another story... > > > > That's a good point actually, since presently 'svn cleanup' is required > > to garbage-collect pristines, having it recurse to externals > > automatically makes even more sense. > > > > Has anyone filed a bug report about this yet? If not I will.. > > (so it's not forgotten, etc) > > There is > http://subversion.tigris.org/issues/show_bug.cgi?id=2325 > > Maybe update its target milestone to be 1.8? > > Here is previous discussion, 6 months ago (dev@, 2011-10): > http://subversion.markmail.org/thread/bogwwrvjxzwuuupf > > For reference, current thread: > http://subversion.markmail.org/thread/koid6zy33s4zd3j3 > > Best regards, > Konstantin Kolinko