Branko Čibej wrote:
> Philip Martin wrote:
>> If the svn__atomic_xxx calls are necessary then I think the panic flag
>> should be "volatile svn__atomic_t", however I don't really see why the
>> svn__atomic_xxx calls are needed or even why the flag is needed. In
>> bdb_cache_get the flag is redundant, it serves to avoid a BDB
>> get_flags() call but only when the flag is TRUE and there is little
>> point in such micro-optimisations in the "recovery needed" path. The
>> other uses of the panic flag are in the code used to close the
>> database, again it seems unnecessary to avoid a get_flags() call in
>> that code. I think we should remove the panic flag and substitute
>> get_flags() calls in the code to close the database.
>>
> The real purpose of the panic flag is to avoid calls to DB_ENV->open
> if we know that the cached environment handle is useless. Once the
> environment is panicked, it's important to release all references to
> it quickly, so that the queue of processes waiting to do the recovery
> doesn't get too long.
>
> At first I'd considered removing a broken environment from the cache,
> but that created races that would've been really hard to avoid. So I
> left it in the cache and marked it useless instead. This also made the
> reference counting cleaner.
Oh yes, I almost forgot -- earlier BDB versions don't have the
DB_ENV->get_flags function, so the only way to notice a panicked
environment is by catching DB_RUNRECOVERY errors in DB->close (or
DB_ENV->close). So (once I commit the patch I asked Ben to test) that
micro-optimisation will only be used with BDB-4.4 and later, and that's
a good thing because it means that a broken environment will get
released that much sooner.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jan 28 02:59:10 2006