ceph.spec.in: bump gcc-toolset to 13 and use it on rhel>=8 #55886

tchaikov · 2024-03-02T04:46:59Z

since gts 13 is out, and the newer the toolchain the merrier. and because clang prefers using a newer gts when its gcc is around. so let's bump up the gcc-toolset from 11 to 13.. both RHEL8 and RHEL9 have gcc-toolset 13, so let's enable it on
rhel>=8, more future-proof this way.

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

athanatos

gcc-13 also seems to have sufficiently good coroutine support to be usable. This seems simpler than switching to clang-16/17.

@rzarzynski @cbodley Does this look ok to you?

tchaikov · 2024-03-02T09:43:08Z

changelog

do not install -annobin subpackage anymore, as it does not exist in gts-13.

tchaikov · 2024-03-02T09:55:02Z

@athanatos Hi Sam, FWIW, Scylladb didn't switch to GCC because of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98056, but it was fixed one year ago. (wanted to add more proof to support your comment at #55825 (comment) )

tchaikov · 2024-03-02T12:19:11Z

-- Performing Test Cxx_Compiler_BZ107852_Free
CMake Error at /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/19.0.0-1808-g79ff8417/rpm/el8/BUILD/ceph-19.0.0-1808-g79ff8417/x86_64-redhat-linux-gnu/CMakeFiles/CMakeScratch/TryCompile-lZ77IE/CMakeLists.txt:26 (target_link_libraries):
  Target "cmTC_2649c" links to:

    fmt::fmt

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

see https://jenkins.ceph.com/job/ceph-dev-new-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=centos8,DIST=centos8,MACHINE_SIZE=gigantic/77261//consoleFull.

strange enough: fmt::fmt was not available when Seastar was checking for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107852

tchaikov · 2024-03-02T14:21:31Z

see also ceph/seastar@d382f24

tchaikov · 2024-03-02T15:41:27Z

i am proposing a solution at ceph/seastar@d382f24#r139292281

athanatos · 2024-03-05T02:36:13Z

Pushed to ci as wip-sjust-testing-2024-03-04

tchaikov · 2024-03-05T04:02:40Z

Pushed to ci as wip-sjust-testing-2024-03-04

@athanatos i was testing it in https://shaman.ceph.com/builds/ceph/wip-pr-55886-kefu/8352c8cc4786283da2db0240841e2e5500699e02/crimson/384525/ as well. but i hit a FTBFS addressed by #55905. probably you could cherrypick #55905 in your branch?

tchaikov · 2024-03-05T13:41:33Z

pushed to https://shaman.ceph.com/builds/ceph/wip-pr-55886-kefu/df11d2889079b3d1d933fe65c0c453b8911011b8/

tchaikov · 2024-03-05T15:13:11Z

now blocked by #55910

cbodley · 2024-03-06T15:28:43Z

@rzarzynski @cbodley Does this look ok to you?

👍👍 would this allow us to drop the 'crimson' build flavor in shaman?

tchaikov · 2024-03-06T15:50:39Z

this is a great question indeed. i think the answer is "YES! but ...".

YES: see the builds at https://shaman.ceph.com/builds/ceph/wip-pr-55886-kefu/df11d2889079b3d1d933fe65c0c453b8911011b8/ .
but: the reason of "crimson" flavor is two-folded:
- compile time dependencies: the last bits are addressed by this change
- runtime problems: ceph-osd versus crimson-osd. we don't have a solution yet to package crimson-osd in a dedicated package. what we have is to package crimson-osd with the name of ceph-osd (@rzarzynski do you still recall the discussion we had on this couple years ago? =))

so i think we cannot retire the crimson flavor yet.

tchaikov · 2024-03-06T15:51:04Z

just pushed the rebased change to https://shaman.ceph.com/builds/ceph/wip-pr-55886-kefu/22ef2c527ade01c8105677709484d3e2fe2b5bbf/

once it's green. i think it's good to go. or we need a better coverage by running the packages with teuthology ?

cbodley · 2024-03-06T16:03:57Z

so i think we cannot retire the crimson flavor yet.

thanks @tchaikov. i've just been feeling the weight of our build matrix lately. the centos8 and ubuntu focal builds should get retired soonish. it would be great if we could find a way to prioritize the crimson part after squid

tchaikov · 2024-03-06T16:31:00Z

i see. yeah, we do need to find a better way to package crimson-osd. sooner or latter, user will need to migrate to crimson osd. and we will probably have a period of time allowing user to opt in crimson osd. so the co-existing of these two packages would be a must have by then.

athanatos · 2024-03-06T21:28:03Z

I think the best way to unify the packages would be to have a single ceph-osd which execs classic-osd or crimson-osd based on a command line flag or config.

gcc-toolset-13 lto can trigger a linker bug resulting in a segfault in SafeTimer (and perhaps elsewhere). See https://tracker.ceph.com/issues/63867 for details. This patch disables lto for now now so that we can switch to gcc-toolset-13. Fixes: https://tracker.ceph.com/issues/63867 Signed-off-by: Samuel Just <sjust@redhat.com>

since gts 13 is out, and GCC-13 brings better support of C++20 coroutines, and because clang prefers using a newer gts when its gcc is around. so let's bump up the gcc-toolset from 11 to 13 when building crimson. because gcc-toolset-13 LTO triggers a linker bug resulting in a segfault in SafeTimer, see https://tracker.ceph.com/issues/63867 , we cannot switch the classic build to gts 13 without proving that it does not incur performance regressions. since annobin plugin package was renamed to gcc-toolset-13-gcc-plugin-annobin, let's update its name accordingly. and use -runtime subpackage instead of -build, as macros.gcc-toolset-13-enable is now located in -runtime subpackage since devtoolset12 Signed-off-by: Kefu Chai <tchaikov@gmail.com>

both RHEL8 and RHEL9 have gcc-toolset 13, and we need to use gts-13 for building crimson, so let's enable it when building crimson, and we need to use gts-11 when building on RHEL7. hence this change. Signed-off-by: Kefu Chai <tchaikov@gmail.com>

tchaikov · 2024-03-11T15:33:00Z

built at https://shaman.ceph.com/builds/ceph/wip-kefu-testing-2024-03-11/913d127c8116ef6f0066d75cfdbba3811939936d/

tchaikov · 2024-03-11T15:34:41Z

being tested at https://pulpito.ceph.com/kchai-2024-03-11_15:32:56-rados-wip-kefu-testing-2024-03-11-distro-default-smithi/

tchaikov · 2024-03-11T15:38:22Z

jenkins test make check

tchaikov · 2024-03-11T16:06:26Z

@athanatos hi Sam, could you take another look? i think it's good to merge now. hopefully, all three tests will pass when you check them.

athanatos · 2024-03-12T01:14:53Z

make check is failing, but for a reason present in other PRs that we're investigating. I'll schedule this one for a crimson run and another 10 run classic run (one of the three failed, though for a reason I've seen elsewhere).

athanatos · 2024-03-12T01:16:02Z

http://pulpito.ceph.com/sjust-2024-03-12_01:14:20-rados-wip-kefu-testing-2024-03-11-distro-default-smithi/
http://pulpito.ceph.com/sjust-2024-03-12_01:15:24-crimson-rados-wip-kefu-testing-2024-03-11-distro-default-smithi/

athanatos · 2024-03-12T01:16:26Z

I think if the above seem ok, it's probably good to merge. @rzarzynski @cbodley final look? The summary of where we are now is that moving to gcc-toolset-13 triggers a linker bug in SafeTimer, so we're now leaving classic builds at 11 by default for centos 8 and 9, using 13 for crimson, and disabling lto for gcc-toolset-13. This should take care of the issues users are seeing with classic on newer distros which default to gcc-toolset-13, allow usage of coroutines in crimson, and leave lto enabled for centos 8 and 9 classic builds for now until we have a gcc-toolset-13 which doesn't have that problem.

https://tracker.ceph.com/issues/63867 is the gcc-toolset-13 ceph linker crash.

tchaikov · 2024-03-12T02:09:07Z

jenkins test make check

athanatos · 2024-03-12T22:08:40Z

https://pulpito.ceph.com/sjust-2024-03-12_01:15:24-crimson-rados-wip-kefu-testing-2024-03-11-distro-default-smithi/ mostly failed. I'm traveling this week, so it may be a bit before I get a chance to look at the logs.

Svelar · 2024-03-13T02:42:56Z

wondering if we can parameterize the ceph-osd systemd service unit, so that it can launch crimson or the classic osd based on user's preference. but we need to persist a mark so user cannot switch to another osd after the the store is initialized. as the disk layouts of crimson and bluestore are different.

Could it be picked up in crimson weekly?

athanatos · 2024-03-13T04:39:06Z

Building wip-sjust-cb2b48d7-2024-03-12 to confirm that the above failures are due to this PR, haven't had a chance to look into the logs yet.

rzarzynski · 2024-03-13T18:13:07Z

Putting DNM due to the comment above.

athanatos · 2024-03-18T19:29:02Z

I think the failures are unrelated to this PR -- main isn't particularly clean at the moment. I'd like to clean up main before merging this though.

athanatos · 2024-03-19T22:18:44Z

~~https://tracker.ceph.com/issues/64996 may be triggered (caused?) by this switch to gcc-13 -- investigating.~~ Nope, not sure why it's popping up now but I can reproduce it on gcc-11 as well.

tchaikov · 2024-03-21T01:50:09Z

wondering if we can parameterize the ceph-osd systemd service unit, so that it can launch crimson or the classic osd based on user's preference. but we need to persist a mark so user cannot switch to another osd after the the store is initialized. as the disk layouts of crimson and bluestore are different.

Could it be picked up in crimson weekly?

i think it might be answered by the attendees =)

tchaikov · 2024-03-21T01:52:09Z

I think the failures are unrelated to this PR -- main isn't particularly clean at the moment. I'd like to clean up main before merging this though.

@athanatos hi Sam, have you started working on this? otherwise i'd like to take a look at main.cc this weekend. but i might be slow.

athanatos · 2024-03-21T02:37:21Z

@tchaikov By main, I meant the crimson-rados suite on ceph/main, not main.cc. I have a set of fixes in a test branch (wip-sjust-crimson-testing-2024-03-20) along with this PR. It finally got through ci, so I just scheduled another suite run. http://pulpito.ceph.com/sjust-2024-03-21_02:35:36-crimson-rados-wip-sjust-crimson-testing-2024-03-20-distro-default-smithi/ Once we get a clean-ish suite run with this PR, we can merge.

tchaikov · 2024-03-21T02:44:18Z

@athanatos ahh, thanks. i misread your comment.

athanatos · 2024-03-21T21:22:15Z

https://pulpito.ceph.com/sjust-2024-03-21_02:35:36-crimson-rados-wip-sjust-crimson-testing-2024-03-20-distro-default-smithi/

Failures unrelated, caused by existing known bugs.

tchaikov added the build/ops label Mar 2, 2024

tchaikov mentioned this pull request Mar 2, 2024

DNM: ceph: default to clang for crimson builds #55825

Draft

tchaikov requested a review from athanatos March 2, 2024 04:48

tchaikov force-pushed the wip-ceph.spec-gcc-13 branch 2 times, most recently from 406516f to dd6015f Compare March 2, 2024 06:27

athanatos approved these changes Mar 2, 2024

View reviewed changes

athanatos requested review from rzarzynski and cbodley March 2, 2024 07:34

tchaikov force-pushed the wip-ceph.spec-gcc-13 branch from dd6015f to 79ff841 Compare March 2, 2024 09:42

rzarzynski approved these changes Mar 2, 2024

View reviewed changes

tchaikov mentioned this pull request Mar 4, 2024

src/seastar: update seastar submodule to fix FTBFS #55897

Merged

14 tasks

Matan-B mentioned this pull request Mar 4, 2024

common/LogEntry.cc: fix clog_type_to_string default return type #55919

Closed

14 tasks

tchaikov mentioned this pull request Mar 5, 2024

common: fix string creation from '0' in LogEntry #55905

Merged

This was referenced Mar 6, 2024

crimson: convert some of client_request to use coroutines #55847

Merged

crimson: add coroutine support for errorated and interruptible futures #55846

Merged

tchaikov force-pushed the wip-ceph.spec-gcc-13 branch from bb5bb0a to 09cde32 Compare March 11, 2024 09:02

athanatos and others added 3 commits March 11, 2024 18:28

tchaikov force-pushed the wip-ceph.spec-gcc-13 branch from 09cde32 to 913d127 Compare March 11, 2024 10:28

athanatos requested a review from rzarzynski March 12, 2024 01:20

rzarzynski added the DNM label Mar 13, 2024

athanatos removed the DNM label Mar 21, 2024

athanatos merged commit dbfb661 into ceph:main Mar 21, 2024
11 checks passed

tchaikov deleted the wip-ceph.spec-gcc-13 branch March 22, 2024 00:31

Svelar mentioned this pull request Apr 25, 2024

rgw/rgw_lua_utils: free std::string #56978

Merged

14 tasks

ceph.spec.in: bump gcc-toolset to 13 and use it on rhel>=8 #55886

ceph.spec.in: bump gcc-toolset to 13 and use it on rhel>=8 #55886

Conversation

tchaikov commented Mar 2, 2024

Contribution Guidelines

Checklist

athanatos left a comment

Choose a reason for hiding this comment

tchaikov commented Mar 2, 2024

tchaikov commented Mar 2, 2024 • edited

tchaikov commented Mar 2, 2024

tchaikov commented Mar 2, 2024

tchaikov commented Mar 2, 2024

athanatos commented Mar 5, 2024

tchaikov commented Mar 5, 2024

tchaikov commented Mar 5, 2024

tchaikov commented Mar 5, 2024

cbodley commented Mar 6, 2024

tchaikov commented Mar 6, 2024 • edited

tchaikov commented Mar 6, 2024 • edited

cbodley commented Mar 6, 2024

tchaikov commented Mar 6, 2024

athanatos commented Mar 6, 2024 • edited

tchaikov commented Mar 11, 2024

tchaikov commented Mar 11, 2024

tchaikov commented Mar 11, 2024

tchaikov commented Mar 11, 2024

athanatos commented Mar 12, 2024

athanatos commented Mar 12, 2024

athanatos commented Mar 12, 2024 • edited

tchaikov commented Mar 12, 2024

athanatos commented Mar 12, 2024

Svelar commented Mar 13, 2024

athanatos commented Mar 13, 2024 • edited

rzarzynski commented Mar 13, 2024

athanatos commented Mar 18, 2024

athanatos commented Mar 19, 2024 • edited

tchaikov commented Mar 21, 2024

tchaikov commented Mar 21, 2024 • edited

athanatos commented Mar 21, 2024

tchaikov commented Mar 21, 2024

athanatos commented Mar 21, 2024

tchaikov commented Mar 2, 2024 •

edited

tchaikov commented Mar 6, 2024 •

edited

tchaikov commented Mar 6, 2024 •

edited

athanatos commented Mar 6, 2024 •

edited

athanatos commented Mar 12, 2024 •

edited

athanatos commented Mar 13, 2024 •

edited

athanatos commented Mar 19, 2024 •

edited

tchaikov commented Mar 21, 2024 •

edited