about "i#52: lock database"

[Ivan Glushkov]

On 5/15/07, Vlad Skvortsov <vss at 73rus.com> wrote:
> Ivan Glushkov wrote:
> >
> > The final decision should be the following.
> > 1. DITrack.Util.Locking should provide our project with OS-independent
> > way for locking and unlocking of any files.
> > Functions in this module:
> >   * DITrack.Util.Locking.lock(file_descriptor) - lock file descriptor.
> > Doesn't return value.
> >      If this file descriptor has already been locked, raise some
> > exception (FileIsLockedError?)
> >   * DITrack.Util.Locking.unlock(file_descriptor) - unlock file
> > descriptor. Doesn't return value.
> Several comments.
>
> * After giving it another thought, I suggest to place this compatibility
> layer in DITrack.Compat.Locking, since it's not just a utility [bikeshed].
"compatibility" - do you mean os-independent?
i suppose all the code should not depend on platform, so it's not a
correct decision to isolate such code into separate module.

from i#155 we have logical splitting into modules (and compatibility
layer might be everywhere in them):

DITrack/        - everything DITrack-related
        ThirdParty      - third-party modules
        Util            - utility/abstraction modules
        Lib             - public-accessible DITrack interface
        dt              - command-line client specific modules

file lock function should be the part of "utility/abstraction module".

>
> * I suggest using object-oriented interface. A lock should be
> represented by an object, where the object lifetime corresponds to the
> lock lifetime.
>
> * We do not need any support for regional locking as of now, so let's
> use whole file locks (it's not something that is wrong in your proposal,
> Ivan; just mentioning for the sake of completeness).
>
> * We do need a notion of lock type: shared/read vs. exclusive/write, so
> it should be mentioned in the arguments.

do we need to distinguish shared/read lock with the absent of lock?
(i.e. do we need shared/read lock at all? What for?)
In previous letter i meant only exclusive/write lock as the necessity
for the simultaneous attempts to change database.

> * We do need a support for blocking on lock (i.e. when we are trying to
> acquire a read lock, but there is another process holding a write lock:
> rather then failing right away, the implementation should try waiting
> for specified amount of time).

i believe that commands that make only reads from database shouldn't
be blocked. We want to get opportunity to read information from db
while changing it.

> * I guess we should pass file objects, not descriptors. Otherwise we
> might run into issues like referring to a wrong (recycled) file descriptor:
>     - open file, got FD 3;
>     - lock FD3;
>     - the application closes the file object (thus closing FD 3), but
> forgets to remove the lock object;
>     - the application opens another file and gets (recycled) FD 3;
>     - now any lock operation will deal with the wrong file.
agree.


> I envision the API along the following lines:
>
>     f = open("somefile", "w")
>
>     # Try to lock, if can't do it within 5 seconds, raise an exception
>     try:
>         # yes, we really need to settle with Module.Naming.Conventions :-)
>         lock = DITrack.Compat.Lock.Lock("r", timeout=5)
hmm... where is "f" (file object) in last string?


>     except LockCannotBeAquired:
>        ...
>
>     # Read the file
>     ...
>
>     # We need to figure, if it's feasible to implement. If not, we can
> live without it for now (or work it around).
>     # Upgrade shared lock to exclusive one to perform DB mods.
>     lock.upgrade()
>
>     # Modify the file
>     ...
>
>     # Downgrade back to the read lock.
>     lock.downgrade()
>
>     # The lock gets deleted once the object is deallocated.
>
>
> >
> > 2. DITrack.DB.Common should provide the following funcions:
> >   * DITrack.DB.Common.lock(db_path) - lock the database directory.
> > Doesn't return values.
>
> It doesn't need to be public though. It's an internal database method;
> why not DITrack.DB.Common.Database._lock()?
why not, actually :)

> >      This function should just create special file (.ditrack/LOCK for
> > now) write self pid into it and lock it, saving file descriptor.
> >      If errors occurs, exceptions would raise (in algorithm described
> > by Vlad in previous letter):
> >>       if there is no lock on the file,
> >>          raise DITrack.DB.Exceptions.DBStaleLockError
> >>       else
> >>          if there is sensible PID in the file,
> >>             raise DITrack.DB.Exceptions.DBIsLockedError(PID)
> >>          else
> >>             raise DITrack.DB.Exceptions.DBLockFileCorruptedError
> >   * DITrack.DB.Common.unlock() - unlock database. Doesn't return value.
> >
> > Proposals?
>
> Hmm, looking at these exceptions with fresh eyes, do we really need them
> all? The caller would [probably] like to _know_ the details (e.g. was
> there a stale lock, etc), but I guess we should either lock or fail.  So
> from the API user perspective, the DB constructor should probably raise
> only DBIsLockedError.
i don't understand. We do either lock or fail. (return fd or raise
some exception), don't we?
DBIsLockedError should not be the same as DBStaleLockError, as the
last one may be catched and processed with some external code.
DBLockFileCorruptedError is useless, i agree.


> Hmm. :-) Screw the locking details either!
>
>
> --
> Vlad Skvortsov, vss at 73rus.com, http://vss.73rus.com
>
>

Ivan.