sts-sponsors team mailing list archive

[Björn Tillenius <mp+433941@xxxxxxxxxxxxxxxxxx>]

Björn Tillenius has proposed merging ~bjornt/maas:baseline-test into maas:machine-list-spike.

Commit message:
Add the performance test for the original machine list implementation.

It's meant to be used as a baseline, to which we can compare the other
implementations.

This commit also adds the basic infrastructure for writing the tests.



Requested reviews:
  Alberto Donato (ack)

For more details, see:
https://code.launchpad.net/~bjornt/maas/+git/maas/+merge/433941
-- 
Your team MAAS Committers is subscribed to branch maas:machine-list-spike.

diff --git a/setup.cfg b/setup.cfg
index cb3e5db..b61150e 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -88,6 +88,7 @@ lint_files =
   src/maascli
   src/maasperf
   src/maasserver
+  src/maasspike
   src/maastesting
   src/metadataserver
   src/provisioningserver
diff --git a/src/maasperf/tests/maasspike/__init__.py b/src/maasperf/tests/maasspike/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/src/maasperf/tests/maasspike/__init__.py
diff --git a/src/maasperf/tests/maasspike/test_listing.py b/src/maasperf/tests/maasspike/test_listing.py
new file mode 100644
index 0000000..6b29bd1
--- /dev/null
+++ b/src/maasperf/tests/maasspike/test_listing.py
@@ -0,0 +1,121 @@
+"""Performance tests for the machine listing spike.
+
+All tests that test the performance of the different implementations for the
+machine listing spike should go in this file.
+
+For each implemenation, we should have a test that lists all machines, and
+a test that list 50 machines.
+
+Each test should measure how long it takes to produce the list, and then
+assert that the listing is the same as the original websocket handler.
+"""
+
+import pytest
+
+from maasserver.models import Machine
+from maasserver.websockets.handlers.machine import MachineHandler
+from maasspike import baseline
+
+
+class ExpectedMachines:
+    def __init__(self, expected_list):
+        self._expected_list = expected_list
+
+    def assert_list(self, machine_list, limit=None):
+        """Assert that the passed in machine list is correct.
+
+        Compare the number of machines in each list, as well as making
+        sure that the first and last machine of each list are equal.
+
+        Comparing the full list takes too long if you have a list of
+        1000 machines.
+        """
+        assert isinstance(machine_list, list)
+        expected_list = (
+            self._expected_list[:limit] if limit else self._expected_list
+        )
+        assert len(machine_list) == len(expected_list)
+        assert machine_list[0] == expected_list[0]
+        assert machine_list[-1] == expected_list[-1]
+
+
+def get_expected_machines(admin):
+    """Get the list of machines that the normal websocket handler returns."""
+    machine_count = Machine.objects.all().count()
+    ws_handler = MachineHandler(admin, {}, None)
+    params = {
+        "filter": {},
+        "page_number": 1,
+        "page_size": machine_count + 1,
+        "sort_direction": "ascending",
+        "sort_key": "hostname",
+    }
+    result = ws_handler.list(params)
+    return ExpectedMachines(result["groups"][0]["items"])
+
+
+@pytest.fixture(scope="session")
+def _expected():
+    """Helper fixture to store the expected machine list for the session.
+
+    A session fixture doesn't have access to the DB, so we make use of a
+    function fixture, expected_machines, to get the machine listing
+    for the first test and store it here.
+
+    The fixture shouldn't be used by any test.
+    """
+    return {}
+
+
+@pytest.fixture
+def expected_machines(admin, _expected):
+    if "machines" not in _expected:
+        _expected["machines"] = get_expected_machines(admin)
+    return _expected["machines"]
+
+
+def test_populate_expected_machines(expected_machines):
+    """A blank test to populate the expected_machines fixture.
+
+    This should be the first test to be run, so that other test
+    don't get any advantage by having the query to the machine
+    listing cached.
+    """
+
+
+@pytest.mark.parametrize("limit", [None, 50])
+class TestListing:
+    """Collection of tests for spike machine listing implementations.
+
+    A class is used to group the tests together to ensure they measure
+    and assert the same thing.
+
+    Each implementation should have a test that does the required setup
+    and then call self.run_listing_test.
+
+    Each test is run once using the full listing, and once using the first
+    50 machines.
+    """
+
+    @pytest.fixture(autouse=True)
+    def set_up(self, perf, admin, expected_machines):
+        self._expected_machines = expected_machines
+        self._admin = admin
+        self._perf = perf
+        yield
+        self._expected_machines = None
+        self._admin = None
+        self._perf = None
+
+    def run_listing_test(self, name, func, limit):
+        record_name = name
+        if limit:
+            record_name += f"_{limit}"
+        else:
+            record_name += "_all"
+        with self._perf.record(record_name):
+            machines = func(self._admin, limit)
+        self._expected_machines.assert_list(machines, limit)
+
+    def test_baseline(self, limit):
+        self.run_listing_test("baseline", baseline.list_machines, limit)
diff --git a/src/maasspike/__init__.py b/src/maasspike/__init__.py
new file mode 100644
index 0000000..5476dfd
--- /dev/null
+++ b/src/maasspike/__init__.py
@@ -0,0 +1,7 @@
+"""This package contains experimental code for the websocket machine listing.
+
+We're doing a spike on how to improve the rendering time of the machine
+listing in the UI. We keep the code in a separate package to make it
+clear what's part of the spike, and any improvements we make that
+can be merged back to master.
+"""
diff --git a/src/maasspike/baseline.py b/src/maasspike/baseline.py
new file mode 100644
index 0000000..f3a6d84
--- /dev/null
+++ b/src/maasspike/baseline.py
@@ -0,0 +1,34 @@
+from maasserver.websockets.handlers.machine import MachineHandler
+
+
+class NoPaginationMachineHandler(MachineHandler):
+    """A machine handler that returns only the machines.
+
+    It doesn't do any grouping, searching, or other things
+    (like caching the ids) which might slow it down.
+
+    The purpose of this is to use our current implementation as a baseline
+    for other implementations to beat. We reuse the current websocket
+    handler as much as possible, so that any improvements to it will
+    be shown here as well.
+    """
+
+    def list(self, params):
+        qs = self.get_queryset(for_list=True)
+        qs = self._sort(qs, "list", params)
+        limit = params.get("limit")
+        if limit:
+            qs = qs[:limit]
+        objs = list(qs)
+        # This is needed to calculate the script result summaries.
+        # It's quite expensive.
+        self._cache_script_results(objs)
+        return [self.full_dehydrate(obj, for_list=True) for obj in objs]
+
+
+def list_machines(admin, limit=None):
+    ws_handler = NoPaginationMachineHandler(admin, {}, None)
+    params = {}
+    if limit:
+        params["limit"] = limit
+    return ws_handler.list(params)