Build on Raspberry Pi 4 fails

[Talkabout]

Describe the bug
Building on Raspberry Pi 4 fails

** Log Files **
In file included from server.cpp:31:
server.h:831:35: error: static assertion failed: object size is critical, don't increase
static_assert(sizeof(redisObject) == 16, "object size is critical, don't increase");

To Reproduce
Quite easy, simply checkout tag 6.0.16 and try to build it.

Bye

[Talkabout]

Hi all,

commented that line and keydb has built successfully. Everything seems to work, hopefully there won't be any side effects. Would be nice though if this issue could be fixed.

Thanks!

[JohnSully]

Hey @Talkabout my rPi CI system broke after a move and I’ve been using this bug to also track getting it up and running again.

I don’t think you’ll have issues commenting out the line but do run “make test” to ensure your build is clean.

[Talkabout]

Hi @JohnSully ,

I ran "make test" with following result:

!!! WARNING The following tests failed:

*** [err]: Make the old master a replica of the new one and check conditions in tests/integration/psync2-pingoff.tcl
Expected '185' to be equal to '199' (context: type eval line 18 cmd {assert_equal [status $R(0) master_repl_offset] [status $R(1) master_repl_offset]} proc ::test)
Cleanup: may take some time... OK

Is this error related to that line? If not, do I need to worry?

Thanks!

Bye

[Talkabout]

On a different RPI "make test" showed the following results:

!!! WARNING The following tests failed:

*** [err]: Connect a replica to the master instance (commands replication) in tests/unit/scripting.tcl
Can't turn the instance into a replica
Cleanup: may take some time... OK

Something to worry about?

Thanks!

[JohnSully]

These could be timing related failures due to the limited cpu power. Are you running on an rPi 4?

[Talkabout]

Yes, both these are rPi 4, but with some other processes running also, that consume some CPU (Unbound, Freeradius, Apache, Nginx...)

[JohnSully]

I will get my rPis back up and validate. I remember I also had to reduce the number of test clients to make them more reliable.

Something like ./runtest —clients 2

Since your failures are timing related I’m not immediately worried ARM is broken except the line you fixed, but I should be able to give a firm answer shortly.

[Talkabout]

Thanks @JohnSully! Will keep keydb running, currently I am not seeing any issues.

[b-hyl]

The issue is that the default integer sizes for 32-bit (Raspbian is 32-bit) are smaller than x86_64.

See the following snippet I hashed out to get it working locally:

--- a/src/server.h
+++ b/src/server.h
@@ -828,7 +828,11 @@ public:
     void addref() const { refcount.fetch_add(1, std::memory_order_relaxed); }
     unsigned release() const { return refcount.fetch_sub(1, std::memory_order_seq_cst) & ~(1U << 31); }
 } robj;
+#ifdef ARM32
+static_assert(sizeof(redisObject) == 12, "object size is critical, don't increase");
+#else
 static_assert(sizeof(redisObject) == 16, "object size is critical, don't increase");
+#endif
 
 class redisObjectStack : public redisObjectExtended, public redisObject
 {

I don't think it is tidy enough for a pull request, but you should be able to get the idea.

[JohnSully]

@b-hyl I’m probably just going to do <= here instead of ifdefs. However the main blocker is getting the CI pipeline back up.

If it weren’t for the rPi I’d just deprecate 32-bit altogether.

[Talkabout]

Yes, understood. PLEASE do not deprecate it yet. I hope that Raspbian 64 bit will be stable enough soon so I can migrate, but this seems to be still risky. I am using KeyDB on 2 rPis as Unbound backend (to avoid duplicate queries for a domain) and it works really great. I really wouldn't want this to get lost :)

[JohnSully]

@Talkabout don’t worry I won’t deprecate the rPi. If 64-bit pis become standard and common I may start pushing in that direction. But regardless the Pi as a platform will be supported.

Right now 64-bit rPis are an oddity and I won’t force people down that rabbit hole.

[Talkabout]

@JohnSully great to read that! KeyDB is really a great piece of software! Keep up the good work!

[b-hyl]

+1 for keeping 32-bit support for embedded devices. Upstream Redis seems to be keeping it around for the foreseeable too.

[christianEQ]

Fixed in #343.