Failing to create table with sstable_compression=ZstdCompressor

[fruch]

Scylla version 2025.1.0~dev-0.20250122.a8805c4fc184 with build-id 3555be198546b70f36699ff8733c67420ba66881

creating a table with WITH compression = {'sstable_compression': 'ZstdCompressor'} fails on master:

E   cassandra.cluster.NoHostAvailable: ('Unable to complete the operation against any hosts', {<Host: 127.0.6.1:9042 datacenter1>: <Error from server: code=0000 [Server error] message="unable to find class 'org.apache.cassandra.io.compress.ZstdCompressor'">, <Host: 127.0.6.2:9042 datacenter1>: <Error from server: code=0000 [Server error] message="unable to find class 'org.apache.cassandra.io.compress.ZstdCompressor'">, <Host: 127.0.6.3:9042 datacenter1>: <Error from server: code=0000 [Server error] message="unable to find class 'org.apache.cassandra.io.compress.ZstdCompressor'">})

reproducer with dtest (seems like we don't have such test anywhere):

diff --git a/putget_test.py b/putget_test.py
index 2ed62b92..9f97820a 100644
--- a/putget_test.py
+++ b/putget_test.py
@@ -42,6 +42,10 @@ class TestPutGet(Tester):
         """Simple put/get on a single row, but hitting multiple sstables (with deflate compression)"""
         self._putget(compression="Deflate")

+    def test_putget_zstd(self):
+        """Simple put/get on a single row, but hitting multiple sstables (with snappy compression)"""
+        self._putget(compression="Zstd")
+
     # Simple queries, but with flushes in between inserts to make sure we hit
     # sstables (and more than one) on reads
     def _putget(self, compression=None):

# fails
./scripts/run_test.sh --scylla-version=unstable/master:latest putget_test.py::TestPutGet::test_putget_zstd
...
=========================== short test summary info ============================
FAILED putget_test.py::TestPutGet::test_putget_zstd - cassandra.cluster.NoHos...
============================== 1 failed in 33.47s ==============================
Docker exitcode: 1

# works on release 6.2 and 2024.2

./scripts/run_test.sh --scylla-version=release:6.2 putget_test.py::TestPutGet::test_putget_zstd
./scripts/run_test.sh --scylla-version=release:2024.2 putget_test.py::TestPutGet::test_putget_zstd

...
putget_test.py::TestPutGet::test_putget_zstd PASSED

============================== 1 passed in 46.65s ==============================
Docker exitcode: 0

[fruch]

I think it's related to change come in as part of source available, but I could track it down myself as why ZstdCompressor isn't registered

[mykaul]

@fruch - can we do a bisect to find out? It may be from the advanced compression, but it is strange (that was for networking, not for storage)
@michoecho - any idea?

[tchaikov]

i am able to reproduce with release build, but not with debug build. looking.

[michoecho]

ZstdCompressor is looked up in a class_registry of compressors, by the compressor_registry::create() call in compress.cc. (N.b. this registry contains only ZstdCompressor, other compressors are looked up directly in compressor::create).

This means that ZstdCompressor becomes visible only after the static initialization (the const class_registrator<compressor, zstd_processor, const compressor::opt_getter&> registrator(COMPRESSOR_NAME);) in zstd.o runs, and registers ZstdCompressor in the registry.

But no symbol in zstd.o is explicitly used by the rest of the codebase. They are only used implicitly, via this static initialization code which causes it to register itself at runtime. So it's prone to being garbage-collected (/ignored) by the linker.

Hypothesis: zstd.o is not picked up by the linker. Likely solution: link it with --whole-archive. (Long-term solution: get rid of the class_registrator, and look up ZstdCompressor directly. ZstdCompressor is not special enough to have its own fancy linking mechanism).

[tchaikov]

it's related to the order of static variables. cooking a patch now.

[tchaikov]

Hypothesis: zstd.o is not picked up by the linker. Likely solution: link it with --whole-archive.

it is linked w/ whole-archive.

(Long-term solution: get rid of the class_registrator, and look up ZstdCompressor directly. ZstdCompressor is not special enough to have its own fancy linking mechanism).

yeah, i concur with you. but i see there were some back and forth in this area.

[fruch]

I've found this issue with:
scylladb/scylla-cluster-tests#9902

it's a side test, use to check SCT machinery, and cassandra-stress working as expected. and we aren't running it regularly.

we should have a test for this feature in test.py or in dtest

[mykaul]

I've found this issue with: scylladb/scylla-cluster-tests#9902

it's a side test, use to check SCT machinery, and cassandra-stress working as expected. and we aren't running it regularly.

we should have a test for this feature in test.py or in dtest

I'd be very surprised if we don't and we indeed need.