diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index bf71aba1f4..6d55299750 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -775,11 +775,7 @@ class GerritConnection(ZKChangeCacheMixin, BaseConnection): change = GerritChange(None) change.number = number change.patchset = patchset - try: - return self._updateChange(key, change, event, history) - except Exception: - self._change_cache.delete(key) - raise + return self._updateChange(key, change, event, history) def _getTag(self, event): tag = event.ref[len('refs/tags/'):] diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 5d749cbae9..3689f8c735 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1308,41 +1308,36 @@ class GithubConnection(ZKChangeCacheMixin, CachedBranchConnection): change.number = number change.patchset = patchset - try: - # This can be called multi-threaded during github event - # preprocessing. In order to avoid data races perform locking - # by cached key. Try to acquire the lock non-blocking at first. - # If the lock is already taken we're currently updating the very - # same chnange right now and would likely get the same data again. - lock = self._change_update_lock.setdefault(key, threading.Lock()) - if lock.acquire(blocking=False): - try: - pull = self.getPull(change.project.name, change.number, - event=event) + # This can be called multi-threaded during github event + # preprocessing. In order to avoid data races perform locking + # by cached key. Try to acquire the lock non-blocking at first. + # If the lock is already taken we're currently updating the very + # same chnange right now and would likely get the same data again. + lock = self._change_update_lock.setdefault(key, threading.Lock()) + if lock.acquire(blocking=False): + try: + pull = self.getPull(change.project.name, change.number, + event=event) - def _update_change(c): - self._updateChange(c, event, pull) + def _update_change(c): + self._updateChange(c, event, pull) - change = self._change_cache.updateChangeWithRetry( - key, change, _update_change) - finally: - # We need to remove the lock here again so we don't leak - # them. - del self._change_update_lock[key] - lock.release() - else: - # We didn't get the lock so we don't need to update the same - # change again, but to be correct we should at least wait until - # the other thread is done updating the change. - log = get_annotated_logger(self.log, event) - log.debug("Change %s is currently being updated, " - "waiting for it to finish", change) - with lock: - log.debug('Finished updating change %s', change) - except Exception: - self.log.warning("Deleting cache key %s due to exception", key) - self._change_cache.delete(key) - raise + change = self._change_cache.updateChangeWithRetry( + key, change, _update_change) + finally: + # We need to remove the lock here again so we don't leak + # them. + del self._change_update_lock[key] + lock.release() + else: + # We didn't get the lock so we don't need to update the same + # change again, but to be correct we should at least wait until + # the other thread is done updating the change. + log = get_annotated_logger(self.log, event) + log.debug("Change %s is currently being updated, " + "waiting for it to finish", change) + with lock: + log.debug('Finished updating change %s', change) return change def _getTag(self, project, event): diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index 4b2e684fc0..62d0641d25 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/zuul/driver/gitlab/gitlabconnection.py @@ -539,21 +539,17 @@ class GitlabConnection(ZKChangeCacheMixin, CachedBranchConnection): change.patchset = patch_number change.url = url or self.getMRUrl(project.name, number) change.uris = [change.url.split('://', 1)[-1]] # remove scheme - try: - log.debug("Getting change mr#%s from project %s" % ( - number, project.name)) - log.info("Updating change from Gitlab %s" % change) - mr = self.getMR(change.project.name, change.number, event=event) - def _update_change(c): - self._updateChange(c, event, mr) + log.debug("Getting change mr#%s from project %s" % ( + number, project.name)) + log.info("Updating change from Gitlab %s" % change) + mr = self.getMR(change.project.name, change.number, event=event) - change = self._change_cache.updateChangeWithRetry(key, change, - _update_change) - except Exception: - self.log.warning("Deleting cache key %s due to exception", key) - self._change_cache.delete(key) - raise + def _update_change(c): + self._updateChange(c, event, mr) + + change = self._change_cache.updateChangeWithRetry(key, change, + _update_change) return change def _updateChange(self, change, event, mr): diff --git a/zuul/driver/pagure/pagureconnection.py b/zuul/driver/pagure/pagureconnection.py index 8d6edeeacd..b211db3e48 100644 --- a/zuul/driver/pagure/pagureconnection.py +++ b/zuul/driver/pagure/pagureconnection.py @@ -617,21 +617,17 @@ class PagureConnection(ZKChangeCacheMixin, BaseConnection): change.uris = [ '%s/%s/pull/%s' % (self.baseurl, project, number), ] - try: - self.log.debug("Getting change pr#%s from project %s" % ( - number, project.name)) - self.log.info("Updating change from pagure %s" % change) - pull = self.getPull(change.project.name, change.number) - def _update_change(c): - self._updateChange(c, event, pull) + self.log.debug("Getting change pr#%s from project %s" % ( + number, project.name)) + self.log.info("Updating change from pagure %s" % change) + pull = self.getPull(change.project.name, change.number) - change = self._change_cache.updateChangeWithRetry(key, change, - _update_change) - except Exception: - self.log.warning("Deleting cache key %s due to exception", key) - self._change_cache.delete(key) - raise + def _update_change(c): + self._updateChange(c, event, pull) + + change = self._change_cache.updateChangeWithRetry(key, change, + _update_change) return change def _getNonPRRef(self, project, event):