You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2003/10/30 02:18:35 UTC
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc
tests/clients/cmdline
philip@tigris.org writes:
> Author: philip
> Date: Wed Oct 29 19:38:13 2003
> New Revision: 7565
> Log:
> Add a mechanism that "repairs" the text-time and prop-time in the
> entries file if they differ from the time of the working copy files
> even though there are no text/property differences. This addresses
> issue 791.
A "broken" text-time can be obtained by editing a file, saving it,
then undoing the edit in the editor and saving it again. A "broken"
prop-time can be obtained by using "svn propset" twice, first to
modify a property and again to restore the original value. In the
past we had bugs that caused broken timestamps during updates. If you
have a working copy with lots of "broken" timestamps then operations
that use them, like "svn status", will take longer than necessary.
The new "repair" code runs automatically whenever Subversion checks
for changes while holding a write lock. Ideally one would be able to
trigger the repair of an entire working copy, at present I know of two
ways to ways to do it:
1) run "svn revert --recursive"
2) run "svn commit"
Neither of these is ideal. Using 1) has the obvious disadvantage that
it destroys any local modifications. Using 2) doesn't require one to
complete the commit, quitting the log message editor without saving
and aborting the commit will do the job, but it's hardly the interface
one would choose.
Do we need a more intuitive interface this repair? There are a number
of things we could do: 1) add a new option to "svn status" to cause it
to take a write lock, 2) make "svn cleanup" run the check for changes
on every entry, 3) add a new "svn repair" command, etc. I'm not
overly keen on any of those, "svn cleanup" is the one I dislike least,
so I'd choose to leave things as they are. Thoughts?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc tests/clients/cmdline
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Oct 29, 2003, at 11:05 PM, Ben Collins-Sussman wrote:
> On Wed, 2003-10-29 at 20:18, Philip Martin wrote:
>
>> Do we need a more intuitive interface this repair? There are a number
>> of things we could do: 1) add a new option to "svn status" to cause it
>> to take a write lock,
>
> Is it possible to make 'svn status' grab write-locks instead of
> read-locks? Is that a bad thing?
Do you mean by default, or optionally? Off the top of my head doing so
would mean you couldn't run 'svn status' while performing another
action that locks the WC, which would be kind of annoying...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc
tests/clients/cmdline
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> Is it possible to make 'svn status' grab write-locks instead of
> read-locks?
"svn status --write-lock" could take write locks rather than read locks.
> Is that a bad thing?
As the default behaviour for "svn status" it would be terrible.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc
tests/clients/cmdline
Posted by Ben Collins-Sussman <su...@collab.net>.
On Wed, 2003-10-29 at 20:18, Philip Martin wrote:
> Do we need a more intuitive interface this repair? There are a number
> of things we could do: 1) add a new option to "svn status" to cause it
> to take a write lock,
Is it possible to make 'svn status' grab write-locks instead of
read-locks? Is that a bad thing?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> > A read-only working copy need not be a fatal error.
>
> The argument is not that svn st would fail if it can't get a write
> lock. The argument is that svn st would cause another, legitimately
> read/write operation (like svn update) to fail because it has a write
> lock.
Thanks, that's a very useful clarification.
If working copy timestamps are kept incrementally up-to-date as a
matter of course, then these locks will be very rare, so it's not a
big deal I think.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-10-31 at 08:54, kfogel@collab.net wrote:
> It doesn't break Subversion...
> A read-only working copy need not be a fatal error.
The argument is not that svn st would fail if it can't get a write
lock. The argument is that svn st would cause another, legitimately
read/write operation (like svn update) to fail because it has a write
lock.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Lübbe Onken <l....@rac.de>.
kfogel@collab.net wrote:
> Sorry, I assumed the phrase "break Subversion" meant technical
> breakage, not just a feeling of inappropriateness among the
> developers.
No problem, next time I'll quote 'break' like this: (-:break:-) to make
my point clear. ;-)
-Lübbe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Lübbe Onken <l....@rac.de> writes:
> Good ole Vinyl-Roms ;-) Broken CDs just don't have the same beat.
Heh, someday that metaphor won't work anymore, you're right...
> > (Sorry to sound like a broken record, but this misunderstanding
> > appears to pervade this thread.)
> >
> > A read-only working copy need not be a fatal error. This optimization
> > can be opportunistic; there is no correctness consequence if it
> > doesn't happen. (And if the working copy is read-only, any other
> > solution is going to fail too, even 'svn cleanup').
>
> This is all well understood. Didn't I write 'read-only operation' and
> not 'read-only WC'?
Ah, good!
Sorry, I assumed the phrase "break Subversion" meant technical
breakage, not just a feeling of inappropriateness among the
developers.
> I was just suggesting an approach that might also be acceptible :-).
Gotcha. Thanks for following up.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Lübbe Onken <l....@rac.de>.
kfogel@collab.net wrote:
> It doesn't break Subversion, it doesn't break Subversion, it doesn't
> break Subversion... :-)
Good ole Vinyl-Roms ;-) Broken CDs just don't have the same beat.
> (Sorry to sound like a broken record, but this misunderstanding
> appears to pervade this thread.)
>
> A read-only working copy need not be a fatal error. This optimization
> can be opportunistic; there is no correctness consequence if it
> doesn't happen. (And if the working copy is read-only, any other
> solution is going to fail too, even 'svn cleanup').
This is all well understood. Didn't I write 'read-only operation' and
not 'read-only WC'?
I was just suggesting an approach that might also be acceptible :-).
Cheers
-Lübbe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Lübbe Onken <l....@rac.de> writes:
> I'm +1 to fixing things on the fly, because I think it's the best
> thing to do from a users point of view. If on the other hand it
> 'breaks' subversion to turn read-only operations into read-write, I've
> got a suggestion to make, see below.
It doesn't break Subversion, it doesn't break Subversion, it doesn't
break Subversion... :-)
(Sorry to sound like a broken record, but this misunderstanding
appears to pervade this thread.)
A read-only working copy need not be a fatal error. This optimization
can be opportunistic; there is no correctness consequence if it
doesn't happen. (And if the working copy is read-only, any other
solution is going to fail too, even 'svn cleanup').
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Lübbe Onken <l....@rac.de>.
Ben Gollmer wrote:
>>.svn dirs and lockfiles. None of that stuff matters to the user.
>>They'll just be happy if things run faster.
I'm +1 to fixing things on the fly, because I think it's the best thing
to do from a users point of view. If on the other hand it 'breaks'
subversion to turn read-only operations into read-write, I've got a
suggestion to make, see below.
> Well, I'm just a humble user, but that's who UIs are for :) Anyway, it seems
> more logical to me to put WC fixup operations in 'svn cleanup'.
That is the best place to put them, no doubt.
> It can be
> documented in the book/online help; 'Over time, timestamps in your WC may get
> out of date, and stuff will slow down. Run svn cleanup to fix.'
Are you sure, this is going to work. Users will complain about
subversion being slow and not read the book...
> It seems possible that as time goes on, other fixup operations may be needed in
> the WC. Then you can say in general, 'If your WC seems screwy, run svn cleanup
> on it.' I also think that this approach would look cleaner from a programming
> standpoint, i.e. lumping all cleanup operations together in one place rather
> than having them scattered over several subcommands.
Hi Folks,
I see another viable solution here. If a(ny) svn operation detects some
'screwyness' in the WC it can report something like 'WC (screwed |
timestamps out of date | locked). run svn cleanup'.
This way status et al. wouldn't have to turn into a read-write operation
and everything could be bundled together in svn cleanup.
But - there's always a but :-)
I don't know how fast working copys degrade over time, so I have no idea
how likely this is to happen. If after every second svn command the user
would be prompted to run cleanup, then I stongly advocate fixing
timestamps on the fly.
Cheers
-Lübbe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Ben Gollmer <be...@jatosoft.com>.
On Thu, 30 Oct 2003 14:36:48 -0600, kfogel@collab.net wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
> > I am still uneasy about the whole idea of having read locks convert
> > into write locks. It takes away my ability to be sure that an
> > operation is read only. Suppose some operation doesn't work the way I
> > expect, how do I investigate it? Simply running 'svn status' might
> > change the working copy. I might have to start using two usernames,
> > one without write access!
>
> I'm not as uncomfortable with it as you are. I guess I started out
> from the point of view that managing the working copy is Subversion's
> business -- any Subversion operation can do whatever it feels is
> appropriate to make things work nicely. We developers are used to
> thinking of 'svn status' as read-only, and now we're considering it as
> read-write, only because we're aware of the underlying implemention of
> .svn dirs and lockfiles. None of that stuff matters to the user.
> They'll just be happy if things run faster.
Well, I'm just a humble user, but that's who UIs are for :) Anyway, it seems
more logical to me to put WC fixup operations in 'svn cleanup'. It can be
documented in the book/online help; 'Over time, timestamps in your WC may get
out of date, and stuff will slow down. Run svn cleanup to fix.'
It seems possible that as time goes on, other fixup operations may be needed in
the WC. Then you can say in general, 'If your WC seems screwy, run svn cleanup
on it.' I also think that this approach would look cleaner from a programming
standpoint, i.e. lumping all cleanup operations together in one place rather
than having them scattered over several subcommands.
--
Ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> >> Are you suggesting that status should attempt to take out a write
> >> lock, and if that fails take out a read lock? I hadn't thought of
> >> that. I guess that would be possible, but there would still be a
> >> performance hit.
> >
> > Exactly, yes. The performance hit would only happen in the case where
> > timestamp fixing was needed (see below).
>
> Exactly, yes or exactly, no? I'm confused. Do you mean a status
> implementation that tries to get write locks before crawling the
> working copy and if it fails falls back on read locks, or one that
> starts with read locks and converts to write locks as it progresses?
> I was thinking of the former, but you appear to be thinking of the
> latter.
Heh. Here's what I'm proposing:
'svn status' uses read locks by default. When it encounters a
timestamp(s) that need fixing, it attempts to convert the appropriate
read lock to a write lock. If it succeeds, it fixes the timestamp.
If it fails, it does not error, it just doesn't fix the timestamp.
(Of course, the reality will be slightly more sophisticated: it will
learn everything it can about a directory first, so it only has to get
a write lock once. But that's just an optimization, albeit a
necessary one for performance reasons.)
> On demand can be more expensive if it is trying and failing. It is
> generally cheaper to try and fail once at the start than to try and
> fail each time a broken lock is encountered. I suppose we could store
> the fact that we failed and not try again.
Yes -- facts that were expensive to discover can be cached.
> I am still uneasy about the whole idea of having read locks convert
> into write locks. It takes away my ability to be sure that an
> operation is read only. Suppose some operation doesn't work the way I
> expect, how do I investigate it? Simply running 'svn status' might
> change the working copy. I might have to start using two usernames,
> one without write access!
I'm not as uncomfortable with it as you are. I guess I started out
from the point of view that managing the working copy is Subversion's
business -- any Subversion operation can do whatever it feels is
appropriate to make things work nicely. We developers are used to
thinking of 'svn status' as read-only, and now we're considering it as
read-write, only because we're aware of the underlying implemention of
.svn dirs and lockfiles. None of that stuff matters to the user.
They'll just be happy if things run faster.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
>> Are you suggesting that status should attempt to take out a write
>> lock, and if that fails take out a read lock? I hadn't thought of
>> that. I guess that would be possible, but there would still be a
>> performance hit.
>
> Exactly, yes. The performance hit would only happen in the case where
> timestamp fixing was needed (see below).
Exactly, yes or exactly, no? I'm confused. Do you mean a status
implementation that tries to get write locks before crawling the
working copy and if it fails falls back on read locks, or one that
starts with read locks and converts to write locks as it progresses?
I was thinking of the former, but you appear to be thinking of the
latter.
>> > But anyway, why is converting a read-lock to a write-lock more
>> > expensive than having the write lock in the first place? In both
>> > cases, a lock file must be dropped into .svn/. And a bit set in a
>> > structure in memory, I suppose, but that's clearly not the expensive
>> > part.
>>
>> This is the important point: read locks do not create a lock file.
>> That makes write locks more expensive, particularly if the working
>> copy is on a network filesystem.
>
> Sure. I thought you were saying that taking out write locks on demand
> would be more expensive than taking them out in advance, and was only
> questioning that assertion (which you may not have been making).
On demand can be more expensive if it is trying and failing. It is
generally cheaper to try and fail once at the start than to try and
fail each time a broken lock is encountered. I suppose we could store
the fact that we failed and not try again.
> I was assuming the optimization of lock-once-per-directory; yes,
> without that, the performance would be horrible. (Sorry, should have
> been more explicit.)
It happens automatically if read locks are converted into write locks
on demend, but if we use temporary write locks it's needs more
thought.
I am still uneasy about the whole idea of having read locks convert
into write locks. It takes away my ability to be sure that an
operation is read only. Suppose some operation doesn't work the way I
expect, how do I investigate it? Simply running 'svn status' might
change the working copy. I might have to start using two usernames,
one without write access!
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
"C. Michael Pilato" <cm...@collab.net> writes:
> We went out of our way to get apr_temp_dir_get() into APR so that we
> could avoid assuming that we have write access to working copies for
> read-only operations. Can we please not invalidate that work? There
> is absolutely no reason why 'svn cat', 'svn log', 'svn status', 'svn
> propget', 'svn proplist', 'svn diff', 'svn diff', etc. should
> *require* write access to disk, even for such a noble cause as
> timestamp touch-up.
Arggh. :-)
No one here is proposing that write access be required. Not me, not
Philip.
> That said, I'm not opposed to some such read-only operations trying to
> fix timestamps as they go along, as long as they promise to never
> return an error if they are barred from doing so as a result of
> permissions problems.
That's what's under discussion.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by "C. Michael Pilato" <cm...@collab.net>.
kfogel@collab.net writes:
> > I think we would still need a way to prevent svn_client_status from
> > taking write locks. Consider a long running GUI that periodically
> > makes calls to svn_client_status to update it's view of the world. If
> > the svn_client_status calls take write locks then that would mean that
> > a command line commit would occasionally fail because it failed to get
> > a write lock.
>
> Good point, I hadn't thought of that. But still, if
>
> 1. Subversion operations are regularly fixing timestamps, and
> 2. otherwise-read-only operations only take out write locks when
> the absolutely need to fix a timestamp(s), and
> 3. Subversion is careful to only take out the write lock once per
> directory, and hold it only for as long as necessary to make all
> the fixes in that directory
>
> ...then any given run of GUI status (or whatever) will not have many
> write locks, and will not interfere with other things.
We went out of our way to get apr_temp_dir_get() into APR so that we
could avoid assuming that we have write access to working copies for
read-only operations. Can we please not invalidate that work? There
is absolutely no reason why 'svn cat', 'svn log', 'svn status', 'svn
propget', 'svn proplist', 'svn diff', 'svn diff', etc. should
*require* write access to disk, even for such a noble cause as
timestamp touch-up.
That said, I'm not opposed to some such read-only operations trying to
fix timestamps as they go along, as long as they promise to never
return an error if they are barred from doing so as a result of
permissions problems.
Finally, while 'svn status' would seem like the perfect place to do
this kind of timestamp patch-up, I strongly believe that it is a
special subcommand that should never write to a working copy, and
instead focus its efforts strictly on read-only reporting.
My thoughts.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> I would not want 'svn status' to *require* a write lock, I want to be
> able to run it on working copies for which I don't have write access.
>
> Are you suggesting that status should attempt to take out a write
> lock, and if that fails take out a read lock? I hadn't thought of
> that. I guess that would be possible, but there would still be a
> performance hit.
Exactly, yes. The performance hit would only happen in the case where
timestamp fixing was needed (see below).
> I think we would still need a way to prevent svn_client_status from
> taking write locks. Consider a long running GUI that periodically
> makes calls to svn_client_status to update it's view of the world. If
> the svn_client_status calls take write locks then that would mean that
> a command line commit would occasionally fail because it failed to get
> a write lock.
Good point, I hadn't thought of that. But still, if
1. Subversion operations are regularly fixing timestamps, and
2. otherwise-read-only operations only take out write locks when
the absolutely need to fix a timestamp(s), and
3. Subversion is careful to only take out the write lock once per
directory, and hold it only for as long as necessary to make all
the fixes in that directory
...then any given run of GUI status (or whatever) will not have many
write locks, and will not interfere with other things.
> > But anyway, why is converting a read-lock to a write-lock more
> > expensive than having the write lock in the first place? In both
> > cases, a lock file must be dropped into .svn/. And a bit set in a
> > structure in memory, I suppose, but that's clearly not the expensive
> > part.
>
> This is the important point: read locks do not create a lock file.
> That makes write locks more expensive, particularly if the working
> copy is on a network filesystem.
Sure. I thought you were saying that taking out write locks on demand
would be more expensive than taking them out in advance, and was only
questioning that assertion (which you may not have been making).
> > Philip, you pointed out (I think) the difference between a permanent,
> > operation-spanning write-lock and lots of little temporary ones which
> > would be released after being used. Why is the latter a performance
> > killer?
>
> The obvious implementation (the one I can see without thinking about
> it) would create and release a write lock for each timestamp changed,
> rather than once for the each directory. Think of the report we had
> some time ago of someone copying a working copy without preserving the
> timestamps, fixing that with temporary locks would involve two
> lock/release cycles for every file.
I was assuming the optimization of lock-once-per-directory; yes,
without that, the performance would be horrible. (Sorry, should have
been more explicit.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> But, I don't see why it's so bad for 'svn status' to take out a write
> lock (obviously, it shouldn't error if it can't -- it just wouldn't do
> the repairs, in that case).
I would not want 'svn status' to *require* a write lock, I want to be
able to run it on working copies for which I don't have write access.
Are you suggesting that status should attempt to take out a write
lock, and if that fails take out a read lock? I hadn't thought of
that. I guess that would be possible, but there would still be a
performance hit.
I think we would still need a way to prevent svn_client_status from
taking write locks. Consider a long running GUI that periodically
makes calls to svn_client_status to update it's view of the world. If
the svn_client_status calls take write locks then that would mean that
a command line commit would occasionally fail because it failed to get
a write lock.
> Also:
>
> Why is it bad, performance-wise, for a read-only operation like status
> to convert read-locks to write-locks on the fly? It would only have
> to do it in the directories where it detected problems, and of course,
> next time around that directory likely wouldn't be a problem -- so any
> given run of status is not likely to be slowed down too much.
Permanent on-the-fly conversion is probably not that bad in
performance terms, just a bit tricky to handle as far as program logic
goes. The following pseudo-code works perfectly at present since read
locks are a pure memory structure
frobnicate()
{
read_lock = svn_wc_adm_open(...)
if (foo (read_lock))
{
write_lock = svn_wc_adm_open(...)
bar (write_lock)
svn_wc_adm_close (write_lock)
}
}
If read locks sometimes become write locks then the function will
sometimes fail (I'm holding read_lock and write_lock simultaneously).
It might never happen during testing, bug reports are likely to say
something like "it occasionally fails, but if I run it again it just
works".
> But anyway, why is converting a read-lock to a write-lock more
> expensive than having the write lock in the first place? In both
> cases, a lock file must be dropped into .svn/. And a bit set in a
> structure in memory, I suppose, but that's clearly not the expensive
> part.
This is the important point: read locks do not create a lock file.
That makes write locks more expensive, particularly if the working
copy is on a network filesystem.
> Philip, you pointed out (I think) the difference between a permanent,
> operation-spanning write-lock and lots of little temporary ones which
> would be released after being used. Why is the latter a performance
> killer?
The obvious implementation (the one I can see without thinking about
it) would create and release a write lock for each timestamp changed,
rather than once for the each directory. Think of the report we had
some time ago of someone copying a working copy without preserving the
timestamps, fixing that with temporary locks would involve two
lock/release cycles for every file.
> In both cases, each lockfile used needs to be both created
> and removed. But if you just lock individual directories when you
> need a lock, then fewer lockfiles will be created and removed in
> total, so I'd actually expect that strategy to be *better*,
> performance-wise. (Incidentally, it shouldn't matter whether the each
> lock is removed immediately after being used, or all the locks removed
> at once at the end, should it?)
>
> Finally:
>
> In theory, it's nice that status doesn't block other operations. But
> in practice, how often does it really matter? I doubt most Subversion
> users would notice if status started taking out write-locks.
Probably not if they are only using one application. If multiple
applications run in parallel they may get problems.
> Again, if for whatever reason a write-lock can't be obtained, it
> wouldn't be an error. The operation simply wouldn't repair the
> timestamp; no big deal.
We would also need more code in status to distinguish the locks taken
by status from the locks that existed before status was run. I don't
want status to tell me that every directory is locked.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> That's better from an implementaion/performance point of view, but I'm
> not convinced that we really want it in the default status output. We
> come back to my suggestion 1) an additional option to status, one that
> causes it to take out a write lock, and to report and repair broken
> timestamps.
Before anything else: yay Philip for attacking issue #791! This will
really help client performance.
On to current questions:
I have to agree with Julian, that exposing this implementation detail
to the user is soooo not the road to go down.
But, I don't see why it's so bad for 'svn status' to take out a write
lock (obviously, it shouldn't error if it can't -- it just wouldn't do
the repairs, in that case).
Also:
Why is it bad, performance-wise, for a read-only operation like status
to convert read-locks to write-locks on the fly? It would only have
to do it in the directories where it detected problems, and of course,
next time around that directory likely wouldn't be a problem -- so any
given run of status is not likely to be slowed down too much.
But anyway, why is converting a read-lock to a write-lock more
expensive than having the write lock in the first place? In both
cases, a lock file must be dropped into .svn/. And a bit set in a
structure in memory, I suppose, but that's clearly not the expensive
part.
Philip, you pointed out (I think) the difference between a permanent,
operation-spanning write-lock and lots of little temporary ones which
would be released after being used. Why is the latter a performance
killer? In both cases, each lockfile used needs to be both created
and removed. But if you just lock individual directories when you
need a lock, then fewer lockfiles will be created and removed in
total, so I'd actually expect that strategy to be *better*,
performance-wise. (Incidentally, it shouldn't matter whether the each
lock is removed immediately after being used, or all the locks removed
at once at the end, should it?)
Finally:
In theory, it's nice that status doesn't block other operations. But
in practice, how often does it really matter? I doubt most Subversion
users would notice if status started taking out write-locks.
Again, if for whatever reason a write-lock can't be obtained, it
wouldn't be an error. The operation simply wouldn't repair the
timestamp; no big deal.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps
Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:
> What about status? If it detects a bogus timestamp, can it at that
> point take out a write lock and attempt to repair?
It could, but it doesn't at present. I don't really like the idea of
a read-only operation like status attempting to become
read-write.
On the implementaion front, it's probably not feasible to permanently
convert an existing read lock into a write lock, so it might have to
attempt to take and release a temporary write lock. That could be a
real performance killer if there are a large number of broken
timestamps, it's much better to do the repairs in an operation that
starts out with a write lock. What if anything should happen if it
failed to get a write lock?
>> There is no way, at present, for the user to find out that Subversion
>> has detected the problem.
>
> Failing the above, would an extra notice in the status output help?
> 'T' for bogus timestamp? This would then prompt the user to run
> svn cleanup (while not requiring it). Ofcourse this could be unclear...
> and given the new status output, potentially overkill.
That's better from an implementaion/performance point of view, but I'm
not convinced that we really want it in the default status output. We
come back to my suggestion 1) an additional option to status, one that
causes it to take out a write lock, and to report and repair broken
timestamps.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Fixing time stamps
Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
> Sent: Thursday, October 30, 2003 2:19 PM
> "Sander Striker" <st...@apache.org> writes:
>
> > So when do we find out? Would we know during an svn status run?
> > During svn update? Does this require a write lock to detect?
>
> Subversion detect's it every time status (or revert or commit) is run.
Ok, I can see us repairing automagically at revert and commit then.
What about status? If it detects a bogus timestamp, can it at that
point take out a write lock and attempt to repair?
> There is no way, at present, for the user to find out that Subversion
> has detected the problem.
Failing the above, would an extra notice in the status output help?
'T' for bogus timestamp? This would then prompt the user to run
svn cleanup (while not requiring it). Ofcourse this could be unclear...
and given the new status output, potentially overkill.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Fixing time stamps, WAS: RE: svn commit: rev 7565 - in
trunk/subversion: include libsvn_wc tests/clients/cmdline
Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:
> So when do we find out? Would we know during an svn status run?
> During svn update? Does this require a write lock to detect?
Subversion detect's it every time status (or revert or commit) is run.
There is no way, at present, for the user to find out that Subversion
has detected the problem.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Fixing time stamps, WAS: RE: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc tests/clients/cmdline
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: Thursday, October 30, 2003 2:12 PM
>> There is no easy way to detect that "broken" timestamps are present.
So when do we find out? Would we know during an svn status run?
During svn update? Does this require a write lock to detect?
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Fixing time stamps, WAS: RE: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc tests/clients/cmdline
Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
> Sent: Thursday, October 30, 2003 2:07 PM
> Julian Foad <ju...@btopenworld.com> writes:
>
> > No, we don't! It's an implementation detail, a performance
> > optimisation, like caching. Why should the user need to even know
> > that it exists, let alone have a command for activating it?
>
> There is no easy way to detect that "broken" timestamps are present.
> If one does not regularly commit from a working copy then broken
> timestamps may accumulate, and a small number of them can have a
> drastic effect on the performance of "svn status".
And telling users they may want to run svn cleanup once in a while
just in case just doesn't cut it. Interesting dilemma.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc
tests/clients/cmdline
Posted by Philip Martin <ph...@codematters.co.uk>.
Julian Foad <ju...@btopenworld.com> writes:
> No, we don't! It's an implementation detail, a performance
> optimisation, like caching. Why should the user need to even know
> that it exists, let alone have a command for activating it?
There is no easy way to detect that "broken" timestamps are present.
If one does not regularly commit from a working copy then broken
timestamps may accumulate, and a small number of them can have a
drastic effect on the performance of "svn status".
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7565 - in trunk/subversion: include libsvn_wc
tests/clients/cmdline
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> philip@tigris.org writes:
>
>>Add a mechanism that "repairs" the text-time and prop-time in the
>>entries file if they differ from the time of the working copy files
>>even though there are no text/property differences. This addresses
>>issue 791.
...
> The new "repair" code runs automatically whenever Subversion checks
> for changes while holding a write lock. Ideally one would be able to
> trigger the repair of an entire working copy, at present I know of two
> ways to ways to do it:
>
> 1) run "svn revert --recursive"
> 2) run "svn commit"
>
> Neither of these is ideal. Using 1) has the obvious disadvantage that
> it destroys any local modifications. Using 2) doesn't require one to
> complete the commit, quitting the log message editor without saving
> and aborting the commit will do the job, but it's hardly the interface
> one would choose.
>
> Do we need a more intuitive interface this repair? There are a number
No, we don't! It's an implementation detail, a performance optimisation, like caching. Why should the user need to even know that it exists, let alone have a command for activating it?
I suspect that it is only because you know what's going on under the hood that you feel the need to make Subversion clean it up.
> of things we could do: 1) add a new option to "svn status" to cause it
> to take a write lock, 2) make "svn cleanup" run the check for changes
> on every entry, 3) add a new "svn repair" command, etc. I'm not
> overly keen on any of those, "svn cleanup" is the one I dislike least,
> so I'd choose to leave things as they are. Thoughts?
+0 on "cleanup" doing it, as "cleanup" is already an "under the hood" command for "un-breaking" Subversion. Cleaning up the entries time stamps is a reasonable thing for it to do.
-1 on any other user interface for activating the clean-up.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org