[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r33819 - branches/fsfs-pack/subversion/libsvn_fs_fs

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Tue, 21 Oct 2008 20:48:47 -0500

Greg Stein wrote:
> On Tue, Oct 21, 2008 at 9:22 AM, <hwright_at_tigris.org> wrote:
>> ...
>> @@ -6552,12 +6549,10 @@ pack_shard(const char *revs_dir,
>> SVN_ERR(svn_io_file_open(&pack_file, pack_file_path,
>> APR_WRITE | APR_CREATE | APR_BUFFERED,
>> APR_OS_DEFAULT, pool));
>> - SVN_ERR(svn_io_open_unique_file2(&manifest_file, &tmp_file_path,
>> - manifest_path, ".manifest",
>> - svn_io_file_del_on_pool_cleanup, pool));
>> + SVN_ERR(svn_stream_open_unique(&pb.manifest_stream, &tmp_file_path, revs_dir,
>> + svn_io_file_del_on_pool_cleanup, pool, pool));
>> pb.next_offset = 0;
>> pb.pack_stream = svn_stream_from_aprfile2(pack_file, FALSE, pool);
>
> Do you need pack_file, or can you just svn_stream_open_writable() on
> the pack_file_path?

Ah, good point. See r33830.

> (and/or as David suggested, do the initial packing on a temp file and
> move into place)

I'll have to go back through my logs of that conversation, but I don't really
understand what that'd buy us. The usual reason to create the temp file is for
atomicity, which we really don't need here because the atomic operation is to
remove the shard directory. (There's probably some much more obvious
explanation which I'm missing; feel free to enlighten me.)

-Hyrum

Received on 2008-10-22 03:49:13 CEST

This is an archived mail posted to the Subversion Dev mailing list.