ceph_assert() vs ceph_assert_always()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matt recently raised the issue of ceph assertions in production code,
which reminded me of Sage's 2016 pr
https://github.com/ceph/ceph/pull/9969 that added a
ceph_assert_always(). the idea was to eventually make ceph_assert()
conditional on NDEBUG to match the behavior of libc's assert(),
leaving ceph_assert_always() as the marked unconditional case

i would love to see this finally happen, but there are some potential risks:

* ceph_assert()s with side effects won't behave as expected in release
builds. assert() documents this same issue at
https://www.man7.org/linux/man-pages/man3/assert.3.html#BUGS. if we
could at least identify these cases, we can switch them to
ceph_assert_always()

* in teuthology, we test the same release builds that we ship to
users. that means teuthology won't catch the code paths that trigger
debug assertions. if those lead to crashes, they could be much harder
to debug without the assertions and backtraces

* conversely, merging pull requests after a successful teuthology run
may introduce new assertions in debug builds. it could be annoying for
developers to track down and fix new assertions after pulling the
latest main or stable release branch

* unused variable warnings in release builds where ceph_assert() was
the only reference. at least the compiler will catch all of these for
us, and [[maybe_unused]] annotations can clear them up


in general, do folks agree that this is a change worth making? if so,
what can we do to mitigate the risks?

if not, how should we handle the use of ceph_assert() vs raw assert()
in new code? should there be some guidance in CodingStyle?

as a half-measure, we might introduce a new ceph_assert_debug() as an
alternative to raw assert(), then convert some existing uses of
ceph_assert() on a case-by-case basis
_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to dev-leave@xxxxxxx



[Index of Archives]     [CEPH Users]     [Ceph Devel]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux