All,

Current behavior of Firebird metadata cache is that every type of object
works different on it, and works different in Super vs (Super)Classic
architectures. For example:
- Multiple procedure versions may run at same time and if one drops a
live version it is forgotten to be died with the database pool.
- Changes to one object doesn't invalidate the ones that depends on it.
For example, if a view is being used on a cached (already used)
procedure, the procedure just doesn't use the new view definition if
it's altered.
- Uncommitted procedures may run on the transaction that defined it -
you may call it feature but seems non-destructive bug for me.
- UDFs aren't handled at all. If you drop and recreate an UDF it just
continue using the old definition (CORE-1973).
- Triggers also works different and remains running with old definitions
under some conditions AFAIR.
- There are two caches (DSQL and JRD), each one working different.

I consider metadata changes a big weak area of FB that should be changed
on V3. It's now very unpredictable and unreliable and a lot (procedure
use counters, for example) of MT-unsafe code is used to support all
these inconsistencies.

I have prototype code that:
- Implements correct (IMHO) locking policy and thread safe caching and
lookups
- Removed DSQL metadata cache

This code is currently only for procedures and functions, is not
complete and I've not yet thought about direct or indirect circular
dependencies handling at DSQL and JRD layers, but allowed to me see that
this locking policy works and is simple (much less code, with isolated
logic to handle locking, invalidation and shared resource lists) to
implement. Proposed implementation just deprecates somewhat big amount
of code handling procedure use counters and locking/convert/unlocking on
DFW. Such locking policy should be used for all object types, I think.

As I don't want to fork entire tree on private branch with not discussed
things, let discuss them. :-)

I should start saying that proposed locking policy is more restrictive
than the many currently being used, but is MT-safe for SuperServer and
should work identical in (Super)Classic. And as it's more restrictive,
MON$ structure and actions may be extended to allow:
- Monitoring of object locks acquired per prepared statement.
- Kill prepared statement - it currently only can kill statement
execution AFAIU.

Main idea is that when an object is used in a prepared statement, a read
lock is acquired on it. The lock is released only when the statement is
unprepared. When altering or dropping an object, a write lock is
acquired. If one attempts to acquire read and write lock for a specific
object on the same attachment, an object in use error is thrown. When I
said about lock and unlock, I mean GlobalRWLock methods and its locking
caching mechanism. So if an object is locked by us, AST will not cause
it to be invalidated. AST runs only when there is no explicit lock,
i.e., only GlobalRWLock cached lock.

All metadata object classes are inherited from MetaObject, a class
that's inherited from GlobalRWLock. MetaObject::fetch sets "valid"
attribute to true and MetaObject::invalidate sets "valid" to false. Both
methods are overridden from GlobalRWLock. There is an abstract class
MetaObjectHolder with methods to add objects to it and all objects are
released on its destructor. Concrete implementations are:
- MetaRefHolder: objects added to it have it "interested" attribute
incremented. Release decrements "interested" of added objects.
- MetaLockHolder: on its constructor parameter is choose between read or
write locking. Objects added are locked. Release unlocks objects.

MetaRefHolder is used on MetaObject::dependenciesHolder, so if procedure
P1 depends on procedure P2 and procedure P1 is cached, procedure P2 will
not be locked, hence droppable.
MetaLockHolder(write lock) is used on DeferredWork class, working
totally consistent with savepoint logic. After VIO adds a deferred work,
it lookups the object and add it to the created
DeferredWork::metaLockHolder. This is done even on creating objects, so
internal read-uncommitted system transaction doesn't allow the use of
the object by others.
MetaLockHolder(read lock) is used in dsql_req, CompilerScratch and
jrd_req classes and DML compilation pass it to metadata lookup functions.

Metadata lookup functions (FUN_lookup_function, MET_lookup_procedure,
etc) receives a reference (called callerLockHolder) to a MetaLockHolder.
If MetaLockHolder is a write holder (i.e., it comes from DeferredWork),
it means we're trying to alter or drop an object and dependencies are
not lookup ed. If it's a read holder, we're trying to use the object so
dependencies should be correctly compiled. In read mode, dependencies
lookups pass a local object in the same mode of callerLockHolder to
subsequent lookups and when everything is ok these locks are transferred
to callerLockHolder. Locks of dependencies are released (for caching
purposes, but remains locked for the caller) and added to object
MetaRefHolder. For purposes of compiled statement caching, such logic of
"soft locks" (through MetaRefHolder) should be used too.

I hope these explanations are understandable, but to make things clear
here are peaces of pseudo-code of key areas. Feel free to ask for details.

Generic part:
----------------
class MetaObjectHolder
{
public:
virtual void add(thread_db* tdbb, MetaObject* obj); // just adds
obj to objects
virtual void release() = 0;

public:
void addHolder(thread_db* tdbb, MetaObjectHolder& holder); // call
add for alll holder.objects
bool validate() const; // returns true if all objects valid is
true and false otherwise

protected:
Array<MetaObject*> objects; // will change to SortedArray for
filter duplicate additions
};

class MetaRefHolder : public MetaObjectHolder
{
public:
~MetaRefHolder()
{
release();
}

public:
virtual void add(thread_db* tdbb, MetaObject* obj); // increments
obj->interested
virtual void release(); // decrements all objects interested
};

class MetaLockHolder : public MetaObjectHolder
{
public:
MetaLockHolder(MemoryPool& p, bool aExclusive);

~MetaLockHolder()
{
release();
}

public:
virtual void add(thread_db* tdbb, MetaObject* obj); // lock
(read/write accordingly to exclusive attribute)
virtual void release(); // unlock (read/write accordingly to
exclusive attribute)

public:
bool isExclusive() const
{
return exclusive;
}

private:
bool exclusive;
};

class MetaObject : public GlobalRWLock
{
public:
MetaObject(thread_db* tdbb, MemoryPool& p, locktype_t lockType,
const string& aLockKey);

virtual ~MetaObject()
{
}

public:
virtual void fetch(thread_db* tdbb);
virtual void invalidate(thread_db* tdbb, bool astHandler);

public:
MetaRefHolder dependenciesHolder;
AtomicCounter interested;
bool valid;
bool dropping; // set to true before releasing write lock
};

MetaObject::MetaObject(thread_db* tdbb, MemoryPool& p, locktype_t lockType,
const string& aLockKey)
: GlobalRWLock(tdbb, p, lockType, aLockKey.length(), (const UCHAR*)
aLockKey.c_str()),
dependenciesHolder(p),
valid(true),
dropping(false)
{
}

void MetaObject::fetch(thread_db* tdbb)
{
valid = true;
}

void MetaObject::invalidate(thread_db* tdbb, bool astHandler)
{
valid = false;
}

class MetaDataCache : public PermanentStorage
{
public:
void release(); // delete functions and procedures

public:
RWLock functionsLock;
GenericMap<Pair<Left<MetaName, UserFunction*> > > functions;

RWLock proceduresLock;
GenericMap<Pair<Left<MetaName, jrd_prc*> > > procedures;
GenericMap<Pair<NonPooled<SSHORT, jrd_prc*> > > proceduresById;
};
----------------

Procedure part:
----------------
class jrd_prc : public MetaObject
{
public:
explicit jrd_prc(thread_db* tdbb, MemoryPool& p, const
Firebird::MetaName& name)
: MetaObject(tdbb, p, LCK_prc_exist, name.c_str()),
prc_name(p, name),
...
};

jrd_prc* MET_lookup_procedure(thread_db* tdbb, const MetaName& name,
MetaLockHolder& callerLockHolder)
{
SET_TDBB(tdbb);
Database* dbb = tdbb->getDatabase();

do
{
jrd_prc* procedure = NULL;

{ // scope
ReadLockGuard readGuard(dbb->metaDataCache.proceduresLock);

if (dbb->metaDataCache.procedures.get(name, procedure) &&
(callerLockHolder.isExclusive() || (procedure->valid &&
!procedure->dropping)))
{
++procedure->interested; // prevent deletion after we
release the array lock
}
else
procedure = NULL;
}

// newProcedure should be declared before localHolder, to be
destroyed after it
AutoPtr<jrd_prc> newProcedure;
MetaLockHolder localHolder(*getDefaultMemoryPool(),
callerLockHolder.isExclusive());

try
{
if (procedure)
{
// acquire locks
localHolder.add(tdbb, procedure);
if (!callerLockHolder.isExclusive())
localHolder.addHolder(tdbb,
procedure->dependenciesHolder);

// If everything is still valid, transfer the locks to
the caller.
if (callerLockHolder.isExclusive() ||
localHolder.validate())
{
callerLockHolder.addHolder(tdbb, localHolder);
procedure->dropping = false;
--procedure->interested;
return procedure;
}
else
{
// Someone else has invalidated this object after we
released the array lock and
// before we acquired the objects locks. We're not
interested on it anymore.
--procedure->interested;
procedure = NULL;
}
}
}
catch (...)
{
if (procedure)
--procedure->interested;
throw;
}

newProcedure = FB_NEW(*dbb->dbb_permanent) jrd_prc(tdbb,
*dbb->dbb_permanent, name);
localHolder.add(tdbb, newProcedure); // acquire lock

if (callerLockHolder.isExclusive())
procedure = newProcedure;
else
{
// ommitted: Lookup procedure in system tables. If it's found,
// set procedure = newProcedure and loads it.

{ // scope
Jrd::ContextPoolHolder context(tdbb, csb_pool);
AutoPtr<CompilerScratch>
csb(CompilerScratch::newCsb(*tdbb->getDefaultPool(), 5));

// ommitted: procedure type and debugging

// Now, check the result of this function here! False
means failure.
// Or should parse_procedure_blr and its callee throw
exceptions instead?
if (!parse_procedure_blr(tdbb, procedure,
&P.RDB$PROCEDURE_BLR, csb, external))
{
if (procedure->prc_request) {
CMP_release(tdbb, procedure->prc_request);
procedure->prc_request = NULL;
}
else {
dbb->deletePool(csb_pool);
}

ERR_post(isc_bad_proc_BLR,
procedure->prc_name.c_str(), isc_arg_end);
}

localHolder.addHolder(tdbb,
procedure->prc_request->metaLockHolder);

// As we didn't want to maintain dependencies locked,
release them now.
// They're already locked by line above on localHolder
for our caller.
procedure->prc_request->metaLockHolder.release();

procedure->prc_request->req_procedure = procedure;

// ommitted: messages
}

// ommitted: scanned flag and valid BLR

procedure->dependenciesHolder.addHolder(tdbb, localHolder);
}

if (procedure)
{
WriteLockGuard writeGuard(dbb->metaDataCache.proceduresLock);

// If someone else already cached the object and it's still
valid, release our version
// and start the lookup again.
jrd_prc* cachedProcedure = NULL;
if (dbb->metaDataCache.procedures.get(name, cachedProcedure)
&& cachedProcedure->valid)
continue;

if (cachedProcedure)
{
// We have an invalid cached procedure. First, release
interest on dependencies.
cachedProcedure->dependenciesHolder.release();

// And if nobody is interested on it, delete it now.
if (cachedProcedure->interested.value() == 0)
delete cachedProcedure;
}

dbb->metaDataCache.procedures.put(name, procedure);
dbb->metaDataCache.proceduresById.put(procedure->prc_id,
procedure);

// Transfer the locks to the caller.
callerLockHolder.addHolder(tdbb, localHolder);

newProcedure.release(); // our lookup succeeded, so don't
destroy the object
}

return procedure;
} while (true);

return NULL; // silence compiler warning
}
----------------

This code, in some circumstances, causes that objects can't be deleted
(due to its interested.value() != 0. It may happen when:
- The object was on the cache and was valid, but before the lock is
acquired it becomes invalid. This can happen when there's two concurrent
read lookups with one write and the write succeeded and invalidated the
object, the first succeeded reader can't delete the object, as it can be
currently being used by the other reader.
- If object is a dependency of a not-yet invalidated object. This may be
improved to cascade invalidate objects.

For such cases, these objects may be removed from the metadata arrays
(procedures, functions) and added to another array of invalid objects.
These arrays can be scanned periodically and objects with
interested.value() == 0 could be deleted.

Thoughts?


Adriano