Yes. It's actually a Bad Thing to be in the middle of one transaction
(via entries.c) and attempt to do an insert (in this case a lock via
wc_db). At this point, transactioning within entries.c is at best a
premature optimization, and possibly unneeded.
-Hyrum
On Mar 15, 2009, at 5:27 PM, Greg Stein wrote:
> For the entries, we probably do want a big transaction. I assume the
> de-txn is because you're going to defer integrity management to wc_db?
> (and for now just assume that midway failure corrupts?)
>
> On Fri, Mar 13, 2009 at 22:37, Hyrum K. Wright
> <hyrum_at_hyrumwright.org> wrote:
>> Author: hwright
>> Date: Fri Mar 13 14:37:35 2009
>> New Revision: 36535
>>
>> Log:
>> More de-transaction-ifing in entries.c, this time with
>> write_entries().
>>
>> * subversion/libsvn_wc/entries.c
>> (entries_write_txn_baton): Remove.
>> (entries_write_body): Reorder parameters, and don't use the baton
>> for them.
>> (svn_wc__entries_write): Call the writing function directly,
>> instead of
>> wrapping it with a transaction.
>>
>> Modified:
>> trunk/subversion/libsvn_wc/entries.c
>>
>> Modified: trunk/subversion/libsvn_wc/entries.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/entries.c?pathrev=36535&r1=36534&r2=36535
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- trunk/subversion/libsvn_wc/entries.c Fri Mar 13 14:24:37
>> 2009 (r36534)
>> +++ trunk/subversion/libsvn_wc/entries.c Fri Mar 13 14:37:35
>> 2009 (r36535)
>> @@ -1912,40 +1912,33 @@ write_entry(svn_sqlite__db_t *wc_db,
>> return SVN_NO_ERROR;
>> }
>>
>> -/* Baton for use with entries_write_body(). */
>> -struct entries_write_txn_baton
>> -{
>> - apr_hash_t *entries;
>> - const svn_wc_entry_t *this_dir;
>> - apr_pool_t *scratch_pool;
>> -};
>> -
>> /* Actually do the sqlite work within a transaction.
>> This implements svn_sqlite__transaction_callback_t */
>> static svn_error_t *
>> -entries_write_body(void *baton,
>> - svn_sqlite__db_t *wc_db)
>> +entries_write_body(svn_sqlite__db_t *wc_db,
>> + apr_hash_t *entries,
>> + const svn_wc_entry_t *this_dir,
>> + apr_pool_t *scratch_pool)
>> {
>> svn_sqlite__stmt_t *stmt;
>> svn_boolean_t have_row;
>> apr_hash_index_t *hi;
>> - struct entries_write_txn_baton *ewtb = baton;
>> - apr_pool_t *iterpool = svn_pool_create(ewtb->scratch_pool);
>> + apr_pool_t *iterpool = svn_pool_create(scratch_pool);
>> const char *repos_root;
>> apr_int64_t repos_id;
>> apr_int64_t wc_id;
>>
>> /* Get the repos ID. */
>> - if (ewtb->this_dir->uuid != NULL)
>> + if (this_dir->uuid != NULL)
>> {
>> SVN_ERR(svn_sqlite__get_statement(&stmt, wc_db,
>> STMT_SELECT_REPOSITORY));
>> - SVN_ERR(svn_sqlite__bindf(stmt, "s", ewtb->this_dir->repos));
>> + SVN_ERR(svn_sqlite__bindf(stmt, "s", this_dir->repos));
>> SVN_ERR(svn_sqlite__step(&have_row, stmt));
>>
>> if (have_row)
>> {
>> repos_id = svn_sqlite__column_int(stmt, 0);
>> - repos_root = svn_sqlite__column_text(stmt, 1, ewtb-
>> >scratch_pool);
>> + repos_root = svn_sqlite__column_text(stmt, 1,
>> scratch_pool);
>> }
>> else
>> {
>> @@ -1961,10 +1954,10 @@ entries_write_body(void *baton,
>> ### any members? */
>> SVN_ERR(svn_sqlite__get_statement(&stmt, wc_db,
>> STMT_INSERT_REPOSITORY));
>> - SVN_ERR(svn_sqlite__bindf(stmt, "ss", ewtb->this_dir-
>> >repos,
>> - ewtb->this_dir->uuid));
>> + SVN_ERR(svn_sqlite__bindf(stmt, "ss", this_dir->repos,
>> + this_dir->uuid));
>> SVN_ERR(svn_sqlite__insert(&repos_id, stmt));
>> - repos_root = ewtb->this_dir->repos;
>> + repos_root = this_dir->repos;
>> }
>>
>> SVN_ERR(svn_sqlite__reset(stmt));
>> @@ -1988,11 +1981,11 @@ entries_write_body(void *baton,
>>
>> /* Write out "this dir" */
>> SVN_ERR(fetch_wc_id(&wc_id, wc_db));
>> - SVN_ERR(write_entry(wc_db, wc_id, repos_id, repos_root, ewtb-
>> >this_dir,
>> - SVN_WC_ENTRY_THIS_DIR, ewtb->this_dir,
>> - ewtb->scratch_pool));
>> + SVN_ERR(write_entry(wc_db, wc_id, repos_id, repos_root, this_dir,
>> + SVN_WC_ENTRY_THIS_DIR, this_dir,
>> + scratch_pool));
>>
>> - for (hi = apr_hash_first(ewtb->scratch_pool, ewtb->entries); hi;
>> + for (hi = apr_hash_first(scratch_pool, entries); hi;
>> hi = apr_hash_next(hi))
>> {
>> const void *key;
>> @@ -2011,7 +2004,7 @@ entries_write_body(void *baton,
>>
>> /* Write the entry. */
>> SVN_ERR(write_entry(wc_db, wc_id, repos_id, repos_root,
>> - this_entry, key, ewtb->this_dir,
>> iterpool));
>> + this_entry, key, this_dir, iterpool));
>> }
>>
>> svn_pool_destroy(iterpool);
>> @@ -2026,7 +2019,6 @@ svn_wc__entries_write(apr_hash_t *entrie
>> {
>> svn_sqlite__db_t *wc_db;
>> const svn_wc_entry_t *this_dir;
>> - struct entries_write_txn_baton ewtb;
>> apr_pool_t *scratch_pool = svn_pool_create(pool);
>>
>> if (svn_wc__adm_wc_format(adm_access) < SVN_WC__WC_NG_VERSION)
>> @@ -2053,11 +2045,8 @@ svn_wc__entries_write(apr_hash_t *entrie
>> SVN_WC__VERSION_EXPERIMENTAL, upgrade_sql,
>> scratch_pool, scratch_pool));
>>
>> - /* Do the work in a transaction. */
>> - ewtb.entries = entries;
>> - ewtb.this_dir = this_dir;
>> - ewtb.scratch_pool = scratch_pool;
>> - SVN_ERR(svn_sqlite__with_transaction(wc_db, entries_write_body,
>> &ewtb));
>> + /* Write the entries. */
>> + SVN_ERR(entries_write_body(wc_db, entries, this_dir,
>> scratch_pool));
>>
>> svn_wc__adm_access_set_entries(adm_access, TRUE, entries);
>> svn_wc__adm_access_set_entries(adm_access, FALSE, NULL);
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1318776
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1329466
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1330565
Received on 2009-03-16 03:38:27 CET