You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Evgeny Kotkov <ev...@visualsvn.com> on 2015/05/12 17:14:16 UTC
Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Subversion 1.9.0-rc1 changed the behavior of commands such as 'svn st'
in situations the wc.db workqueue is not empty, i.e., contains stale items.
These read-only commands no longer fail with SVN_ERR_WC_CLEANUP_REQUIRED:
svn co https://svn.apache.org/repos/asf/subversion/trunk .
sqlite3 .svn\wc.db "INSERT INTO work_queue (work) VALUES ('abc')"
binaries-1.8.13\svn st
svn: E155037: Previous operation has not finished; run 'cleanup' if it ...
binaries-1.9.0-rc1\svn st
(no output)
Non-readonly operations such as 'svn up' behave the same way in both versions
and fail with SVN_ERR_WC_CLEANUP_REQUIRED.
I am wondering whether this behavior is expected. As far as I realize, this is
a consequence of us fixing issue #4390, "parallel nested checkout not possible
with wc-ng" in r1501338 [1,2]. With this fix, the workqueue emptiness check
only occurs when the db is locked for writing, which is not the case of svn st.
>From my reading of how this works, if the workqueue contains unprocessed items,
the state of wc.db is, strictly speaking, unknown. Hence, operations on the
database could potentially yield incomplete or incorrect results.
Any thoughts on this?
Here is a quick proof-of-concept test for this behavior:
[[[
Index: subversion/tests/cmdline/wc_tests.py
===================================================================
--- subversion/tests/cmdline/wc_tests.py (revision 1678956)
+++ subversion/tests/cmdline/wc_tests.py (working copy)
@@ -360,7 +360,20 @@ def checkout_within_locked_wc(sbox):
[], "checkout", sbox.repo_url + '/A/B/E',
sbox.ospath("nested-wc"))
+def status_stale_workqueue(sbox):
+ """status with stale workqueue items"""
+ sbox.build(read_only = True)
+
+ sbox.simple_mkdir('X')
+ svntest.actions.lock_admin_dir(sbox.ospath(''), True, True)
+ svntest.actions.run_and_verify_svn(None,
+ "svn: E155037: Previous operation has "
+ "not finished; run 'cleanup' if it was "
+ "interrupted",
+ 'status', sbox.ospath('X'))
+
+
########################################################################
# Run the tests
@@ -384,6 +397,7 @@ test_list = [ None,
cleanup_unversioned_items_in_locked_wc,
cleanup_dir_external,
checkout_within_locked_wc,
+ status_stale_workqueue,
]
if __name__ == '__main__':
]]]
[1] http://subversion.tigris.org/issues/show_bug.cgi?id=4390
[2] https://svn.apache.org/r1501338
Regards,
Evgeny Kotkov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Evgeny Kotkov <ev...@visualsvn.com> writes:
> Indeed, I see that non-readonly operations are locking the necessary portions
> of the working copy and that 'svn st' reports it if the operation is aborted:
>
> svn-1.9 co https://svn.apache.org/repos/asf/subversion/trunk/ .
> svn-1.9 up -r {2015-05-01}
> ^C
> svn-1.9 st
To sum this up, I agree that the new behavior of 'svn st' makes sense for
users, assuming that libsvn_wc guarantees that a working copy is locked before
any write operations — and that's what I observed from a couple of quick tests.
Regards,
Evgeny Kotkov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
> Subversion still reports the working copy as ‘locked’ in this case, just
> like Subversion 1.0-1.6 reported this case when we still used 'loggy’
> operations. So there is still a quite visible status that tells you that
> there is something to be done.
> (Every directory shows as status write locked; not unmodified or something)
>
>
> The change in 1.7 was an implementation detailed that caused us to break
> operations that opened the database… But if your client opened it earlier
> (or while there were temporarily no workqueue items… as happens every time
> the workqueue is run), it could just open it… and use it during all future
> operations.
>
> Blocking out just initial db opens, but not blocking new operations when a
> db is open (e.g. when cached in svn_client_ctxt’s svn_wc__db_t instance) is
> in my opionion more inconsistent than the current behavior which does a
> check on obtaining the write lock.
Indeed, I see that non-readonly operations are locking the necessary portions
of the working copy and that 'svn st' reports it if the operation is aborted:
svn-1.9 co https://svn.apache.org/repos/asf/subversion/trunk/ .
svn-1.9 up -r {2015-05-01}
^C
svn-1.9 st
! L .
L build
L build\ac-macros
L build\generator
L build\generator\swig
L build\generator\templates
L build\generator\util
L build\hudson
L build\hudson\jobs
L build\hudson\jobs\subversion-1.6.x-solaris
L build\hudson\jobs\subversion-1.6.x-ubuntu
L build\hudson\jobs\subversion-doxygen
L build\hudson\jobs\subversion-javadoc
L build\hudson\jobs\subversion-trunk-solaris
L build\hudson\jobs\subversion-trunk-ubuntu
L build\win32
L contrib
L contrib\cgi
L contrib\client-side
L contrib\client-side\emacs
...
Just to clalify, the change got caught by our test suite, and I wanted to make
sure it's not something unexpected. Thanks again for shedding light on this.
Regards,
Evgeny Kotkov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Bert Huijben <be...@qqmail.nl>.
Subversion still reports the working copy as ‘locked’ in this case, just like Subversion 1.0-1.6 reported this case when we still used 'loggy’ operations. So there is still a quite visible status that tells you that there is something to be done.
(Every directory shows as status write locked; not unmodified or something)
The change in 1.7 was an implementation detailed that caused us to break operations that opened the database… But if your client opened it earlier (or while there were temporarily no workqueue items… as happens every time the workqueue is run), it could just open it… and use it during all future operations.
Blocking out just initial db opens, but not blocking new operations when a db is open (e.g. when cached in svn_client_ctxt’s svn_wc__db_t instance) is in my opionion more inconsistent than the current behavior which does a check on obtaining the write lock.
Bert
From: 'Evgeny Kotkov'
Sent: Wednesday, May 13, 2015 5:23 PM
To: Bert Huijben
Cc: Ivan Zhakov, dev@subversion.apache.org, Stefan Sperling
Bert Huijben <be...@qqmail.nl> writes:
> In Subversion 1.7 we queued database operations that contained further
> operations that would change the database further. This left the database in
> an inconsistent, mostly unsupported intermediate state, which would provide
> invalid results on certain operations. The cleanup was really required to
> make the database consistent again.
>
> All these problems were resolved for 1.8.0, but for 1.8 we didn't remove the
> restriction while we could have done that (stsp wrote that patch some time
> after 1.8).
>
> The interesting cases are things like recursive base-deletes (part of
> update processing). In 1.7 that operation is done as many recursive db
> transactions, which each schedule workqueue items. Since 1.8 there is a
> single db operation that schedules all workqueue operations, and can never
> leave the database with a half a base-delete.
Thank you for the explanation. It's good to know that the consistency of the
database with a non-empty workqueue is not a problem.
> When we moved this check I did another carefull check to see if all wq
> operatios were safe (and I checked the wq operations again short before
> branching 1.9). I don't see actual cases why we would really need to block
> out read-only operations any more.
However, there is the user experience part of the issue. Say, after an
aborted write, we are left with a database having a non-empty workqueue.
As per above, its integrity is not a suspect, but performing a read-only
operation such as 'svn st' could yield incomplete results due to these
unprocessed items in the workqueue.
As I see it, the problem is that if we no longer throw an error in this case,
we're potentially telling a bunch of lies to the end-user — or maybe to an
automated CI builder that relies on this output. Furthermore, there is no
way of finding it out, and this fact will only show up upon the next database
write, that may or may not happen at all. In 1.8.13 everything is simple —
if you try to do anything with a stale workqueue, you get an error and have
to run 'svn cleanup'.
I am not sure if there are other benefits in the current behavior, but I think
that if we have to choose between not fixing issue #4390 in 1.9.0-rc1 and
potentially misleading users during common read-only operations, the first
option is better.
Am I missing something crucial?
Regards,
Evgeny Kotkov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
> In Subversion 1.7 we queued database operations that contained further
> operations that would change the database further. This left the database in
> an inconsistent, mostly unsupported intermediate state, which would provide
> invalid results on certain operations. The cleanup was really required to
> make the database consistent again.
>
> All these problems were resolved for 1.8.0, but for 1.8 we didn't remove the
> restriction while we could have done that (stsp wrote that patch some time
> after 1.8).
>
> The interesting cases are things like recursive base-deletes (part of
> update processing). In 1.7 that operation is done as many recursive db
> transactions, which each schedule workqueue items. Since 1.8 there is a
> single db operation that schedules all workqueue operations, and can never
> leave the database with a half a base-delete.
Thank you for the explanation. It's good to know that the consistency of the
database with a non-empty workqueue is not a problem.
> When we moved this check I did another carefull check to see if all wq
> operatios were safe (and I checked the wq operations again short before
> branching 1.9). I don't see actual cases why we would really need to block
> out read-only operations any more.
However, there is the user experience part of the issue. Say, after an
aborted write, we are left with a database having a non-empty workqueue.
As per above, its integrity is not a suspect, but performing a read-only
operation such as 'svn st' could yield incomplete results due to these
unprocessed items in the workqueue.
As I see it, the problem is that if we no longer throw an error in this case,
we're potentially telling a bunch of lies to the end-user — or maybe to an
automated CI builder that relies on this output. Furthermore, there is no
way of finding it out, and this fact will only show up upon the next database
write, that may or may not happen at all. In 1.8.13 everything is simple —
if you try to do anything with a stale workqueue, you get an error and have
to run 'svn cleanup'.
I am not sure if there are other benefits in the current behavior, but I think
that if we have to choose between not fixing issue #4390 in 1.9.0-rc1 and
potentially misleading users during common read-only operations, the first
option is better.
Am I missing something crucial?
Regards,
Evgeny Kotkov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Bert Huijben <be...@qqmail.nl>.
In Subversion 1.7 we queued database operations that contained further operations that would change the database further. This left the database in an inconsistent, mostly unsupported intermediate state, which would provide invalid results on certain operations. The cleanup was really required to make the database consistent again.
All these problems were resolved for 1.8.0, but for 1.8 we didn't remove the restriction while we could have done that (stsp wrote that patch some time after 1.8).
The interesting cases are things like recursive base-deletes (part of update processing). In 1.7 that operation is done as many recursive db transactions, which each schedule workqueue items. Since 1.8 there is a single db operation that schedules all workqueue operations, and can never leave the database with a half a base-delete.
When we moved this check I did another carefull check to see if all wq operatios were safe (and I checked the wq operations again short before branching 1.9). I don't see actual cases why we would really need to block out read-only operations any more.
Some write operations certainly need higher guarantees, but that is what the write lock is for.
Bert
Sent from Surface
From: Ivan Zhakov
Sent: Tuesday, May 12, 2015 5:55 PM
To: 'Evgeny Kotkov', dev@subversion.apache.org, Stefan Sperling
On 12 May 2015 at 18:28, Stefan Sperling <st...@elego.de> wrote:
> On Tue, May 12, 2015 at 06:14:16PM +0300, Evgeny Kotkov wrote:
>> Subversion 1.9.0-rc1 changed the behavior of commands such as 'svn st'
>> in situations the wc.db workqueue is not empty, i.e., contains stale items.
>> These read-only commands no longer fail with SVN_ERR_WC_CLEANUP_REQUIRED:
>
>> Any thoughts on this?
>
> r1501338 was never backported to 1.8.x. So far, I hadn't heard a detailed
> explanation of what exactly was wrong -- it was simply .0rejected on grounds
> of being a "destructive change" without further details.
> I've since removed the backport nomination in r1665846.
>
Just to clarify: my concerns regarding r1501338 nomination was mostly
procedural: it was not regression nor critical bugfix. I didn't have
enough time to analyze the change the change itself.
--
Ivan Zhakov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in 1.9.x
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 12 May 2015 at 18:28, Stefan Sperling <st...@elego.de> wrote:
> On Tue, May 12, 2015 at 06:14:16PM +0300, Evgeny Kotkov wrote:
>> Subversion 1.9.0-rc1 changed the behavior of commands such as 'svn st'
>> in situations the wc.db workqueue is not empty, i.e., contains stale items.
>> These read-only commands no longer fail with SVN_ERR_WC_CLEANUP_REQUIRED:
>
>> Any thoughts on this?
>
> r1501338 was never backported to 1.8.x. So far, I hadn't heard a detailed
> explanation of what exactly was wrong -- it was simply rejected on grounds
> of being a "destructive change" without further details.
> I've since removed the backport nomination in r1665846.
>
Just to clarify: my concerns regarding r1501338 nomination was mostly
procedural: it was not regression nor critical bugfix. I didn't have
enough time to analyze the change the change itself.
--
Ivan Zhakov
Re: Executing 'svn st' with stale wc.db workqueue doesn't fail in
1.9.x
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 12, 2015 at 06:14:16PM +0300, Evgeny Kotkov wrote:
> Subversion 1.9.0-rc1 changed the behavior of commands such as 'svn st'
> in situations the wc.db workqueue is not empty, i.e., contains stale items.
> These read-only commands no longer fail with SVN_ERR_WC_CLEANUP_REQUIRED:
> Any thoughts on this?
r1501338 was never backported to 1.8.x. So far, I hadn't heard a detailed
explanation of what exactly was wrong -- it was simply rejected on grounds
of being a "destructive change" without further details.
I've since removed the backport nomination in r1665846.
You've now provided a good reason for reverting r1501338 on trunk and 1.9.x
and closing issue #4390 as WONTFIX.