Browse Source

Update node request during locking

After locking a node request we most of the time update ot to double
check if the state changed on us. This is important to not operate on
stale data as we only have a guarantee that the node request data is
updated if we update the data after getting the lock (regardless if we
operate on cached data or not). However it is cumbersome to do this
after every lockNodeRequest call so doing this centrally reduces the
risk to forget that.

This does not introduce any behavior change.

Change-Id: Ie6713fdd01225bbcff7839605ac9f94dc6cb2f78
tags/3.4.0
Tobias Henkel 6 months ago
parent
commit
1d278ffaee
No account linked to committer's email address
2 changed files with 34 additions and 8 deletions
  1. 1
    2
      nodepool/launcher.py
  2. 33
    6
      nodepool/zk.py

+ 1
- 2
nodepool/launcher.py View File

@@ -200,8 +200,7 @@ class PoolWorker(threading.Thread):
200 200
                 continue
201 201
 
202 202
             # Make sure the state didn't change on us after getting the lock
203
-            req2 = self.zk.getNodeRequest(req_id)
204
-            if req2 and req2.state != zk.REQUESTED:
203
+            if req.state != zk.REQUESTED:
205 204
                 self.zk.unlockNodeRequest(req)
206 205
                 continue
207 206
 

+ 33
- 6
nodepool/zk.py View File

@@ -475,13 +475,17 @@ class NodeRequest(BaseModel):
475 475
         '''
476 476
         o = NodeRequest(o_id)
477 477
         super(NodeRequest, o).fromDict(d)
478
-        o.declined_by = d.get('declined_by', [])
479
-        o.node_types = d.get('node_types', [])
480
-        o.nodes = d.get('nodes', [])
481
-        o.reuse = d.get('reuse', True)
482
-        o.requestor = d.get('requestor')
478
+        o.updateFromDict(d)
483 479
         return o
484 480
 
481
+    def updateFromDict(self, d):
482
+        super().fromDict(d)
483
+        self.declined_by = d.get('declined_by', [])
484
+        self.node_types = d.get('node_types', [])
485
+        self.nodes = d.get('nodes', [])
486
+        self.reuse = d.get('reuse', True)
487
+        self.requestor = d.get('requestor')
488
+
485 489
 
486 490
 class Node(BaseModel):
487 491
     '''
@@ -1586,6 +1590,24 @@ class ZooKeeper(object):
1586 1590
         d.stat = stat
1587 1591
         return d
1588 1592
 
1593
+    def updateNodeRequest(self, request):
1594
+        '''
1595
+        Update the data of a node request object in-place
1596
+
1597
+        :param request: the node request object to update
1598
+        '''
1599
+
1600
+        path = self._requestPath(request.id)
1601
+        data, stat = self.client.get(path)
1602
+
1603
+        if data:
1604
+            d = self._bytesToDict(data)
1605
+        else:
1606
+            d = {}
1607
+
1608
+        request.updateFromDict(d)
1609
+        request.stat = stat
1610
+
1589 1611
     def storeNodeRequest(self, request, priority="100"):
1590 1612
         '''
1591 1613
         Store a new or existing node request.
@@ -1632,7 +1654,9 @@ class ZooKeeper(object):
1632 1654
         Lock a node request.
1633 1655
 
1634 1656
         This will set the `lock` attribute of the request object when the
1635
-        lock is successfully acquired.
1657
+        lock is successfully acquired. Also this will update the node request
1658
+        with the latest data after acquiring the lock in order to guarantee
1659
+        that it has the latest state if locking was successful.
1636 1660
 
1637 1661
         :param NodeRequest request: The request to lock.
1638 1662
         :param bool blocking: Whether or not to block on trying to
@@ -1662,6 +1686,9 @@ class ZooKeeper(object):
1662 1686
 
1663 1687
         request.lock = lock
1664 1688
 
1689
+        # Do an in-place update of the node request so we have the latest data
1690
+        self.updateNodeRequest(request)
1691
+
1665 1692
     def unlockNodeRequest(self, request):
1666 1693
         '''
1667 1694
         Unlock a node request.

Loading…
Cancel
Save