Merge pull request #148 from tharropoulos/fix/144-mark-answering-node

fix: mark the node that answered as healthy (#144)

Author: tharropoulos

src/typesense/async_/api_call.py

@@ -487,6 +487,7 @@ async def _execute_request(
         try:
             return await self._make_request_and_process_response(
                 method,
+                node,
                 url,
                 entity_type,
                 as_json,
@@ -511,12 +512,13 @@ async def _execute_request(
     async def _make_request_and_process_response(
         self,
         method: str,
+        node: Node,
         url: str,
         entity_type: typing.Type[TEntityDict],
         as_json: bool,
         **kwargs: typing.Unpack[SessionFunctionKwargs[TParams, TBody]],
     ) -> typing.Union[TEntityDict, str]:
-        """Make the async API request and process the response."""
+        """Make the async API request to `node` and process the response."""
         request_response = await self.request_handler.make_request(
             method=method,
             url=url,
@@ -525,10 +527,7 @@ async def _make_request_and_process_response(
             client=self._client,
             **kwargs,
         )
-        self.node_manager.set_node_health(
-            self.node_manager.get_node(),
-            is_healthy=True,
-        )
+        self.node_manager.set_node_health(node, is_healthy=True)
         return (
             typing.cast(TEntityDict, request_response)
             if as_json

src/typesense/sync/api_call.py

@@ -487,6 +487,7 @@ def _execute_request(
         try:
             return self._make_request_and_process_response(
                 method,
+                node,
                 url,
                 entity_type,
                 as_json,
@@ -511,12 +512,13 @@ def _execute_request(
     def _make_request_and_process_response(
         self,
         method: str,
+        node: Node,
         url: str,
         entity_type: typing.Type[TEntityDict],
         as_json: bool,
         **kwargs: typing.Unpack[SessionFunctionKwargs[TParams, TBody]],
     ) -> typing.Union[TEntityDict, str]:
-        """Make the async API request and process the response."""
+        """Make the async API request to `node` and process the response."""
         request_response = self.request_handler.make_request(
             method=method,
             url=url,
@@ -525,10 +527,7 @@ def _make_request_and_process_response(
             client=self._client,
             **kwargs,
         )
-        self.node_manager.set_node_health(
-            self.node_manager.get_node(),
-            is_healthy=True,
-        )
+        self.node_manager.set_node_health(node, is_healthy=True)
         return (
             typing.cast(TEntityDict, request_response)
             if as_json

tests/api_call_test.py

@@ -742,3 +742,73 @@ async def test_async_client_side_error_does_not_mark_node_unhealthy(
         assert not node0_route.called
 
     assert fake_async_api_call.config.nearest_node.healthy is True
+
+
+def test_round_robin_visits_each_node_in_turn(fake_api_call: ApiCall) -> None:
+    """Test that successful requests advance the round-robin by one node each."""
+    fake_api_call.config.nearest_node = None
+
+    with respx.mock:
+        for host in ("node0", "node1", "node2"):
+            respx.get(f"http://{host}:8108/").mock(
+                return_value=httpx.Response(200, json={"key": "value"}),
+            )
+
+        for _ in range(6):
+            fake_api_call.get("/", entity_type=typing.Dict[str, str])
+
+        assert [str(call.request.url) for call in respx.calls] == [
+            "http://node0:8108/",
+            "http://node1:8108/",
+            "http://node2:8108/",
+            "http://node0:8108/",
+            "http://node1:8108/",
+            "http://node2:8108/",
+        ]
+
+
+async def test_async_round_robin_visits_each_node_in_turn(
+    fake_async_api_call: AsyncApiCall,
+) -> None:
+    """Test that successful requests advance the round-robin by one node each (async)."""
+    fake_async_api_call.config.nearest_node = None
+
+    with respx.mock:
+        for host in ("node0", "node1", "node2"):
+            respx.get(f"http://{host}:8108/").mock(
+                return_value=httpx.Response(200, json={"key": "value"}),
+            )
+
+        for _ in range(6):
+            await fake_async_api_call.get("/", entity_type=typing.Dict[str, str])
+
+        assert [str(call.request.url) for call in respx.calls] == [
+            "http://node0:8108/",
+            "http://node1:8108/",
+            "http://node2:8108/",
+            "http://node0:8108/",
+            "http://node1:8108/",
+            "http://node2:8108/",
+        ]
+
+
+def test_success_marks_only_the_answering_node_healthy(
+    fake_api_call: ApiCall,
+) -> None:
+    """Test that a success refreshes the node that answered and no other."""
+    fake_api_call.config.nearest_node = None
+    answering_node, unhealthy_node, _ = fake_api_call.node_manager.nodes
+    answering_node.last_access_ts = 0
+    unhealthy_node.healthy = False
+    unhealthy_node.last_access_ts = int(time.time())
+
+    with respx.mock:
+        respx.get("http://node0:8108/").mock(
+            return_value=httpx.Response(200, json={"key": "value"}),
+        )
+
+        fake_api_call.get("/", entity_type=typing.Dict[str, str])
+
+    assert answering_node.healthy is True
+    assert answering_node.last_access_ts > 0
+    assert unhealthy_node.healthy is False