You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@hyrumwright.org> on 2009/09/30 21:08:33 UTC
[PATCH] Move .svn/lock files into the sdb
Greg, et. al.,
The attached patch is an attempt to move the .svn/lock files into a
separate table in .svn/wc.db. The reason to do this is so that we can
use the sdb to manage the locked status of a directory, particularly
as we start using svn_wc__entry_modify2(), which currently does *not*
do a write check on the access baton. Eventually, I plan to add a
member to pdh->wcroot which moves the in-memory state from the access
baton to wc_db.
Whether or not we'll need these locks in the long term is still an
open question (I hope we don't), but we still need them for the time
being, particularly as we move from access batons. Even if this table
eventually goes away, I think it better for wc_db to be managing this
state than the access baton, hence this patch.
This patch passes all tests locally for me. I'm not committing it
just yet because I want to make sure the approach is valid, and that
I've correctly plugged into the upgrade code in bumping the format to
f14.
Comments appreciated.
-Hyrum
[[[
Bump the wc_db format to 14, moving the physical locks files into an
sqlite table.
* subversion/tests/libsvn_wc/db-test.c
(data_loading_sql): Add the format 14 sql.
(test_upgrading_to_f13): Rename to...
(test_upgrading_to_f14): ...this, since we have to upgrade through
f13 to
get to f14, we don't lose any test coverage here.
(test_funcs): Update test name.
* subversion/tests/cmdline/upgrade_tests.py
(get_current_format): Update with the now-current f14.
* subversion/tests/cmdline/svntest/actions.py
(lock_admin_dir): Apply the lock into the sdb, rather than dropping
a lock
file.
* subversion/libsvn_wc/wc.h
(SVN_WC__VERSION): Bump to 14.
(SVN_WC__PHYSICAL_LOCK_FILE): New.
* subversion/libsvn_wc/lock.c
(create_lock): Remove.
(adm_access_alloc): Call the wc_db lock functions, rather than
dealing
a physical lock.
(close_single, svn_wc__adm_write_check): Same.
(svn_wc_locked): Create a context, and use it to call into wc_db to
inquire about locked status.
* subversion/libsvn_wc/wc-queries.sql
(STMT_INSERT_WC_LOCK, STMT_SELECT_WC_LOCK, STMT_DELETE_WC_LOCK): New.
* subversion/libsvn_wc/wc-metadata.sql:
Introduce new format 14 stuff, and keep the old format 14 stuff as
well.
* subversion/libsvn_wc/wc_db.c
(upgrade_sql): Add the format 14 upgrade sql.
(svn_wc__db_wclock_set, svn_wc__db_wclocked,
svn_wc__db_wclock_remove): New.
* subversion/libsvn_wc/wc_db.h
(svn_wc__db_wclock_set, svn_wc__db_wclocked,
svn_wc__db_wclock_remove): New.
* subversion/libsvn_wc/upgrade.c
(create_physical_lock): New.
(upgrade_to_wcng): Instead of using access batons to steal the
write lock,
just do it outselves.
(migrate_locks): Move existing locks into the wc.db.
(bump15_baton): Renamed from this...
(bump14_baton): ...to this.
(bump_database_to_15): Renamed from this...
(bump_database_to_14): ...to this.
(bump_to_15): Renamed from this...
(bump_to_14): ...to this.
(svn_wc__upgrade_sdb): Add a section for bumping to the new format
14.
]]]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402286
Re: [PATCH] Move .svn/lock files into the sdb
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
A varient of this patch was committed in r39740.
On Sep 30, 2009, at 5:08 PM, Hyrum K. Wright wrote:
> Greg, et. al.,
> The attached patch is an attempt to move the .svn/lock files into a
> separate table in .svn/wc.db. The reason to do this is so that we can
> use the sdb to manage the locked status of a directory, particularly
> as we start using svn_wc__entry_modify2(), which currently does *not*
> do a write check on the access baton. Eventually, I plan to add a
> member to pdh->wcroot which moves the in-memory state from the access
> baton to wc_db.
>
> Whether or not we'll need these locks in the long term is still an
> open question (I hope we don't), but we still need them for the time
> being, particularly as we move from access batons. Even if this table
> eventually goes away, I think it better for wc_db to be managing this
> state than the access baton, hence this patch.
>
> This patch passes all tests locally for me. I'm not committing it
> just yet because I want to make sure the approach is valid, and that
> I've correctly plugged into the upgrade code in bumping the format to
> f14.
>
> Comments appreciated.
>
> -Hyrum
>
> [[[
> Bump the wc_db format to 14, moving the physical locks files into an
> sqlite table.
>
> * subversion/tests/libsvn_wc/db-test.c
> (data_loading_sql): Add the format 14 sql.
> (test_upgrading_to_f13): Rename to...
> (test_upgrading_to_f14): ...this, since we have to upgrade through
> f13 to
> get to f14, we don't lose any test coverage here.
> (test_funcs): Update test name.
>
> * subversion/tests/cmdline/upgrade_tests.py
> (get_current_format): Update with the now-current f14.
>
> * subversion/tests/cmdline/svntest/actions.py
> (lock_admin_dir): Apply the lock into the sdb, rather than dropping
> a lock
> file.
>
> * subversion/libsvn_wc/wc.h
> (SVN_WC__VERSION): Bump to 14.
> (SVN_WC__PHYSICAL_LOCK_FILE): New.
>
> * subversion/libsvn_wc/lock.c
> (create_lock): Remove.
> (adm_access_alloc): Call the wc_db lock functions, rather than
> dealing
> a physical lock.
> (close_single, svn_wc__adm_write_check): Same.
> (svn_wc_locked): Create a context, and use it to call into wc_db to
> inquire about locked status.
>
> * subversion/libsvn_wc/wc-queries.sql
> (STMT_INSERT_WC_LOCK, STMT_SELECT_WC_LOCK, STMT_DELETE_WC_LOCK):
> New.
>
> * subversion/libsvn_wc/wc-metadata.sql:
> Introduce new format 14 stuff, and keep the old format 14 stuff as
> well.
>
> * subversion/libsvn_wc/wc_db.c
> (upgrade_sql): Add the format 14 upgrade sql.
> (svn_wc__db_wclock_set, svn_wc__db_wclocked,
> svn_wc__db_wclock_remove): New.
>
> * subversion/libsvn_wc/wc_db.h
> (svn_wc__db_wclock_set, svn_wc__db_wclocked,
> svn_wc__db_wclock_remove): New.
>
> * subversion/libsvn_wc/upgrade.c
> (create_physical_lock): New.
> (upgrade_to_wcng): Instead of using access batons to steal the
> write lock,
> just do it outselves.
> (migrate_locks): Move existing locks into the wc.db.
> (bump15_baton): Renamed from this...
> (bump14_baton): ...to this.
> (bump_database_to_15): Renamed from this...
> (bump_database_to_14): ...to this.
> (bump_to_15): Renamed from this...
> (bump_to_14): ...to this.
> (svn_wc__upgrade_sdb): Add a section for bumping to the new format
> 14.
>
> ]]]
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?
> dsForumId=462&dsMessageId=2402286<lock_files.patch>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402629
Re: [PATCH] Move .svn/lock files into the sdb
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Oct 1, 2009, at 5:50 AM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum@hyrumwright.org]
>> Sent: woensdag 30 september 2009 23:09
>> To: Greg Stein
>> Cc: dev@subversion.tigris.org
>> Subject: [PATCH] Move .svn/lock files into the sdb
>>
>> Greg, et. al.,
>> The attached patch is an attempt to move the .svn/lock files into a
>> separate table in .svn/wc.db. The reason to do this is so that we
>> can
>> use the sdb to manage the locked status of a directory, particularly
>> as we start using svn_wc__entry_modify2(), which currently does *not*
>> do a write check on the access baton. Eventually, I plan to add a
>> member to pdh->wcroot which moves the in-memory state from the access
>> baton to wc_db.
>
> This patch doesn't solve the issues with svn_wc__entry_modify2().
Correct. This is planned for a followup patch which will add a LOCKED
member to the wcroot. This member will convey similar information as
svn_wc_adm_access_t.type, and help answer the question "do I own the
lock?" By putting this in a wc_db struct, we won't have a resurrect
or create access batons to answer this question.
The patch to move the lock "files" into the sdb is seen as a precursor
to the above, since it means that we can then answer the question
"does a lock exist on this path?" without having to go back into lock.c.
I (believe I) understand the corruption issues you describe below, and
that the above plan solves them. Are there any concerns about the
implementation of this patch? (I've since updated it to include the
additional columns for format 14 added in r39725.)
-Hyrum
> This patch answers the question: "Is there a lock on X?" while we
> have to
> check "Am I the owner of an existing lock on X?" or simpler "Do I
> own a lock
> that allows me to update X?"
>
> This ownership question is the only important bit when updating a
> working
> copy.
>
>
> We are not interested in whether another client has it locked in
> this case..
> We simply can't do an exclusive update without being the lock owner.
>
>
> If an exclusive update in Process 1 does a recursive switch on the
> entire
> working copy from one branch to another, you don't want another svn
> (Process
> 2) performing a move of a file below that same root at the same time..
>
> The "Is there a lock?" will just return TRUE... so the revert might be
> perfectly valid after this patch, but it could very well corrupt your
> working copy.
>
>
> A "Am I the lock owner?" check will return FALSE, so the revert will
> not
> happen.
>
>
>
> Your recent changes replaced a few of the "Am I the lock owner?"
> questions
> with "Is there a lock?" and this new api makes it easier to do the
> WRONG
> THING.
>
>
> Moving the lock files in the DB is a completely different thing,
> that is
> unrelated to this question, and something that should+could happen
> independently
>
>
>
> If we want multiple locks on a working copy later we could just
> write the
> locked roots in a db table and keep a record on the locks we own.
> Every path
> below a locked path is locked by the wc_db. (Very cheap to check as we
> usually just lock one path.. so one dirent_is_ancestor() check is
> enough).
>
> (A proper transaction around inserting the lock can handle parent
> and child
> locks without the extreme penalty we have in WC-1.0)
>
>
> We could write a process identifier + workstation name or something
> with the
> lock in the db to specify who owns it for better error messages, but
> the
> real knowledge is in owning the lock itself... which is currently only
> handled via the flag in the access batons. And you can't just put
> that
> knowledge in the database and think you're done with that.
>
>
> Bert
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402486
RE: [PATCH] Move .svn/lock files into the sdb
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Hyrum K. Wright [mailto:hyrum@hyrumwright.org]
> Sent: woensdag 30 september 2009 23:09
> To: Greg Stein
> Cc: dev@subversion.tigris.org
> Subject: [PATCH] Move .svn/lock files into the sdb
>
> Greg, et. al.,
> The attached patch is an attempt to move the .svn/lock files into a
> separate table in .svn/wc.db. The reason to do this is so that we can
> use the sdb to manage the locked status of a directory, particularly
> as we start using svn_wc__entry_modify2(), which currently does *not*
> do a write check on the access baton. Eventually, I plan to add a
> member to pdh->wcroot which moves the in-memory state from the access
> baton to wc_db.
This patch doesn't solve the issues with svn_wc__entry_modify2().
This patch answers the question: "Is there a lock on X?" while we have to
check "Am I the owner of an existing lock on X?" or simpler "Do I own a lock
that allows me to update X?"
This ownership question is the only important bit when updating a working
copy.
We are not interested in whether another client has it locked in this case..
We simply can't do an exclusive update without being the lock owner.
If an exclusive update in Process 1 does a recursive switch on the entire
working copy from one branch to another, you don't want another svn (Process
2) performing a move of a file below that same root at the same time..
The "Is there a lock?" will just return TRUE... so the revert might be
perfectly valid after this patch, but it could very well corrupt your
working copy.
A "Am I the lock owner?" check will return FALSE, so the revert will not
happen.
Your recent changes replaced a few of the "Am I the lock owner?" questions
with "Is there a lock?" and this new api makes it easier to do the WRONG
THING.
Moving the lock files in the DB is a completely different thing, that is
unrelated to this question, and something that should+could happen
independently
If we want multiple locks on a working copy later we could just write the
locked roots in a db table and keep a record on the locks we own. Every path
below a locked path is locked by the wc_db. (Very cheap to check as we
usually just lock one path.. so one dirent_is_ancestor() check is enough).
(A proper transaction around inserting the lock can handle parent and child
locks without the extreme penalty we have in WC-1.0)
We could write a process identifier + workstation name or something with the
lock in the db to specify who owns it for better error messages, but the
real knowledge is in owning the lock itself... which is currently only
handled via the flag in the access batons. And you can't just put that
knowledge in the database and think you're done with that.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2402423