[PATCH] Re: mbk's recovery lock change

[Tobias Ringström <to...@ringstrom.mine.nu>]

Greg Hudson wrote:

>So, to summarize:
>
>  What we have now:  No diagnostic, blocking lock attempt
>  mbk's alternative: Diagnostic if we can't get lock, then block
>  My alternative:    Fail if we can't get lock
>  Other alternative: Diagnostic if we can't get lock, then block,
>                      and print another message when we do get lock
>  
>
A problem with the current solution that you didn't mention is that it's 
really hard to kill svnadmin while it's waiting to acquire the lock 
because we ignore SIGINT and SIGKILL. I don't like the "fail if we can't 
get lock" approach for the reason you quoted from mbk, but it's not only 
a problem when running svnserve from inetd. You can get the same 
problems with file:// and svn+ssh://. If you have a busy server that 
serves multiple repositories, you don't want to shut down the whole 
server just to recover a single repository.

The attached patch implements your "other alternative" above, and also 
makes it possible to interrupt svnadmin while waiting for the lock. 
Opinions?

/Tobias

[[[
Make 'svnadmin recover' print a warning message when it's waiting for
the exclusive lock and a message when the recovery has started.  Allow
svnadmin to be interrupted while waiting for the lock.

* svnadmin/main.c
  (setup_cancellation_signals): New helper function.

  (recovery_started): New function to inform the user that the recovery
  has started.

  (subcommand_recover): Restore default signal handling before calling
  svn_repos_recover2.  Call the updated svn_repos_recover2 with
  recovery_started as argument.

  (main): Use the new setup_cancellation_signals.

* include/svn_repos.h
  (svn_repos_recover2): Add a callback to be called when the recovery
  starts and a baton to be supplied to the callback.

* libsvn_repos/repos.c
  (svn_repos_recover2): Call the callback after acquiring the lock.

* bindings/java/javahl/native/SVNAdmin.cpp
  (SVNAdmin::recover): Update to match updated svn_repos_recover2.
]]]

Re: [PATCH] Re: mbk's recovery lock change

[Tobias Ringström <to...@ringstrom.mine.nu>]

Greg Hudson wrote:

>I'd still like to see more people's opinions on what the
>correct behavior is.
>  
>
Yes, of course. I got carried away while thinking about the issue. 
That's why my opinion came with a patch attached. :-)

/Tobias


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: mbk's recovery lock change

[Greg Hudson <gh...@MIT.EDU>]

On Sun, 2004-08-29 at 10:10, Tobias Ringström wrote:
> The attached patch implements your "other alternative" above, and also 
> makes it possible to interrupt svnadmin while waiting for the lock. 
> Opinions?

Your patch looks correct to me (I'm surprised that APR's apr_signal
layer is so thin that it's appropriate to use SIG_DFL, but it seems to
be that way).  I'd still like to see more people's opinions on what the
correct behavior is.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: mbk's recovery lock change

[Max Bowsher <ma...@ukf.net>]

Tobias Ringström wrote:
> Greg Hudson wrote:
>
>>So, to summarize:
>>
>>  What we have now:  No diagnostic, blocking lock attempt
>>  mbk's alternative: Diagnostic if we can't get lock, then block
>>  My alternative:    Fail if we can't get lock
>>  Other alternative: Diagnostic if we can't get lock, then block,
>>                      and print another message when we do get lock
>>
>>
> A problem with the current solution that you didn't mention is that it's
> really hard to kill svnadmin while it's waiting to acquire the lock
> because we ignore SIGINT and SIGKILL. I don't like the "fail if we can't
> get lock" approach for the reason you quoted from mbk, but it's not only
> a problem when running svnserve from inetd. You can get the same
> problems with file:// and svn+ssh://. If you have a busy server that
> serves multiple repositories, you don't want to shut down the whole
> server just to recover a single repository.
>
> The attached patch implements your "other alternative" above, and also
> makes it possible to interrupt svnadmin while waiting for the lock.
> Opinions?

+1 (concept), especially about being able to Ctrl-C whilst waiting on the 
lock.

Nothing against the patch, just haven't had time to review in detail.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org