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

Re: svn commit: r1604421 - in /subversion/trunk: subversion/libsvn_fs_fs/ subversion/tests/cmdline/ subversion/tests/libsvn_fs_fs/ tools/server-side/svnfsfs/

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Wed, 25 Jun 2014 18:57:23 +0400

On 21 June 2014 17:15, <stefan2_at_apache.org> wrote:
> Author: stefan2
> Date: Sat Jun 21 15:15:30 2014
> New Revision: 1604421
>
> URL: http://svn.apache.org/r1604421
> Log:
> Append index data to the rev data file instead of using separate files.
>
> This gives unpacked FSFS format 7 similar I/O characteristics and disk
> space usage as earlier formats. It also eliminates the need for retries
> after a potential background pack run because each file is now always
> consistent with itself (earlier, data or index files might already have
> been deleted while the respective other was still valid). Overall,
> most of this patch is removing code necessary to handle separate files.
>
> The new file format is simple:
>
> <rep data><l2p index><p2l index><footer><footer length>
>
> with the first three being identical what we had before. <footer length>
> is a single byte giving the length of the preceding footer, so it's
> easier to extract than the pre-f7 rev trailer and there is only one
> per pack / rev file.
>
[..]

>Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1604421&r1=1604420&r2=1604421&view=diff
>==============================================================================
>--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
>+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Jun 21 15:15:30 2014
>@@ -1449,15 +1449,35 @@ write_revision_zero(svn_fs_t *fs)
>+ SVN_ERR(svn_io_file_create_binary(path_revision_zero,
>+ "PLAIN\nEND\nENDREP\n"
>+ "id: 0.0.r0/2\n"
>+ "type: dir\n"
>+ "count: 0\n"
>+ "text: 0 3 4 4 "
>+ "2d2977d1c96f487abe4a1e202dd03b4e\n"
>+ "cpath: /\n"
>+ "\n\n"
>+
>+ /* L2P index */
>+ "\0\x80\x40" /* rev 0, 8k entries per page */
>+ "\1\1\1" /* 1 rev, 1 page, 1 page in 1st rev */
>+ "\6\4" /* page size: bytes, count */
>+ "\0\xd6\1\xb1\1\x21" /* phys offsets + 1 */
>+
>+ /* P2L index */
>+ "\0\x6b" /* start rev, rev file size */
>+ "\x80\x80\4\1\x1D" /* 64k pages, 1 page using 29 bytes */
>+ "\0" /* offset entry 0 page 1 */
>+ /* len, item & type, rev, checksum */
>+ "\x11\x34\0\xf5\xd6\x8c\x81\x06"
>+ "\x59\x09\0\xc8\xfc\xf6\x81\x04"
>+ "\1\x0d\0\x9d\x9e\xa9\x94\x0f"
>+ "\x95\xff\3\x1b\0\0" /* last entry fills up 64k page */
>+
>+ /* Footer */
>+ "107 121\7",
>+ 107 + 14 + 38 + 7 + 1, fs->pool));
This constant makes code unreadable, unmaintainable and very error prone.

> Modified: subversion/trunk/subversion/libsvn_fs_fs/rev_file.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/rev_file.c?rev=1604421&r1=1604420&r2=1604421&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/rev_file.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/rev_file.c Sat Jun 21 15:15:30 2014
> @@ -23,6 +23,7 @@
> #include "rev_file.h"
> #include "fs_fs.h"
> #include "index.h"
> +#include "low_level.h"
> #include "util.h"
>
> #include "../libsvn_fs/fs-loader.h"
> @@ -46,6 +47,10 @@ init_revision_file(svn_fs_fs__revision_f
> file->stream = NULL;
> file->p2l_stream = NULL;
> file->l2p_stream = NULL;
> + file->block_size = ffd->block_size;
> + file->l2p_offset = -1;
> + file->p2l_offset = -1;
> + file->footer_offset = -1;
> file->pool = pool;
> }
>
> @@ -127,14 +132,40 @@ svn_fs_fs__open_pack_or_rev_file(svn_fs_
> }
>
> svn_error_t *
> -svn_fs_fs__reopen_revision_file(svn_fs_fs__revision_file_t *file,
> - svn_fs_t *fs,
> - svn_revnum_t rev)
> +svn_fs_fs__auto_read_footer(svn_fs_fs__revision_file_t *file)
> {
> - if (file->file)
> - svn_fs_fs__close_revision_file(file);
> + if (file->l2p_offset == -1)
> + {
> + apr_off_t filesize = 0;
> + unsigned char footer_length;
> + svn_stringbuf_t *footer;
> +
> + /* Determine file size. */
> + SVN_ERR(svn_io_file_seek(file->file, APR_END, &filesize, file->pool));
> +
> + /* Read last byte (containing the length of the footer). */
> + SVN_ERR(svn_io_file_aligned_seek(file->file, file->block_size, NULL,
> + filesize - 1, file->pool));
> + SVN_ERR(svn_io_file_read_full2(file->file, &footer_length,
> + sizeof(footer_length), NULL, NULL,
> + file->pool));
> +
> + /* Read footer. */
> + footer = svn_stringbuf_create_ensure(footer_length, file->pool);
> + SVN_ERR(svn_io_file_aligned_seek(file->file, file->block_size, NULL,
> + filesize - 1 - footer_length,
> + file->pool));
> + SVN_ERR(svn_io_file_read_full2(file->file, footer->data, footer_length,
> + &footer->len, NULL, file->pool));
You're passing pointer to possible 32-bit value to function accepting
pointer to 64-bit value. This will lead unpredictable results.

> + footer->data[footer->len] = '\0';
> +
> + /* Extract index locations. */
> + SVN_ERR(svn_fs_fs__parse_footer(&file->l2p_offset, &file->p2l_offset,
> + footer, file->start_revision));
> + file->footer_offset = filesize - footer_length - 1;
> + }
>
> - return svn_error_trace(open_pack_or_rev_file(file, fs, rev, file->pool));
> + return SVN_NO_ERROR;
> }
>
> svn_error_t *
> @@ -165,9 +196,6 @@ svn_fs_fs__close_revision_file(svn_fs_fs
> if (file->file)
> SVN_ERR(svn_io_file_close(file->file, file->pool));
>
> - SVN_ERR(svn_fs_fs__packed_stream_close(file->l2p_stream));
> - SVN_ERR(svn_fs_fs__packed_stream_close(file->p2l_stream));
> -
> file->file = NULL;
> file->stream = NULL;
> file->l2p_stream = NULL;
>

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-06-25 16:58:11 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.