build/ops: rpm: set build parallelism based on available memory #19122

smithfarm · 2017-11-23T23:02:25Z

Set build parallelism based on available memory (avoids OOM on machines that have lots of cores, but little memory) and other changes needed to build master in the openSUSE Build Service (OBS).

smithfarm · 2017-11-24T15:24:14Z

Sorry for the initial instability - this PR is now ready for review.

smithfarm · 2017-11-25T21:30:58Z

@sysrich I pulled your recent contribution into this upstream PR (JFYI - thanks).

smithfarm · 2017-11-26T09:58:46Z

https://shaman.ceph.com/builds/ceph/wip-lowmem-builder/d476ca7817298a33aec5192167fa94bdeeadab29/

tchaikov · 2017-12-11T07:46:02Z

ceph.spec.in

@@ -979,7 +978,6 @@ rm -rf %{buildroot}
 %{_sbindir}/ceph-disk
 %{_sbindir}/ceph-volume
 %{_sbindir}/ceph-volume-systemd
-%{_sbindir}/rcceph


could you put more context of this change in the commit messages?

tchaikov · 2017-12-11T07:50:51Z

ceph.spec.in

@@ -33,6 +33,10 @@
 %bcond_with ceph_test_package
 %bcond_with cephfs_java
 %bcond_without lowmem_builder
+#Compat macro for new _fillupdir macro introduced in Nov 2017
+%if ! %{defined _fillupdir}
+  %define _fillupdir /var/adm/fillup-templates


shall we keep using ${_localstatedir} here? or we cannot define macro using another macro?

@smithfarm ping?

tchaikov · 2017-12-11T07:53:35Z

ceph.spec.in

@@ -35,7 +36,7 @@
 %bcond_without lowmem_builder
 #Compat macro for new _fillupdir macro introduced in Nov 2017
 %if ! %{defined _fillupdir}
-  %define _fillupdir /var/adm/fillup-templates
+%global _fillupdir /var/adm/fillup-templates


nit, this change belongs to another commit.

tchaikov · 2017-12-11T07:55:09Z

ceph.spec.in

 %if 0%{?suse_version}
+# _insert_obs_source_lines_here


@smithfarm so, instead of hardwiring to a certain path, OBS will inject another _remote_tarball_prefix when building rpm?

@tchaikov No, in the OBS builds the tarball is not downloaded at all, _remote_tarball_prefix expands to the empty string. In OBS builds, the tarball is generated from the git repo by running make-dist, before running rpmbuild.

I guess I should have linked to the checkin script: https://build.opensuse.org/package/view_file/filesystems:ceph:mimic/ceph/checkin.sh?expand=1

The # _insert_obs_source_lines_here is used by pre_checkin.sh to add some OBS-specific "Source:" lines to the spec file.

got it! thanks for the explanation. for those who are curious enough, see also https://build.opensuse.org/package/view_file/filesystems:ceph:mimic/ceph/pre_checkin.sh?expand=1, _insert_obs_source_lines_here is used as a mark there, and will be replaced by "Source99: ceph-rpmlintrc" by that script.

tchaikov · 2017-12-11T08:03:39Z

src/tools/CMakeLists.txt

@@ -29,6 +30,7 @@ install(TARGETS ceph-osdomap-tool DESTINATION bin)
 add_executable(ceph-monstore-tool ceph_monstore_tool.cc)
 target_link_libraries(ceph-monstore-tool os global Boost::program_options)
 install(TARGETS ceph-monstore-tool DESTINATION bin)
+if(WITH_TESTS)


please just move https://github.com/ceph/ceph/blob/master/src/tools/CMakeLists.txt#L25-L33 out of the WITH_TEST block.

tchaikov · 2017-12-11T08:19:03Z

ceph.spec.in

-# extract the number of processors for use with cmake
-%define _smp_ncpus %(echo %{_smp_mflags} | sed 's/-j//')
+export CEPH_PARALLEL_BUILD
+export CEPH_SMP_NCPUS=$(echo "$CEPH_PARALLEL_BUILD" | sed 's/-j//')


no need to export these two variables. we pass them to cmake and make explicitly.

I'm exporting them so they appear in env so the user can see their values in the build log.

Do you think the export is dangerous in this case? If not, I would argue that it's beneficial to see the values in the build log...

tchaikov · 2017-12-11T08:29:29Z

ceph.spec.in

+# Parallel build settings ...
+# unlimit _smp_mflags in system macro
+%global _smp_ncpus_max 0
+CEPH_PARALLEL_BUILD="%{?_smp_mflags}"


why not just set _smp_mflags? we are practically overriding it here. also the CEPH_PARALLEL_BUILD is confusing, might want to use something like CEPH_MFLAGS_JOBS.

@tchaikov Good question. I tried, but failed, to create the equivalent of CEPH_MFLAGS_JOBS="-j$lo_jobs" as an RPM macro definition. That doesn't mean there is no way to get an expanded shell variable into a macro definition - just that I don't know how.

I changed CEPH_PARALLEL_BUILD to CEPH_MFLAGS_JOBS.

smithfarm · 2018-01-19T09:38:36Z

https://shaman.ceph.com/builds/ceph/wip-lowmem-builder/9d42bd4799c97920eeef32d8912c97d3b3a73ae5/

smithfarm · 2018-01-19T09:40:25Z

Here is what the parallelism-reduction codepath looks like - taken from OBS build log https://build.opensuse.org/public/build/home:smithfarm:branches:filesystems:ceph:mimic/openSUSE_Tumbleweed/x86_64/ceph/_log

[   78s] + CEPH_MFLAGS_JOBS=-j6
[   78s] ++ echo -j6
[   78s] ++ sed s/-j//
[   78s] + CEPH_SMP_NCPUS=6
[   78s] + echo 'Available memory:'
[   78s] Available memory:
[   78s] + free -h
[   78s]               total        used        free      shared  buff/cache   available
[   78s] Mem:           7.8G         90M        5.8G          0B        1.9G        7.4G
[   78s] Swap:          2.0G          0B        2.0G
[   78s] + echo 'System limits:'
[   78s] System limits:
[   78s] + ulimit -a
[   78s] core file size          (blocks, -c) unlimited
[   78s] data seg size           (kbytes, -d) unlimited
[   78s] scheduling priority             (-e) 0
[   78s] file size               (blocks, -f) unlimited
[   78s] pending signals                 (-i) 31869
[   78s] max locked memory       (kbytes, -l) 64
[   78s] max memory size         (kbytes, -m) unlimited
[   78s] open files                      (-n) 1024
[   78s] pipe size            (512 bytes, -p) 8
[   78s] POSIX message queues     (bytes, -q) 819200
[   78s] real-time priority              (-r) 0
[   78s] stack size              (kbytes, -s) 8192
[   78s] cpu time               (seconds, -t) unlimited
[   78s] max user processes              (-u) 4096
[   78s] virtual memory          (kbytes, -v) unlimited
[   78s] file locks                      (-x) unlimited
[   78s] + test -n 6 -a 6 -gt 1
[   78s] + mem_per_process=1800
[   78s] ++ LANG=C
[   78s] ++ free -m
[   78s] ++ sed -n 's|^Mem: *\([0-9]*\).*$|\1|p'
[   78s] + max_mem=7977
[   78s] + max_jobs=4
[   78s] + test 6 -gt 4
[   78s] + CEPH_SMP_NCPUS=4
[   78s] + echo 'Warning: Reducing build parallelism to -j4 because of memory limits'
[   78s] Warning: Reducing build parallelism to -j4 because of memory limits
[   78s] + test 4 -le 0
[   78s] + export CEPH_SMP_NCPUS
[   78s] + export CEPH_MFLAGS_JOBS=-j4
[   78s] + CEPH_MFLAGS_JOBS=-j4
[   78s] + sort
[   78s] + env
[   78s] CEPH_MFLAGS_JOBS=-j4
[   78s] CEPH_SMP_NCPUS=4

smithfarm · 2018-01-19T09:41:33Z

@b-ranto Re-worked and re-tested.

b-ranto · 2018-01-19T10:39:14Z

@smithfarm looks great to me, thanks a lot! I did suspect this might even help us speed up the builds on machines with enough cores/memory like the 16-core centos builder you mentioned.

smithfarm · 2018-01-19T10:41:54Z

@b-ranto Not sure about the speed-up. %_smp_mflags is set to 12, so I'm guessing the Shaman builder has 12 cores. But it has enough memory to sustain 16 parallel builds. But in this case we don't exceed %_smp_mflags - i.e. we set -j12 (no speed-up).

smithfarm · 2018-01-19T10:45:02Z

The purpose of the patch is to avoid OOM condition on machines with high numbers of cores, but low memory.

If you like, I can have it set -j{MAX} regardless of whether that might exceed %_smp_mflags (but that seems like it might confuse people who are setting %_smp_mflags explicitly - if there are any such people)?

smithfarm · 2018-01-19T10:51:02Z

Reinstating "needs-qa" because the PR has been completely re-worked.

b-ranto · 2018-01-19T11:01:36Z

I am happy with the PR at its current state. We override the _smp_mflags option only when we need to which is imo the best approach. If there is no speed up then I suppose we are hitting I/O limits but that is no reason to limit this. The expected speed up with 12 compared to 8 cores would also be relatively small (~25% maybe?). Thanks again for doing this!

Disable java build completely. Enable lttng build on SLES only, and only for certain architectures. Signed-off-by: Nathan Cutler <ncutler@suse.com>

Replace references to /var/adm/fillup-templates with new %_fillupdir macro Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1069468 Signed-off-by: Richard Brown <rbrown@suse.com>

With this macro, we can use a single Source0 line for all supported distros. RH/CentOS/Fedora needs the prefix, while SUSE builds in the OBS use a local tarball. Signed-off-by: Nathan Cutler <ncutler@suse.com>

Fixes RPMLINT warning "non-standard-group Development/Libraries" Also, the Group: line is only needed for SUSE so put it in an appropriate distro conditional. Signed-off-by: Nathan Cutler <ncutler@suse.com>

Sometimes the build machine has lots of processor cores and not enough memory to successfully build Ceph on all of them at once. Calculate how many parallel build processes we can sustain with the memory we have and set a lower build parallelism if necessary. Never exceed the value set by %_smp_mflags even if memory is aplenty. Credits to Tomáš Chvátal for the original idea and implementation. Signed-off-by: Nathan Cutler <ncutler@suse.com>

Signed-off-by: Nathan Cutler <ncutler@suse.com>

As a follow-up to d7b493a we need to stop guarding ceph-osdomap-tool ceph-monstore-tool with WITH_TESTS because they have been moved out of the ceph-test package. (N.B. ceph-kvstore-tool was also moved out of ceph-test, but apparently never had the guard.) Signed-off-by: Nathan Cutler <ncutler@suse.com>

smithfarm · 2018-01-24T22:23:45Z

Rebased to fix wrong Group name in "build/ops: rpm: fix Group for rados-objclass-devel subpackage"

smithfarm · 2018-01-25T06:00:55Z

This PR passed a rados run here: http://pulpito.ceph.com/smithfarm-2018-01-24_19:46:55-rados-wip-smithfarm-testing-distro-basic-smithi/

197 of 202 tests passed.

Failed tests:

"2018-01-25 02:00:26.400626 osd.5 osd.5 172.21.15.77:6810/36898 23 : cluster [ERR] 3.0 scrub stat mismatch, got 48/48 objects, 5/5 clones, 23/23 dirty, 0/0 omap, 0/0 pinned, 1/1 hit_set_archive, 7/7 whiteouts, 112856156/114189825 bytes, 249/249 hit_set_archive bytes." in cluster log
"2018-01-25 01:54:14.602444 mgr.z client.4375 172.21.15.193:0/3577525578 1 : cluster [ERR] Unhandled exception from module 'prometheus' while running on mgr.z: error(98, 'Address already in use')" in cluster log
"2018-01-25 01:40:33.702645 mon.f mon.0 172.21.15.94:6789/0 118 : cluster [WRN] Health check failed: 1 slow ops, oldest one blocked for 33 sec (SLOW_OPS)" in cluster log

Two tests are still running.

smithfarm requested review from tchaikov, ktdreyer and b-ranto November 23, 2017 23:02

smithfarm force-pushed the wip-lowmem-builder branch from 0a207ec to 636937a Compare November 24, 2017 04:57

smithfarm mentioned this pull request Nov 24, 2017

SES6 initial build SUSE/ceph#168

Closed

smithfarm force-pushed the wip-lowmem-builder branch from 9f7ac55 to 107661b Compare November 24, 2017 09:32

smithfarm added build/ops feature labels Nov 24, 2017

smithfarm force-pushed the wip-lowmem-builder branch from 107661b to 107616c Compare November 24, 2017 13:00

smithfarm requested a review from dmick November 24, 2017 13:01

smithfarm force-pushed the wip-lowmem-builder branch from ae4f2ec to a292d52 Compare November 24, 2017 13:26

smithfarm changed the title ~~build/ops: rpm: make lowmem_builder code much smarter~~ build/ops: rpm: smarter lowmem_builder and other changes necessary to build master in OBS Nov 24, 2017

smithfarm force-pushed the wip-lowmem-builder branch from a292d52 to 7f0a1fd Compare November 25, 2017 15:02

smithfarm force-pushed the wip-lowmem-builder branch 2 times, most recently from b7af483 to d476ca7 Compare November 26, 2017 09:57

This was referenced Nov 27, 2017

SES6 initial build SUSE/ceph#171

Merged

spdk: update SPDK to v17.10 #19208

Merged

build/ops: rpm: rip out rcceph script SUSE/ceph#174

Closed

tchaikov reviewed Dec 11, 2017

View reviewed changes

dmick removed their request for review December 12, 2017 00:02

smithfarm force-pushed the wip-lowmem-builder branch 3 times, most recently from dca0dcb to ef1cf1e Compare January 10, 2018 11:06

tchaikov self-requested a review January 10, 2018 11:22

smithfarm changed the title ~~build/ops: rpm: smarter lowmem_builder and other changes necessary to build master in OBS~~ build/ops: rpm: changes necessary to build master in OBS Jan 10, 2018

smithfarm force-pushed the wip-lowmem-builder branch 2 times, most recently from 022e5a2 to 651877e Compare January 10, 2018 18:38

smithfarm force-pushed the wip-lowmem-builder branch from 34e1e50 to 9d42bd4 Compare January 19, 2018 07:39

b-ranto approved these changes Jan 19, 2018

View reviewed changes

smithfarm added the needs-qa label Jan 19, 2018

smithfarm mentioned this pull request Jan 23, 2018

cmake: enabled py3 only build #20064

Merged

smithfarm added the wip-smithfarm-testing label Jan 23, 2018

This was referenced Jan 23, 2018

build/ops: rpm: conditionalize Python 2 availability to enable Ceph build on Python 3-only system #20018

Merged

libradosstriper: conditional compile #18197

Closed

smithfarm force-pushed the wip-lowmem-builder branch from 2c81055 to 18255ad Compare January 23, 2018 22:42

smithfarm changed the title ~~build/ops: rpm: changes necessary to build master in OBS~~ build/ops: rpm: set build parallelism based on available memory Jan 24, 2018

smithfarm and others added 3 commits January 24, 2018 17:16

build/ops: rpm: reconstruct SUSE bconds

dd08d17

Disable java build completely. Enable lttng build on SLES only, and only for certain architectures. Signed-off-by: Nathan Cutler <ncutler@suse.com>

build/ops: rpm: use %_fillupdir macro in openSUSE/SLES

2305bf0

Replace references to /var/adm/fillup-templates with new %_fillupdir macro Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1069468 Signed-off-by: Richard Brown <rbrown@suse.com>

build/ops: rpm: introduce _remote_tarball_prefix macro

290d909

With this macro, we can use a single Source0 line for all supported distros. RH/CentOS/Fedora needs the prefix, while SUSE builds in the OBS use a local tarball. Signed-off-by: Nathan Cutler <ncutler@suse.com>

smithfarm force-pushed the wip-lowmem-builder branch from 18255ad to 705eb95 Compare January 24, 2018 16:16

smithfarm added 4 commits January 24, 2018 23:20

build/ops: rpm: fix Group for rados-objclass-devel subpackage

353ff67

Fixes RPMLINT warning "non-standard-group Development/Libraries" Also, the Group: line is only needed for SUSE so put it in an appropriate distro conditional. Signed-off-by: Nathan Cutler <ncutler@suse.com>

build/ops: rpm: let ctest benefit from memory calculation

d048cb2

Signed-off-by: Nathan Cutler <ncutler@suse.com>

smithfarm force-pushed the wip-lowmem-builder branch from 705eb95 to e020ec3 Compare January 24, 2018 22:22

smithfarm merged commit 34cc708 into ceph:master Jan 25, 2018

smithfarm deleted the wip-lowmem-builder branch January 25, 2018 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build/ops: rpm: set build parallelism based on available memory #19122

build/ops: rpm: set build parallelism based on available memory #19122

smithfarm commented Nov 23, 2017 •

edited

smithfarm commented Nov 24, 2017

smithfarm commented Nov 25, 2017

smithfarm commented Nov 26, 2017

tchaikov Dec 11, 2017

smithfarm Jan 10, 2018

tchaikov Dec 11, 2017

tchaikov Jan 11, 2018

tchaikov Dec 11, 2017

tchaikov Dec 11, 2017

smithfarm Jan 10, 2018

smithfarm Jan 10, 2018

tchaikov Jan 11, 2018

tchaikov Dec 11, 2017

smithfarm Jan 10, 2018

tchaikov Dec 11, 2017

smithfarm Jan 10, 2018

smithfarm Jan 10, 2018

tchaikov Dec 11, 2017

smithfarm Jan 10, 2018

smithfarm Jan 10, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

b-ranto commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

b-ranto commented Jan 19, 2018

smithfarm commented Jan 24, 2018

smithfarm commented Jan 25, 2018

build/ops: rpm: set build parallelism based on available memory #19122

build/ops: rpm: set build parallelism based on available memory #19122

Conversation

smithfarm commented Nov 23, 2017 • edited

smithfarm commented Nov 24, 2017

smithfarm commented Nov 25, 2017

smithfarm commented Nov 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

b-ranto commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

smithfarm commented Jan 19, 2018

b-ranto commented Jan 19, 2018

smithfarm commented Jan 24, 2018

smithfarm commented Jan 25, 2018

smithfarm commented Nov 23, 2017 •

edited