Skip to content
Snippets Groups Projects
  1. Sep 11, 2008
    • Guido Trotter's avatar
      LockSet: forbid add() on a partially owned set · d2aff862
      Guido Trotter authored
      This patch bans add() on a half-acquired set. This behavior was
      previously possible, but created a deadlock if someone tried to acquire
      the set-lock in the meantime, and thus is now forbidden. The
      testAddRemove unit test is fixed for this new behavior, and includes a
      few more lines of testing and a new testConcurrentSetLockAdd function
      tests its behavior in the concurrent case.
      
      Reviewed-by: imsnah
      d2aff862
    • Guido Trotter's avatar
      Fix typo in a locking.py comment · ab62526c
      Guido Trotter authored
      Reviewed-by: imsnah
      ab62526c
    • Guido Trotter's avatar
      Add GanetiLockManager.is_owned function · d4f4b3e7
      Guido Trotter authored
      This is a public version of the private function we already had.
      We don't just change the previous version because it had lots of users
      in the library itself and in the testing code.
      
      Reviewed-by: imsnah
      d4f4b3e7
    • Guido Trotter's avatar
      Fix LockSet._names() to work with the set-lock · d4803c24
      Guido Trotter authored
      If the set-lock is acquired, currently, the _names function will fail on
      a double acquire of a non-recursive lock. This patch fixes the behavior,
      and some lines of code added to the testAcquireSetLock test check that
      this and other functioins behave properly.
      
      Reviewed-by: imsnah
      d4803c24
  2. Sep 05, 2008
    • Guido Trotter's avatar
      Add locking.ALL_SET constant and use it · e310b019
      Guido Trotter authored
      Rather than specifying None in needed_locks every time, with a nice
      comment saying to read what we mean rather than what we write, and that
      None actually means All, in our magic world, we'll hide this secret
      under the ALL_SET constant in the locking module, which has value, you
      guessed it, None. After that we'll substitute all usage in cmdlib.
      
      Some comments and examples have been fixed as well.
      
      Reviewed-by: iustinp
      e310b019
  3. Aug 18, 2008
    • Guido Trotter's avatar
      LockSet: allow lists with duplicate values · 34ca3914
      Guido Trotter authored
      If a list with a duplicate value is passed to a lockset what the code
      now does is to try to acquire the lock twice, generating a
      double-acquire exception in the SharedLock code. This is definitely an
      issue. In order to solve it we can either forbit double values in a list
      or just delete the duplicates. In this patch we go for the latter
      solution, removing any duplicate values when creating the acquire_list.
      
      Reviewed-by: imsnah
      34ca3914
  4. Jul 23, 2008
    • Guido Trotter's avatar
      Invert nodes/instances locking order · 04e1bfaf
      Guido Trotter authored
      An implementation mistake from the original design caused nodes to be
      locked before instances, rather than after. This patch inverts the level
      numbering, changing also the relevant unittests and the recursive
      locking function starting point.
      
      Reviewed-by: iustinp
      04e1bfaf
  5. Jul 08, 2008
    • Guido Trotter's avatar
      Locking: remove LEVEL_CONFIG lockset · 08a6c581
      Guido Trotter authored
      Since the ConfigWriter now handles its own locking it's not necessary to
      have a specific level for the config in the Locking Manager anymore.
      This patch thus removes it, and all the unittest calls that used it, or
      depended on it being present.
      
      Reviewed-by: iustinp
      08a6c581
    • Guido Trotter's avatar
      Locking: add ssynchronized decorator · 42a999d1
      Guido Trotter authored
      This patch creates a new decorator function ssynchronized in the locking
      library, which takes as input a SharedLock, and synchronizes access to
      the decorated functions using it. The usual SharedLock semantics apply,
      so it's possible to call more than one synchronized function at the same
      time, when the lock is acquired in shared mode, and still protect
      against exclusive access.
      
      The patch also adds a few unit test to check the basic decorator's
      functionality, and to provide an example on how to use it.
      
      Reviewed-by: iustinp
      42a999d1
  6. Apr 15, 2008
    • Guido Trotter's avatar
      SharedLock: restrict assertion condition · 94285814
      Guido Trotter authored
      When we release a shared lock if there are no exclusive waiter then the number
      of shared waiters must be exactly equal to the ones scheduled to pass.
      
      Reviewed-by: iustinp
      94285814
  7. Apr 02, 2008
  8. Mar 18, 2008
    • Guido Trotter's avatar
      Locking: remove an empty space at End Of Line · 21a6c826
      Guido Trotter authored
      Reviewed-by: imsnah
      21a6c826
    • Guido Trotter's avatar
      Increase SharedLock fairness · 4d686df8
      Guido Trotter authored
      Previously if a shared thread was notified, together with the rest, and was not
      fast enough in waking up and acquiring the lock, another one could release it,
      decide there were no more sharers, and let an exclusive one in instead. With
      this patch we make sure all the shared holders which were waiting have passed,
      before declaring it's time to make an exclusive one pass.
      
      This also allows us to reintroduce a slight variation of the assertion removed
      in r665, which makes our code safer.
      
      Reviewed-by: imsnah
      4d686df8
  9. Mar 04, 2008
    • Guido Trotter's avatar
      LockSet: handle empty case · b2dabfd6
      Guido Trotter authored
      A LockSet is mostly useful when it has some locks in it. On the other hand
      there are cases in which it must function even when empty. For example if a
      cluster has no instances in it there's no reason why locking all of them
      shouldn't work anyway. This patch adds test code for that situation and
      implements the necessary fixes to make it work.
      
      Reviewed-by: imsnah
      
      b2dabfd6
    • Guido Trotter's avatar
      LockSet: collapse two try/except into one · ea3f80bf
      Guido Trotter authored
      Reviewed-by: imsnah
      
      ea3f80bf
    • Guido Trotter's avatar
      SharedLock: remove wrong assertion in code · 9a39f854
      Guido Trotter authored
      r644 contained some cleanup code for LockSet. Among other things it removed a
      syntax error that allowed an assertion that previously wan't really checked to
      trigger. It turns out that even though the spirit of that assertion was correct
      its actual implementation was wrong.
      
      While it's true that no sharers must be waiting if an exclusive holder is not
      present it might happen that when all the sharers wake up one of them releases
      the lock before some other even has had a chance to run. In this case
      __shr_wait would still be greater than 0, even if the sharer is not actually
      waiting, just pending a wakeup to proceed.
      
      Thus, removing the assertion in question.
      
      Reviewed-by: imsnah
      9a39f854
    • Michael Hanselmann's avatar
      Codestyle updates for locking code · cdb08f44
      Michael Hanselmann authored
      Reviewed-by: ultrotter
      cdb08f44
    • Guido Trotter's avatar
      LockSet: make acquire() able to get the whole set · 3b7ed473
      Guido Trotter authored
      This new functionality makes it possible to acquire a whole set, by passing
      "None" to the acquire() function as the list of elements. This will avoid new
      additions to the set, and then acquire all the current elements. The list of
      all elements acquired will be returned at the end.
      
      Deletions can still happen during the acquire process and we'll deal with it by
      just skipping the deleted elements: it's effectively as if they were deleted
      before we called the function. After we've finished though we hold all the
      elements, so no more deletes can be performed before we release them.
      
      Any call to release() will then first of all release the "set-level" lock if
      we're holding it, and then all or some of the locks we have.
      
      Some new tests checks that this feature works as intended.
      
      Reviewed-by: imsnah
      
      3b7ed473
    • Guido Trotter's avatar
      LockSet: encapsulate acquire() in try-except · 806e20fd
      Guido Trotter authored
      This patch adds a try/except area around most of the acquire() code (everything
      after the intial condition checks). Since the except: clause contains just a
      'raise' nothing really changes except the indentation of the code.
      
      This is done in a separate commit to insulate and make clearer what the real
      code changes done in the upcoming patch are.
      
      Reviewed-by: imsnah
      
      806e20fd
    • Guido Trotter's avatar
      Make LockSet.__names() return a list, not a set · 0cf257c5
      Guido Trotter authored
      Previously the private version of the __names function returned directly a set.
      We'll keep this in the public interface but change the private version to a
      list in order to be able to sort() its result and then loop on it, even though
      we'll need to do this with the usual care that some keys may disappear in
      between.
      
      Reviewed-by: imsnah
      
      0cf257c5
    • Guido Trotter's avatar
      LockSet: improve remove() api · 3f404fc5
      Guido Trotter authored
      Lockset's remove() function used to return a list of locks we failed to remove.
      Rather than doing this we'll return a list of removed locks, so it's more
      similar to how acquire() behaves. This patch also fixes the relevant unit tests.
      
      Reviewed-by: imsnah
      
      3f404fc5
    • Guido Trotter's avatar
      LockSet: make acquire() return the set of names · 0cc00929
      Guido Trotter authored
      In a LockSet acquire() returned True on success. This code changes that to
      return a set containing the names of the elements acquired. This is still a
      true value if we acquired any lock but is slightly more useful (because if
      needed one has access to this data without querying for it). The only change
      happens if acquiring no locks, which though is a usage which should not
      normally happen because it has no practical use.
      
      The patch also changes a some tests to check that the new format is respected.
      
      Reviewed-by: imsnah
      
      0cc00929
    • Guido Trotter's avatar
      LockSet: invert try/for nesting in acquire() · 8b68f394
      Guido Trotter authored
      This patch changes nothing to the functionality of a LockSet. Rather than
      trying to do the whole for loop we try each of its steps. This opens the way to
      handle differently a single failure.
      
      Reviewed-by: imsnah
      8b68f394
    • Guido Trotter's avatar
      Initial GanetiLockManager implementation · 7ee7c0c7
      Guido Trotter authored
      Includes some locking-related constants and explanations on how the
      LockManager should be used, the class itself and its test cases.
      
      The class includes:
        - a basic constructor
        - functions to acquire and release lists of locks at the same level
        - functions to add and remove list of locks at modifiable levels
        - dynamic checks against out-of-order acquisitions and other illegal ops
      
      Its testing library checks that the LockManager behaves correctly and that the
      external assumptions it relies on are respected.
      
      Reviewed-by: imsnah
      7ee7c0c7
  10. Feb 28, 2008
    • Guido Trotter's avatar
      LockSet: make acquire() fail faster on wrong locks · e6c200d6
      Guido Trotter authored
      This patch makes acquire() first look up all the locks in the dict and then try
      to acquire them later. The advantage is that if a lockname is already wrong
      since the beginning we won't need to first queue and acquire other locks to
      find this out.
      
      Of course since there is no locking between the two steps a delete() could
      still happen in between, but SharedLocks are safe in this regard and will just
      make the .acquire() operation fail if this unfortunate condition happens.
      
      Since the right way to check if an instance/node exists and make sure it won't
      stop existing after that is acquiring its lock this improves the common case
      (checking for an incorrect name) while not penalizing correctness, or
      performance as would happen if we kept a lock for the whole process.
      
      Reviewed-by: iustinp
      e6c200d6
    • Guido Trotter's avatar
      LockSet implementation and unit tests · aaae9bc0
      Guido Trotter authored
      A LockSet represents locking for a set of resources of the same type. A thread
      can acquire multiple resources at the same time, and release some or all of
      them, but cannot acquire more resources incrementally at different times
      without releasing all of them in between.
      
      Internally a LockSet uses a SharedLock for each resource to be able to grant
      both exclusive and shared acquisition. It also supports safe addition and
      removal of resources at runtime. Acquisitions are ordered alphabetically in
      order to grant them to be deadlock-free. A lot of assumptions about how the
      code interacts are made in order to grant both safety and speed; in order to
      document all of them the code features pretty lenghty comments.
      
      The test suit tries to catch most common interactions but cannot really tests
      tight race conditions, for which we still need to rely on human checking.
      
      This is the second basic building block for the Ganeti Lock Manager. Instance
      and Node locks will be put in LockSets to manage their acquisition and release.
      
      Reviewed-by: imsnah
      aaae9bc0
  11. Feb 19, 2008
    • Guido Trotter's avatar
      Add the delete() operation to SharedLock · a95fd5d7
      Guido Trotter authored
      This new operation lets a lock be cleanly deleted. The lock will be exclusively
      held before deletion, and after it pending and future acquires will raise an
      exception. Other SharedLock operations are modify to deal with delete() and to
      avoid code duplication.
      
      This patch also adds unit testing for the new function and its interaction with
      the other lock features. The helper threads are sligtly modified to handle and
      report the condition of a deleted lock. As a bonus a non-related unit test
      about not supporting non-blocking mode yet has been added as well.
      
      This feature will be used by the LockSet in order to support deadlock-free
      delete of resources. This in turn will be useful to gracefully handle the
      removal of instances and nodes from the cluster dealing with the fact that
      other operations may be pending on them.
      
      Reviewed-by: iustinp
      a95fd5d7
  12. Feb 18, 2008
  13. Feb 08, 2008
    • Guido Trotter's avatar
      Shared Lock implementation and unit tests. · 162c1c1f
      Guido Trotter authored
      Adding a locking.py file for the ganeti locking library. Its first component is
      the implementation of a non-recursive blocking shared lock complete with a
      testing library.
      
      Reviewed-by: imsnah, iustinp
      162c1c1f
Loading