Skip to content

Commit

Permalink
Return secondary nodes too on cluster members endpoint (#1723)
Browse files Browse the repository at this point in the history
  • Loading branch information
lferran authored Jan 12, 2024
1 parent efd32a9 commit ed080cf
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 8 deletions.
5 changes: 4 additions & 1 deletion nucliadb/nucliadb/common/cluster/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ def __init__(
self.primary_id = primary_id

def __str__(self):
return f"{self.__class__.__name__}({self.id}, {self.address})"
if self.primary_id is None:
return f"{self.__class__.__name__}({self.id}, {self.address})"
else:
return f"{self.__class__.__name__}({self.id}, {self.address}, primary_id={self.primary_id})"

def __repr__(self):
return self.__str__()
Expand Down
7 changes: 5 additions & 2 deletions nucliadb/nucliadb/common/cluster/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@
READ_REPLICA_INDEX_NODES: dict[str, set[str]] = {}


def get_index_nodes() -> list[AbstractIndexNode]:
return [inode for inode in INDEX_NODES.values() if inode.primary_id is None]
def get_index_nodes(include_secondary: bool = False) -> list[AbstractIndexNode]:
all_nodes = [inode for inode in INDEX_NODES.values()]
if not include_secondary:
return [inode for inode in all_nodes if inode.primary_id is None]
return all_nodes


def get_index_node(node_id: str) -> Optional[AbstractIndexNode]:
Expand Down
3 changes: 2 additions & 1 deletion nucliadb/nucliadb/search/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ async def node_members(request: Request) -> JSONResponse:
"type": node.label,
"shard_count": node.shard_count,
"dummy": node.dummy,
"primary_id": node.primary_id,
}
for node in manager.get_index_nodes()
for node in manager.get_index_nodes(include_secondary=True)
]
)

Expand Down
28 changes: 24 additions & 4 deletions nucliadb/nucliadb/search/tests/unit/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,54 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import json
from unittest.mock import patch

import pytest

from nucliadb.common.cluster.index_node import IndexNode
from nucliadb.search import app

pytestmark = pytest.mark.asyncio


async def test_alive():
with patch.object(app.manager, "get_index_nodes", return_value={"node1": "node1"}):
with patch.object(app.manager, "get_index_nodes", return_value=[{"id": "node1"}]):
resp = await app.alive(None)
assert resp.status_code == 200


async def test_not_alive():
with patch.object(app.manager, "get_index_nodes", return_value={}):
with patch.object(app.manager, "get_index_nodes", return_value=[]):
resp = await app.alive(None)
assert resp.status_code == 503


async def test_ready():
with patch.object(app.manager, "get_index_nodes", return_value={"node1": "node1"}):
with patch.object(app.manager, "get_index_nodes", return_value=[{"id": "node1"}]):
resp = await app.ready(None)
assert resp.status_code == 200


async def test_not_ready():
with patch.object(app.manager, "get_index_nodes", return_value={}):
with patch.object(app.manager, "get_index_nodes", return_value=[]):
resp = await app.ready(None)
assert resp.status_code == 503


async def test_node_members():
nodes = [
IndexNode(id="node1", address="node1", shard_count=0, dummy=True),
IndexNode(
id="node2", address="node2", shard_count=0, dummy=True, primary_id="node1"
),
]
with patch.object(app.manager, "get_index_nodes", return_value=nodes):
resp = await app.node_members(None)
assert resp.status_code == 200
members = json.loads(resp.body)
sorted(members, key=lambda x: x["id"])
assert members[0]["id"] == "node1"
assert members[0]["primary_id"] is None
assert members[1]["id"] == "node2"
assert members[1]["primary_id"] == "node1"
46 changes: 46 additions & 0 deletions nucliadb/nucliadb/tests/unit/common/cluster/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
import asyncio
from unittest import mock
from unittest.mock import MagicMock

import pytest
Expand Down Expand Up @@ -174,3 +175,48 @@ def test_choose_node():
)

manager.INDEX_NODES.clear()


@pytest.fixture(scope="function")
def standalone_mode_off():
prev = settings.standalone_mode
settings.standalone_mode = False
yield
settings.standalone_mode = prev


@pytest.fixture(scope="function")
def index_nodes():
index_nodes = {}
with mock.patch.object(manager, "INDEX_NODES", new=index_nodes):
yield index_nodes


def test_get_index_nodes(standalone_mode_off, index_nodes):
# Add a primary node
manager.add_index_node(
id="node-0",
address="nohost",
shard_count=0,
dummy=True,
)
# Add a secondary replica of node-0
manager.add_index_node(
id="node-1",
address="nohost",
shard_count=0,
dummy=True,
primary_id="node-0",
)

# By default, only primary nodes are returned
nodes = manager.get_index_nodes()
assert len(nodes) == 1
assert nodes[0].id == "node-0"

# If we ask for secondary, we get both
nodes = manager.get_index_nodes(include_secondary=True)
assert len(nodes) == 2
sorted(nodes, key=lambda x: x.id)
assert nodes[0].id == "node-0"
assert nodes[1].id == "node-1"

3 comments on commit ed080cf

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: ed080cf Previous: 5a633b0 Ratio
nucliadb/search/tests/unit/search/test_fetch.py::test_highligh_error 12791.174876785648 iter/sec (stddev: 1.003630886236973e-7) 12745.686329086004 iter/sec (stddev: 1.7317806991721728e-7) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: ed080cf Previous: 5a633b0 Ratio
nucliadb/search/tests/unit/search/test_fetch.py::test_highligh_error 12830.004187372868 iter/sec (stddev: 0.000001744922733126715) 12745.686329086004 iter/sec (stddev: 1.7317806991721728e-7) 0.99

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: ed080cf Previous: 5a633b0 Ratio
nucliadb/search/tests/unit/search/test_fetch.py::test_highligh_error 13129.559224315117 iter/sec (stddev: 8.453819611959503e-7) 12745.686329086004 iter/sec (stddev: 1.7317806991721728e-7) 0.97

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.