This avoids interfering with the production settings if any.

Forcing a change of the EC profile has catastrophic data availability
consequences, so avoid doing that at all

The monotonic clock is decorrelated with real life, so it can't be
matched to statistics.

We run the benchmark workers in a process pool with fixed pids, so the
files would get clobbered on every worker restart. Instead, append to
the file by outputting a line of zeros to mark the reset.

These tables are append-only and short-lived, there's not much point
using resources to autovacuum them.

Remove the use of unittest.TestCase class.

Replace use of unittest assertions by pytest ones.

Use pytest autouse fixtures instead of setup/teardown methods.

Use tmpdir fixture to manage temporary directories.

Especially the main one in swh/objstorage needs to be removed since this
later is a namespace, in addition to being a package, in which
swh-objstorage-replayer installs itself. So for the namespace importing
mechanism to work properly, it must be an actual namespace (handled by
the NamespaceLoader, not a package backed by the __init__ file).

The idea is to allow using Azure's secondary endpoint
(BlobSecondaryEndpoint) to craft the download url the storage used as
public download URL.

This is needed for the integration test where the endpoint used to add
content in an azure-backend objstorage is different from the public URL
that can be used to download the blob directly from azure (azurite in the
context of integration tests).

Related to swh/meta#5075.

Jérémy Bobbio (Lunar) authored 1 year ago and Nicolas Dandrimont committed 1 year ago

We now allow to delete objects from Winery. The object is first marked
as deleted in the shared base. This makes it inaccessible from further
`get()` calls. If the object still in a RWShard, the relevant line in
the `objects` table is deleted as well.

To take care of removing the data in ROShards, a dedicated host able to
map images in read-write mode can then call:

    swh objstorage winery clean-deleted-objects

This will zero out any deleted object present in the images and remove
their key from the indices. Object keys will then be removed from the
shared database.

Thanks to olasd for proposing several improvements over the initial
submission.

Addresses swh-alter#4

Jérémy Bobbio (Lunar) authored 1 year ago and Nicolas Dandrimont committed 1 year ago

This paves the way to implement object deletion as another status.
(So we can later cleanup the shards in a batch process.)

Thanks to olasd for proposing several improvements over the initial
submission.

Jérémy Bobbio (Lunar) authored 1 year ago and Nicolas Dandrimont committed 1 year ago

As an alternative to running the tests with a Ceph backend, we
implement an alternative image pool based on simple files.
We represent unmapped images by setting their permissions to 0o000.

The tests are modified to run with both the file-backed image pool
and the Ceph pool. Those latter ones will be skipped if Ceph is not
available.

moto v5 rejigged the fixtures in a way that's incompatible with
swh.objstorage, pin it down for now.

Running `test_list_content*` on cloud implementations is pretty useless
as we won’t be listing 17 billion contents from Azure or S3 in a single
thread operation. As each one takes a minute per cloud, we skip these
tests to improve the runtime of the test suite.

The test suite is now reaching the C.I. timeout. In order to reduce the
overall runtime, we now skip testing most compression methods (bzip2,
zlib, lzma) in cloud objstorages by default. Testing all compression
methods can still be done by specifying `--all-compression-methods` on
pytest command-line.

diff --git a/swh/objstorage/tests/conftest.py b/swh/objstorage/tests/conftest.py
index 377778e..664aade 100644
--- a/swh/objstorage/tests/conftest.py
+++ b/swh/objstorage/tests/conftest.py
@@ -1,5 +1,7 @@
 import sys

+import pytest
+

 def pytest_configure(config):
     config.addinivalue_line("markers", "shard_max_size: winery backend")
@@ -14,6 +16,13 @@ def pytest_configure(config):
     config.addinivalue_line(
         "markers", "use_benchmark_flags: use the --winery-bench-* CLI flags"
     )
+    config.addinivalue_line(
+        "markers",
+        (
+            "all_compression_methods: "
+            "test all compression methods instead of only the most common ones"
+        ),
+    )

 def pytest_addoption(parser):
@@ -106,3 +115,15 @@ def pytest_addoption(parser):
         help="Maximum number of bytes per second write",
         default=100 * 1024 * 1024,
     )
+    parser.addoption(
+        "--all-compression-methods",
+        action="store_true",
+        default=False,
+        help="Test all compression methods",
+    )
+
+
+def pytest_runtest_setup(item):
+    if item.get_closest_marker("all_compression_methods"):
+        if not item.config.getoption("--all-compression-methods"):
+            pytest.skip("`--all-compression-methods` has not been specified")
diff --git a/swh/objstorage/tests/test_objstorage_azure.py b/swh/objstorage/tests/test_objstorage_azure.py
index b78e3d4..3679258 100644
--- a/swh/objstorage/tests/test_objstorage_azure.py
+++ b/swh/objstorage/tests/test_objstorage_azure.py
@@ -269,14 +269,17 @@ class TestMockedAzureCloudObjStorageGzip(TestMockedAzureCloudObjStorage):
     compression = "gzip"

+@pytest.mark.all_compression_methods
 class TestMockedAzureCloudObjStorageZlib(TestMockedAzureCloudObjStorage):
     compression = "zlib"

+@pytest.mark.all_compression_methods
 class TestMockedAzureCloudObjStorageLzma(TestMockedAzureCloudObjStorage):
     compression = "lzma"

+@pytest.mark.all_compression_methods
 class TestMockedAzureCloudObjStorageBz2(TestMockedAzureCloudObjStorage):
     compression = "bz2"

diff --git a/swh/objstorage/tests/test_objstorage_cloud.py b/swh/objstorage/tests/test_objstorage_cloud.py
index 49cbd62..1c50850 100644
--- a/swh/objstorage/tests/test_objstorage_cloud.py
+++ b/swh/objstorage/tests/test_objstorage_cloud.py
@@ -148,6 +148,7 @@ class TestCloudObjStorage(ObjStorageTestFixture, unittest.TestCase):
         pass

+@pytest.mark.all_compression_methods
 class TestCloudObjStorageBz2(TestCloudObjStorage):
     compression = "bz2"

@@ -156,10 +157,12 @@ class TestCloudObjStorageGzip(TestCloudObjStorage):
     compression = "gzip"

+@pytest.mark.all_compression_methods
 class TestCloudObjStorageLzma(TestCloudObjStorage):
     compression = "lzma"

+@pytest.mark.all_compression_methods
 class TestCloudObjStorageZlib(TestCloudObjStorage):
     compression = "zlib"

diff --git a/swh/objstorage/tests/test_objstorage_pathslicing.py b/swh/objstorage/tests/test_objstorage_pathslicing.py
index d1f5568..72c3305 100644
--- a/swh/objstorage/tests/test_objstorage_pathslicing.py
+++ b/swh/objstorage/tests/test_objstorage_pathslicing.py
@@ -8,6 +8,8 @@ import tempfile
 import unittest
 from unittest.mock import DEFAULT, patch

+import pytest
+
 from swh.model import hashutil
 from swh.objstorage import exc
 from swh.objstorage.constants import ID_DIGEST_LENGTH
@@ -144,13 +146,16 @@ class TestPathSlicingObjStorageGzip(TestPathSlicingObjStorage):
     compression = "gzip"

+@pytest.mark.all_compression_methods
 class TestPathSlicingObjStorageZlib(TestPathSlicingObjStorage):
     compression = "zlib"

+@pytest.mark.all_compression_methods
 class TestPathSlicingObjStorageBz2(TestPathSlicingObjStorage):
     compression = "bz2"

+@pytest.mark.all_compression_methods
 class TestPathSlicingObjStorageLzma(TestPathSlicingObjStorage):
     compression = "lzma"

Since migration to PEP 517, executing "make test" or "pytest" in the root
directory of the objstorage package triggers the following error:

ImportError: cannot import name 'add_filters' from 'swh.objstorage.multiplexer.filter'

Explicitly setting the testpaths option in pytest.ini and TEST_DIRS
variable in Makefile.local to swh/objstorage/tests fix the issue.

This will allow it to be deployed under a Type=notify service that other
winery components can depend on.

This should allow click to print a nicer error message.

This is probably overkill, but it helped find the shard leakage in the
benchmark code...

This better helps introspect the state of the winery storage when the
benchmark has ended.

This helps distinguish the logs from various processes when running the
benchmark.

Helpful pytest options:
  --log-cli-format='%(asctime)s %(processName)-20s %(levelname)-8s %(name)s:%(filename)s:%(lineno)d %(message)s'
  --log-cli-date-format="%Y-%m-%d %H:%M:%S"

This ensures that shards are properly returned to the STANDBY state once
the writer completes its duties.

In the production deployment, we'll need to find a way to have gunicorn
do that when terminating a worker as well.

A RW storage would get instantiated for all workers, even the
short-lived pack worker, leaving a bunch of lingering WRITING shards and
associated databases.

This avoids a race-condition where the ROShard object is instantiated while the
image is mapped read-write for use by a packer process, which prevents the
image from being remapped read-only.

Introduce a `image_mapped` function in the Pool, that checks whether an
image is mapped, and if so, if it's mapped read-only or read-write.

This uses file permissions on the image path to determine which mode the
shard is mapped into. For this to work, the udev rules must be applied
like so:

  KERNEL=="rbd[0-9]*", ENV{DEVTYPE}=="disk", ACTION=="add", ATTR{device/pool}=="*shards", ATTR{ro}=="0", GROUP="winery-writer", MODE="0664"
  KERNEL=="rbd[0-9]*", ENV{DEVTYPE}=="disk", ACTION=="add", ATTR{device/pool}=="*shards", ATTR{ro}!="0", GROUP="winery-writer", MODE="0444"
  KERNEL=="rbd[0-9]*", ENV{DEVTYPE}=="disk", ACTION=="add", ATTR{device/pool}=="test-winery-ceph-pool", ATTR{ro}=="0", GROUP="winery-writer", MODE="0664"
  KERNEL=="rbd[0-9]*", ENV{DEVTYPE}=="disk", ACTION=="add", ATTR{device/pool}=="test-winery-ceph-pool", ATTR{ro}!="0", GROUP="winery-writer", MODE="0444"

Instead of generating a callback function, just pass the attempt number
to the main function, which avoids needing to keep state across
iterations.

This is more descriptive and matches the corresponding Ceph operation.

This is more appropriately performed by a udev rule such as:

KERNEL=="rbd[0-9]*", ENV{DEVTYPE}=="disk", ACTION=="add", ATTR{device/pool}=="shards", GROUP="winery-writer", MODE="0664"

which will give the winery-writer group write permissions, and other
users read permissions on the images in rbd pool `shards`.

The standalone cleaner detects whether enough hosts have mapped the
image read-only, and deletes the corresponding read-write shard.

This will allow separating the privileges of the tool allowed to manage
RBD images, with the rest of the winery code.