You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2008/10/28 18:00:04 UTC
Re: svn commit: r33935 - in trunk/subversion/libsvn_fs_base: . bdb notes
Is there a lot of code duplication around those next-key columns and
generation? Could you write a utility function that manages all of
them in the same way? The initialization, the fetching and increment,
etc? "Here is a table. Do your thing."
On Tue, Oct 28, 2008 at 8:24 AM, <cm...@tigris.org> wrote:
> Author: cmpilato
> Date: Tue Oct 28 08:24:50 2008
> New Revision: 33935
>
> Log:
> Lose the FSGUID stuff, and just give the `checksum-reps' table its own
> private uniquifier.
>
> * subversion/libsvn_fs_base/notes/structure
> Note changes to the schema, namely that the FSGUID stuff has been
> removed, and that the `checksum-reps' table now carries a unique key
> tracker (like so many of the other tables do, too).
>
> * subversion/libsvn_fs_base/fsguid.h,
> * subversion/libsvn_fs_base/fsguid.c
> Removed as unused.
>
> * subversion/libsvn_fs_base/bdb/checksum-reps-table.h
> (svn_fs_bdb__reserve_rep_reuse_id): New function.
>
> * subversion/libsvn_fs_base/bdb/checksum-reps-table.c
> (svn_fs_bdb__open_checksum_reps_table): If creating the
> `checksum-reps' table, don't forget to initialize the 'next-key' row.
> (svn_fs_bdb__reserve_rep_reuse_id): New function.
>
> * subversion/libsvn_fs_base/dag.c
> (svn_fs_base__dag_finalize_edits): Use svn_fs_bdb__reserve_rep_reuse_id
> instead of svn_fs_base__reserve_fsguid. Also, avoid an unnecessary
> format check.
>
> Deleted:
> trunk/subversion/libsvn_fs_base/fsguid.c
> trunk/subversion/libsvn_fs_base/fsguid.h
> Modified:
> trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c
> trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.h
> trunk/subversion/libsvn_fs_base/dag.c
> trunk/subversion/libsvn_fs_base/notes/structure
>
> Modified: trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c?pathrev=33935&r1=33934&r2=33935
> ==============================================================================
> --- trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c Tue Oct 28 08:01:50 2008 (r33934)
> +++ trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c Tue Oct 28 08:24:50 2008 (r33935)
> @@ -20,6 +20,7 @@
> #include "bdb_compat.h"
> #include "../fs.h"
> #include "../err.h"
> +#include "../key-gen.h"
> #include "dbt.h"
> #include "../trail.h"
> #include "bdb-err.h"
> @@ -49,6 +50,15 @@ int svn_fs_bdb__open_checksum_reps_table
> BDB_ERR(checksum_reps->close(checksum_reps, 0));
> return svn_fs_bdb__open_checksum_reps_table(checksum_reps_p, env, TRUE);
> }
> +
> + /* Create the initial `next-key' table entry. */
> + if (create)
> + {
> + DBT key, value;
> + BDB_ERR(checksum_reps->put(checksum_reps, 0,
> + svn_fs_base__str_to_dbt(&key, NEXT_KEY_KEY),
> + svn_fs_base__str_to_dbt(&value, "0"), 0));
> + }
>
> BDB_ERR(error);
>
> @@ -151,3 +161,43 @@ svn_error_t *svn_fs_bdb__delete_checksum
> trail->db_txn, &key, 0)));
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *svn_fs_bdb__reserve_rep_reuse_id(const char **id_p,
> + svn_fs_t *fs,
> + trail_t *trail,
> + apr_pool_t *pool)
> +{
> + base_fs_data_t *bfd = fs->fsap_data;
> + DBT query, result;
> + apr_size_t len;
> + char next_key[MAX_KEY_SIZE];
> + int db_err;
> +
> + svn_fs_base__str_to_dbt(&query, NEXT_KEY_KEY);
> +
> + /* Get the current value associated with the `next-key' key in the
> + `checksum-reps' table. */
> + svn_fs_base__trail_debug(trail, "checksum-reps", "get");
> + SVN_ERR(BDB_WRAP(fs, _("allocating new representation reuse ID "
> + "(getting 'next-key')"),
> + bfd->checksum_reps->get(bfd->checksum_reps, trail->db_txn,
> + &query,
> + svn_fs_base__result_dbt(&result),
> + 0)));
> + svn_fs_base__track_dbt(&result, pool);
> +
> + /* Set our return value. */
> + *id_p = apr_pstrmemdup(pool, result.data, result.size);
> +
> + /* Bump to future key. */
> + len = result.size;
> + svn_fs_base__next_key(result.data, &len, next_key);
> + svn_fs_base__trail_debug(trail, "checksum_reps", "put");
> + db_err = bfd->checksum_reps->put(bfd->checksum_reps, trail->db_txn,
> + svn_fs_base__str_to_dbt(&query,
> + NEXT_KEY_KEY),
> + svn_fs_base__str_to_dbt(&result, next_key),
> + 0);
> +
> + return BDB_WRAP(fs, _("bumping next copy key"), db_err);
> +}
>
> Modified: trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.h?pathrev=33935&r1=33934&r2=33935
> ==============================================================================
> --- trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.h Tue Oct 28 08:01:50 2008 (r33934)
> +++ trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.h Tue Oct 28 08:24:50 2008 (r33935)
> @@ -68,6 +68,15 @@ svn_error_t *svn_fs_bdb__delete_checksum
> trail_t *trail,
> apr_pool_t *pool);
>
> +/* Reserve a unique reuse ID in the `checksum-reps' table in FS for a
> + new instance of a re-used representation as part of TRAIL. Return
> + the slot's id in *REUSE_ID_P, allocated in POOL. */
> +svn_error_t *svn_fs_bdb__reserve_rep_reuse_id(const char **reuse_id_p,
> + svn_fs_t *fs,
> + trail_t *trail,
> + apr_pool_t *pool);
> +
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> Modified: trunk/subversion/libsvn_fs_base/dag.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/dag.c?pathrev=33935&r1=33934&r2=33935
> ==============================================================================
> --- trunk/subversion/libsvn_fs_base/dag.c Tue Oct 28 08:01:50 2008 (r33934)
> +++ trunk/subversion/libsvn_fs_base/dag.c Tue Oct 28 08:24:50 2008 (r33935)
> @@ -33,7 +33,6 @@
> #include "reps-strings.h"
> #include "revs-txns.h"
> #include "id.h"
> -#include "fsguid.h"
>
> #include "util/fs_skels.h"
>
> @@ -1289,8 +1288,8 @@ svn_fs_base__dag_finalize_edits(dag_node
> if (! err)
> {
> useless_data_key = noderev->edit_key;
> - err = svn_fs_base__reserve_fsguid(trail->fs, &data_key_uniquifier,
> - trail, pool);
> + err = svn_fs_bdb__reserve_rep_reuse_id(&data_key_uniquifier,
> + trail->fs, trail, pool);
> }
> else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
> {
> @@ -1317,9 +1316,9 @@ svn_fs_base__dag_finalize_edits(dag_node
> SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, old_data_key, txn_id,
> trail, pool));
>
> - /* Throw away our edit -- there was an existing representation whose
> - contents matched our goal. */
> - if (useless_data_key && (bfd->format >= SVN_FS_BASE__MIN_REP_SHARING_FORMAT))
> + /* If we've got a discardable rep (probably because we ended us
> + re-using a preexisting one). Throw out the discardable rep. */
> + if (useless_data_key)
> SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, useless_data_key,
> txn_id, trail, pool));
>
>
> Deleted: trunk/subversion/libsvn_fs_base/fsguid.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/fsguid.c?pathrev=33934
>
> Deleted: trunk/subversion/libsvn_fs_base/fsguid.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/fsguid.h?pathrev=33934
>
> Modified: trunk/subversion/libsvn_fs_base/notes/structure
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/notes/structure?pathrev=33935&r1=33934&r2=33935
> ==============================================================================
> --- trunk/subversion/libsvn_fs_base/notes/structure Tue Oct 28 08:01:50 2008 (r33934)
> +++ trunk/subversion/libsvn_fs_base/notes/structure Tue Oct 28 08:24:50 2008 (r33935)
> @@ -863,7 +863,7 @@ Berkeley DB tables
> "locks" : btree(TOKEN -> LOCK)
> "lock-tokens" : btree(PATH -> TOKEN)
> "node-origins" : btree(NODE-ID -> ID)
> - "checksum-reps" : btree(SHA1SUM -> REP)
> + "checksum-reps" : btree(SHA1SUM -> REP, "next-key" -> number-36)
> "miscellaneous" : btree(STRING -> STRING)
>
> Syntactic elements
> @@ -1067,7 +1067,3 @@ introduced.
> 4 forward-delta-rev Youngest revision in the repository as of
> the moment when it was upgraded to support
> forward deltas.
> -
> - 4 next-fsguid Next filesystem-global unique identifier
> - available for use.
> -
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33935 - in trunk/subversion/libsvn_fs_base: . bdb
notes
Posted by "C. Michael Pilato" <cm...@collab.net>.
Greg Stein wrote:
> Is there a lot of code duplication around those next-key columns and
> generation? Could you write a utility function that manages all of
> them in the same way? The initialization, the fetching and increment,
> etc? "Here is a table. Do your thing."
I looked into this. There is some duplication, yes.
Certainly the creation of the initial `next-key' row looks the same across
the various tables. But there's not much to be gained by trading one
function call (DB->put) for another.
As for the more complex logic of fetching the next-key value and bumping it
to a new value, different tables do this different ways. Some just
get-then-bump as an atomic operation. Some get-then-do-something-then-bump
(to avoid burning up keys, perhaps). And a naive abstraction for those bits
that are identical in behavior would either need to have generic error
messages that don't reveal the table being operated on (bad), or dynamically
allocated error strings which may never be used (not ideal), or be passed in
a collection of static error strings for possible use (ugly). So, again,
I'm not sure the abstraction is really worth it at this stage.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand