Fix race in test_lost_merge_requests

At the end of this test, we check the merger api cache to see if the
merge requset is cleaned up.  We may need to give the cache time to
update, so use iterate_timeout.

We should also be consistent about which merger api object we use;
to that end, the test is updated to only use the api object owned by
the merger client when checking the cache contents (since that's the
one that we use to run the cleanup method).

Finally, it appears this was subject to the same issue that was fixed
in 4f96125007 where we were not handling
deleted znodes correctly.

Change-Id: Ic39b01b2d2eb138cb79c7f5b412c6390c07112bf
This commit is contained in:
James E. Blair 2021-08-07 10:35:05 -07:00 committed by Clark Boylan
parent 14af6950b7
commit d783aaa119
2 changed files with 10 additions and 12 deletions

View File

@ -21,7 +21,6 @@ import git
import testtools
from zuul.merger.merger import MergerTree, Repo
from zuul.zk.merger import MergerApi
import zuul.model
from zuul.model import MergeRequest
from tests.base import (
@ -1020,9 +1019,11 @@ class TestMerger(ZuulTestCase):
self.executor_server.merger_loop_wake_event.set()
self.executor_server.merger_thread.join()
merger_client = self.scheds.first.sched.merger
merger_api = merger_client.merger_api
# Create a fake lost merge request. This is based on
# test_lost_merge_requests in test_zk.
merger_api = MergerApi(self.zk_client)
payload = {'merge': 'test'}
merger_api.submit(MergeRequest(
@ -1058,11 +1059,13 @@ class TestMerger(ZuulTestCase):
self.assertEqual(b.path, lost_merge_requests[0].path)
# Exercise the cleanup code
merger_client = self.scheds.first.sched.merger
self.log.debug("Removing lost merge requests")
merger_client.cleanupLostMergeRequests()
lost_merge_requests = list(merger_api.lostRequests())
self.assertEqual(0, len(lost_merge_requests))
cache = merger_api._cached_requests
for _ in iterate_timeout(30, "cache to be empty"):
if not cache:
break
class TestMergerTree(BaseTestCase):

View File

@ -127,12 +127,7 @@ class JobRequestQueue(ZooKeeperSimpleBase):
return watch
def _watchState(self, path, data, stat, event=None):
if not event or event.type == EventType.CHANGED:
# Don't process change events w/o any data. This can happen when a
# "slow" change watch tried to retrieve the data of a znode that
# was deleted in the meantime.
if not data:
return
if not event or (event.type == EventType.CHANGED and data is not None):
# As we already get the data and the stat value, we can directly
# use it without asking ZooKeeper for the data again.
content = self._bytesToDict(data)
@ -163,7 +158,7 @@ class JobRequestQueue(ZooKeeperSimpleBase):
):
self.request_callback()
elif event.type == EventType.DELETED:
elif (event.type == EventType.DELETED or data is None):
request = self._cached_requests.get(path)
with suppress(KeyError):
del self._cached_requests[path]