Project

General

Profile

Actions

Bug #63305

closed

quincy: osd/scrubber/pg_scrubber.cc: formattable fails to compile on Debain Bullseye

Added by Laura Flores 7 months ago. Updated 6 months ago.

Status:
Resolved
Priority:
Normal
Category:
-
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
quincy
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(RADOS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

https://jenkins.ceph.com/job/ceph-build/566/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=bullseye,DIST=bullseye,MACHINE_SIZE=gigantic/consoleFull

usr/include/fmt/core.h: In instantiation of 'int fmt::v7::detail::check(fmt::v7::detail::unformattable) [with T = std::set<snapid_t>]':
/usr/include/fmt/core.h:1442:18:   required from 'fmt::v7::detail::value<Context> fmt::v7::detail::make_arg(const T&) [with bool IS_PACKED = true; Context = fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<char>, char>; fmt::v7::detail::type <anonymous> = fmt::v7::detail::type::custom_type; T = std::set<snapid_t>; typename std::enable_if<IS_PACKED, int>::type <anonymous> = 0]'
/usr/include/fmt/core.h:1593:64:   required from 'fmt::v7::format_arg_store<Context, Args>::format_arg_store(const Args& ...) [with Context = fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<char>, char>; Args = {const pg_shard_t, const spg_t, const hobject_t, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >}]'
/usr/include/fmt/core.h:1630:18:   required from 'fmt::v7::format_arg_store<fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<Char>, Char>, fmt::v7::remove_reference_t<Args>...> fmt::v7::make_args_checked(const S&, fmt::v7::remove_reference_t<Args>& ...) [with Args = {const pg_shard_t&, const spg_t&, const hobject_t&, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >&, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >&}; S = char [87]; Char = char]'
/usr/include/fmt/core.h:2079:54:   required from 'std::__cxx11::basic_string<Char> fmt::v7::format(const S&, Args&& ...) [with S = char [87]; Args = {const pg_shard_t&, const spg_t&, const hobject_t&, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >&, const std::set<snapid_t, std::less<snapid_t>, std::allocator<snapid_t> >&}; Char = char]'
/build/ceph-17.2.7/src/osd/scrubber/pg_scrubber.cc:1012:50:   required from here
/usr/include/fmt/core.h:1427:7: error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
 1427 |       formattable<T>(),
      |       ^~~~~~~~~~~~~~~~
[ 55%] Building CXX object src/librbd/CMakeFiles/rbd_internal.dir/crypto/openssl/DataCryptor.cc.o

Actions #1

Updated by Laura Flores 7 months ago

This failure was found on an initial build of quincy-release. I reproduced the issue in a debian:bullseye container.

Steps to reproduce:

1. `podman pull debian:bullseye`
2. `podman run -it debian:bullseye`
3. `apt update`
4. `apt install -y git`
5. `git clone https://github.com/ceph/ceph.git --branch=quincy-release`
6. `./install-deps.sh`
7. `./do_cmake.sh`
8. `ninja osd`

Actions #2

Updated by Laura Flores 7 months ago

Reverting https://github.com/ceph/ceph/pull/52256 allowed it to compile.

Actions #3

Updated by Ronen Friedman 7 months ago

I will invest more time in this tomorrow, but please note:
AFAIK, we do not support version 7 of the fmt lib.
See https://github.com/ceph/ceph/pull/48705 which set the min version to 8.something
(the current version, BTW, is 10.1.1)

Actions #4

Updated by Laura Flores 7 months ago

On the quincy-release branch, this diff made `ninja osd` succeed:

diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 3165e4d70fb..12d9ef37c40 100644
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -1004,19 +1004,6 @@ void PgScrubber::apply_snap_mapper_fixes(
          ceph_abort();
        }
       }
-
-      m_osds->clog->error() << fmt::format(
-         "osd.{} found snap mapper error on pg {} oid {} snaps in mapper: {}, " 
-         "oi: " 
-         "{} ...repaired",
-         m_pg_whoami, m_pg_id, hoid, bogus_snaps, snaps);
-
-    } else {
-
-      m_osds->clog->error() << fmt::format(
-         "osd.{} found snap mapper error on pg {} oid {} snaps missing in " 
-         "mapper, should be: {} ...repaired",
-         m_pg_whoami, m_pg_id, hoid, snaps);
     }

     // now - insert the correct snap-set
@@ -1181,19 +1168,11 @@ std::optional<Scrub::snap_mapper_fix_t> PgScrubber::scan_object_snaps(
   }

   if (*cur_snaps == obj_snaps) {
-    dout(20) << fmt::format(
-                 "{}: {}: snapset match SnapMapper's ({})", __func__, hoid,
-                 obj_snaps)
-            << dendl;
     return std::nullopt;
   }

   // add this object to the list of snapsets that needs fixing. Note
   // that we also collect the existing (bogus) list, for logging purposes
-  dout(20) << fmt::format(
-               "{}: obj {}: was: {} updating to: {}", __func__, hoid,
-               *cur_snaps, obj_snaps)
-          << dendl;
   return Scrub::snap_mapper_fix_t{
     snap_mapper_op_t::update, hoid, obj_snaps, *cur_snaps};
 }

I removed four instances of fmt (one by one, compiling each time and removing whatever the compiler didn't like).

Actions #5

Updated by Laura Flores 7 months ago

Here is the commit that adds those four fmt lines: https://github.com/ronen-fr/ceph/commit/d1bde00f0bb15d8da3a50bfee4c1708f3d8bfcdf

The interesting thing is that there are other lines that have fmt in the file, but the compiler is okay with them. So there is something different with the four added fmt lines from that commit.

Actions #6

Updated by Radoslaw Zarzynski 7 months ago

I think the problem is the lack of a formatter but only for std::set<snapid_t>:

/usr/include/fmt/core.h: In instantiation of 'int fmt::v7::detail::check(fmt::v7::detail::unformattable) [with T = std::set<snapid_t>]':

All the debugs format snaps, I believe. This rises 2 questions: * is the formatter for std::set<T> included? I bet it rather is. * Is the formatter for snapid_t included (include/object_fmt.h)?

Actions #7

Updated by Laura Flores 7 months ago

Radek came up with this commit (adding the osd_fmt file), and I tested it on my bullseye env by cherry-picking the commit on top of quincy-release. `ninja osd` works with this change as well:
https://github.com/ceph/ceph/compare/quincy-release...rzarzynski:ceph:wip-bug-63305
https://github.com/ceph/ceph/commit/07c14436f9c626c02af8c381642518c1ca9dffa4
https://github.com/ceph/ceph/pull/54175

diff --git a/src/include/object_fmt.h b/src/include/object_fmt.h
new file mode 100644
index 00000000000..d27821bbea6
--- /dev/null
+++ b/src/include/object_fmt.h
@@ -0,0 +1,30 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+#pragma once
+
+/**
+ * \file fmtlib formatters for some object.h structs
+ */
+#include <fmt/format.h>
+
+#include "object.h" 
+
+
+template <>
+struct fmt::formatter<snapid_t> {
+
+  constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); }
+
+  template <typename FormatContext>
+  auto format(const snapid_t& snp, FormatContext& ctx) const
+  {
+    if (snp == CEPH_NOSNAP) {
+      return fmt::format_to(ctx.out(), "head");
+    }
+    if (snp == CEPH_SNAPDIR) {
+      return fmt::format_to(ctx.out(), "snapdir");
+    }
+    return fmt::format_to(ctx.out(), "{:x}", snp.val);
+  }
+};
+
diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 3165e4d70fb..f4782778651 100644
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -10,6 +10,7 @@
 #include "debug.h" 

 #include "common/errno.h" 
+#include "include/object_fmt.h" 
 #include "messages/MOSDOp.h" 
 #include "messages/MOSDRepScrub.h" 
 #include "messages/MOSDRepScrubMap.h" 
Actions #8

Updated by Laura Flores 7 months ago

  • Pull request ID set to 54175

Direct merge to Quincy.

Actions #9

Updated by Radoslaw Zarzynski 6 months ago

  • Subject changed from osd/scrubber/pg_scrubber.cc: formattable fails to compile on Debain Bullseye to quincy: osd/scrubber/pg_scrubber.cc: formattable fails to compile on Debain Bullseye
  • Status changed from New to Resolved
  • Assignee set to Radoslaw Zarzynski

The patch was targeting quincy directly and now it's merged. Resolving.

Actions

Also available in: Atom PDF