fix: proper handling of retried removals

benchmarks/core/network.py

@@ -57,8 +57,11 @@ class Node(ABC, Generic[TNetworkHandle, TInitialMetadata]):
         pass
 
     @abstractmethod
-    def remove(self, handle: TNetworkHandle):
+    def remove(self, handle: TNetworkHandle) -> bool:
         """Removes the file associated with the handle from this node. For seeders, this means the node will stop
         seeding it. For leechers, it will stop downloading it. In both cases, the file will be removed from the node's
-        storage."""
+        storage.
+
+        :return: True if the file exists and was successfully removed, False if the file didn't exit.
+        """
         pass

benchmarks/deluge/deluge_node.py

@@ -9,14 +9,20 @@ from typing import List, Optional, Self, Dict, Any
 
 import pathvalidate
 from deluge_client import DelugeRPCClient
-from tenacity import retry, wait_exponential, stop_after_attempt
+from deluge_client.client import RemoteException
+from tenacity import (
+    retry,
+    wait_exponential,
+    stop_after_attempt,
+    retry_if_not_exception_type,
+)
 from tenacity.stop import stop_base
 from tenacity.wait import wait_base
 from torrentool.torrent import Torrent
 from urllib3.util import Url
 
 from benchmarks.core.experiments.experiments import ExperimentComponent
-from benchmarks.core.network import DownloadHandle
+from benchmarks.core.network import DownloadHandle, Node
 from benchmarks.core.utils import await_predicate
 from benchmarks.deluge.agent.client import DelugeAgentClient
 
@@ -32,7 +38,7 @@ class DelugeMeta:
     announce_url: Url
 
 
-class DelugeNode(ExperimentComponent):
+class DelugeNode(Node[Torrent, DelugeMeta], ExperimentComponent):
     def __init__(
         self,
         name: str,
@@ -110,7 +116,20 @@ class DelugeNode(ExperimentComponent):
         )
 
     def remove(self, handle: Torrent):
-        self.rpc.core.remove_torrent(handle.info_hash, remove_data=True)
+        try:
+            self.rpc.core.remove_torrent(handle.info_hash, remove_data=True)
+            return True
+        except RemoteException as ex:
+            # DelugeRPCClient creates remote exception types dynamically, so there's
+            # actually no way of testing for them other than this.
+            exception_type = str(ex.__class__)
+            if "deluge_client.client.InvalidTorrentError" in exception_type:
+                # This might happen when we retry a failed delete - maybe we got a bad response back,
+                # but the node managed to delete it already.
+                logger.warning(f"Torrent {handle.name} was not found on {self.name}.")
+                return False
+            else:
+                raise ex
 
     def torrent_info(self, name: str) -> List[Dict[bytes, Any]]:
         return list(self.rpc.core.get_torrents_status({"name": name}, []).values())
@@ -161,7 +180,11 @@ class ResilientCallWrapper:
         self.stop_policy = stop_policy
 
     def __call__(self, *args, **kwargs):
-        @retry(wait=self.wait_policy, stop=self.stop_policy)
+        @retry(
+            wait=self.wait_policy,
+            stop=self.stop_policy,
+            retry=retry_if_not_exception_type(RemoteException),
+        )
         def _resilient_wrapper():
             return self.node(*args, **kwargs)
 

benchmarks/deluge/tests/test_deluge_node.py

@@ -73,10 +73,23 @@ def test_should_remove_files(deluge_node1: DelugeNode, tracker: Tracker):
     )
     assert_is_seed(deluge_node1, name="dataset1", size=megabytes(1))
 
-    deluge_node1.remove(torrent)
+    assert deluge_node1.remove(torrent)
     assert not deluge_node1.torrent_info(name="dataset1")
 
 
+@pytest.mark.integration
+def test_should_return_false_when_file_does_not_exist(
+    deluge_node1: DelugeNode, deluge_node2: DelugeNode, tracker: Tracker
+):
+    torrent = deluge_node1.genseed(
+        size=megabytes(1),
+        seed=1234,
+        meta=DelugeMeta(name="dataset1", announce_url=tracker.announce_url),
+    )
+
+    assert not deluge_node2.remove(torrent)
+
+
 class FlakyClient:
     def __init__(self):
         self.count = 0