[pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[Alexandre Derumier]

They are a bug in current debian packaging,
where rocksdb is build in debug mode (not inheriting from cmake flags)

patch 1 already exist in ubuntu
patch 2 is a newer patch commited in ceph recently

(patch 2 should be enough, but I'm keeping patch1 by security).


I have tested it, 4k randwrite iops x2, 

fio: 4k randwrite:queue depth=64:sync write:  cache=none:
before:  16000 iops
https://pbs.twimg.com/media/GDKu9dqXYAAeH4I?format=jpg&name=large

after:  38000 iops
https://pbs.twimg.com/media/GDKu9dqW8AA3dE8?format=jpg&name=large

sandre-win.altima-hosting.fr
sandre-win.altima-hosting.fr


and submit latency /3
https://pbs.twimg.com/media/GDFmBJOW8AApWYZ?format=jpg&name=large

Alexandre Derumier (2):
  patch: debian/rules: fix build type
  patch: add 0022-rocksb-inherit-parent-cmake-cxx-flags.patch

 patches/0021-debian-rules-fix-buildtype.patch | 22 +++++++
 ...ocksb-inherit-parent-cmake-cxx-flags.patch | 57 +++++++++++++++++++
 patches/series                                |  2 +
 3 files changed, 81 insertions(+)
 create mode 100644 patches/0021-debian-rules-fix-buildtype.patch
 create mode 100644 patches/0022-rocksb-inherit-parent-cmake-cxx-flags.patch

-- 
2.39.2

[pve-devel] [PATCH ceph 1/2] patch: debian/rules: fix build type

[Alexandre Derumier]

source: https://github.com/ceph/ceph/pull/54891

build packages with 'RelWithDebInfo' to avoid to build rocksdb in debug

This is already the default in ubuntu packages
https://bugs.launchpad.net/ubuntu/+source/ceph/+bug/1894453
---
 patches/0021-debian-rules-fix-buildtype.patch | 22 +++++++++++++++++++
 patches/series                                |  1 +
 2 files changed, 23 insertions(+)
 create mode 100644 patches/0021-debian-rules-fix-buildtype.patch

diff --git a/patches/0021-debian-rules-fix-buildtype.patch b/patches/0021-debian-rules-fix-buildtype.patch
new file mode 100644
index 000000000..8b6ef6b56
--- /dev/null
+++ b/patches/0021-debian-rules-fix-buildtype.patch
@@ -0,0 +1,22 @@
+From 1f4b106d49fc916994d97e273599f75caa904c3b Mon Sep 17 00:00:00 2001
+From: Mark Nelson <mark.nelson@clyso.com>
+Date: Thu, 14 Dec 2023 05:19:46 +0000
+Subject: [PATCH] debian/rules: Fix build_type for massive performance gain
+
+Signed-off-by: Mark Nelson <mark.nelson@clyso.com>
+---
+ debian/rules | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/debian/rules b/debian/rules
+index ed7f4a255ed4b..b28abb7d62788 100755
+--- a/debian/rules
++++ b/debian/rules
+@@ -29,6 +29,7 @@ extraopts += -DWITH_PYTHON3=3
+ extraopts += -DWITH_CEPHFS_JAVA=ON
+ extraopts += -DWITH_CEPHFS_SHELL=ON
+ extraopts += -DWITH_SYSTEMD=ON -DCEPH_SYSTEMD_ENV_DIR=/etc/default
++extraopts += -DCMAKE_BUILD_TYPE=RelWithDebInfo
+ extraopts += -DWITH_GRAFANA=ON
+ ifeq ($(DEB_HOST_ARCH), amd64)
+   extraopts += -DWITH_RBD_RWL=ON
diff --git a/patches/series b/patches/series
index 67a52ae7c..d9a3ff8f4 100644
--- a/patches/series
+++ b/patches/series
@@ -13,3 +13,4 @@
 0016-d-rules-fix-no-restart-on-upgrade.patch
 0017-python3.10-pep-620.patch
 0020-fix-4759-run-ceph-crash-daemon-with-www-data-group-f.patch
+0021-debian-rules-fix-buildtype.patch
\ No newline at end of file
-- 
2.39.2

[pve-devel] [PATCH ceph 2/2] patch: add 0022-rocksb-inherit-parent-cmake-cxx-flags.patch

[Alexandre Derumier]

upstream patch: https://github.com/ceph/ceph/pull/54918
---
 ...ocksb-inherit-parent-cmake-cxx-flags.patch | 57 +++++++++++++++++++
 patches/series                                |  3 +-
 2 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 patches/0022-rocksb-inherit-parent-cmake-cxx-flags.patch

diff --git a/patches/0022-rocksb-inherit-parent-cmake-cxx-flags.patch b/patches/0022-rocksb-inherit-parent-cmake-cxx-flags.patch
new file mode 100644
index 000000000..8d61ce7e0
--- /dev/null
+++ b/patches/0022-rocksb-inherit-parent-cmake-cxx-flags.patch
@@ -0,0 +1,57 @@
+From 620b68a348f07145c49c12668576a89dee8198cb Mon Sep 17 00:00:00 2001
+From: Kefu Chai <tchaikov@gmail.com>
+Date: Fri, 15 Dec 2023 19:01:46 +0800
+Subject: [PATCH] cmake/modules/BuildRocksDB.cmake: inherit parent's CMAKE_CXX_FLAGS
+
+CMake allows us to customize `CMAKE_CXX_FLAGS` by setting CXXFLAGS
+environmental variable. and Debian's debhelper also sets CXXFLAGS
+when it builds cmake projects for customizing the building flags.
+
+but we fail to populate this setting down when building external
+projects. this is important when it comes to the projects which
+is critical to the performance. RocksDB is one of them.
+
+in this change, we pass the `CMAKE_CXX_FLAGS` down in
+`BuildRocksDB.cmake` so that its `CMAKE_CXX_FLAGS` contains
+the same set of `CMAKE_CXX_FLAGS` used by its parent project.
+
+this should help with the performance in the bluestore, where
+RocksDB is used.
+
+Signed-off-by: Kefu Chai <tchaikov@gmail.com>
+---
+ cmake/modules/BuildRocksDB.cmake | 4 ++--
+ cmake/modules/BuildRocksDB.cmake | 1 +
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/cmake/modules/BuildRocksDB.cmake b/cmake/modules/BuildRocksDB.cmake
+index f9a28274c40d2..f81f5248506dd 100644
+--- a/cmake/modules/BuildRocksDB.cmake
++++ b/cmake/modules/BuildRocksDB.cmake
+@@ -60,11 +60,11 @@ function(build_rocksdb)
+   include(CheckCXXCompilerFlag)
+   check_cxx_compiler_flag("-Wno-deprecated-copy" HAS_WARNING_DEPRECATED_COPY)
+   if(HAS_WARNING_DEPRECATED_COPY)
+-    set(rocksdb_CXX_FLAGS -Wno-deprecated-copy)
++    string(APPEND rocksdb_CXX_FLAGS " -Wno-deprecated-copy")
+   endif()
+   check_cxx_compiler_flag("-Wno-pessimizing-move" HAS_WARNING_PESSIMIZING_MOVE)
+   if(HAS_WARNING_PESSIMIZING_MOVE)
+-    set(rocksdb_CXX_FLAGS "${rocksdb_CXX_FLAGS} -Wno-pessimizing-move")
++    string(APPEND rocksdb_CXX_FLAGS " -Wno-pessimizing-move")
+   endif()
+   if(rocksdb_CXX_FLAGS)
+     list(APPEND rocksdb_CMAKE_ARGS -DCMAKE_CXX_FLAGS='${rocksdb_CXX_FLAGS}')
+
+diff --git a/cmake/modules/BuildRocksDB.cmake b/cmake/modules/BuildRocksDB.cmake
+index f81f5248506dd..e0208f6545b70 100644
+--- a/cmake/modules/BuildRocksDB.cmake
++++ b/cmake/modules/BuildRocksDB.cmake
+@@ -59,6 +59,7 @@ function(build_rocksdb)
+   endif()
+   include(CheckCXXCompilerFlag)
+   check_cxx_compiler_flag("-Wno-deprecated-copy" HAS_WARNING_DEPRECATED_COPY)
++  set(rocksdb_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+   if(HAS_WARNING_DEPRECATED_COPY)
+     string(APPEND rocksdb_CXX_FLAGS " -Wno-deprecated-copy")
+   endif()
diff --git a/patches/series b/patches/series
index d9a3ff8f4..eddd47031 100644
--- a/patches/series
+++ b/patches/series
@@ -13,4 +13,5 @@
 0016-d-rules-fix-no-restart-on-upgrade.patch
 0017-python3.10-pep-620.patch
 0020-fix-4759-run-ceph-crash-daemon-with-www-data-group-f.patch
-0021-debian-rules-fix-buildtype.patch
\ No newline at end of file
+0021-debian-rules-fix-buildtype.patch
+0022-rocksb-inherit-parent-cmake-cxx-flags.patch
\ No newline at end of file
-- 
2.39.2

Re: [pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[Thomas Lamprecht]

Am 09/01/2024 um 15:50 schrieb Alexandre Derumier:
> They are a bug in current debian packaging,
> where rocksdb is build in debug mode (not inheriting from cmake flags)
> 
> patch 1 already exist in ubuntu
> patch 2 is a newer patch commited in ceph recently
> 
> (patch 2 should be enough, but I'm keeping patch1 by security).

thanks, but this does not apply to the master branch of our ceph.git, or
is this for an older release than Reef 18.2?

Re: [pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[DERUMIER, Alexandre]

oh, I have tested with 17.2.7-pve1


I'll look for 18.2

-------- Message initial --------
De: Thomas Lamprecht <t.lamprecht@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Alexandre Derumier <aderumier@odiso.com>
Objet: Re: [pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode
Date: 09/01/2024 16:39:52

Am 09/01/2024 um 15:50 schrieb Alexandre Derumier:
> They are a bug in current debian packaging,
> where rocksdb is build in debug mode (not inheriting from cmake
> flags)
> 
> patch 1 already exist in ubuntu
> patch 2 is a newer patch commited in ceph recently
> 
> (patch 2 should be enough, but I'm keeping patch1 by security).

thanks, but this does not apply to the master branch of our ceph.git,
or
is this for an older release than Reef 18.2?

Re: [pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[Thomas Lamprecht]

Am 09/01/2024 um 16:46 schrieb DERUMIER, Alexandre:
> oh, I have tested with 17.2.7-pve1
> 
> 
> I'll look for 18.2

I managed to port it over, and in the second patch I squashed both
changes into one diff hunk, those two hunks touching the same file
confused me slightly initially ^^

Re: [pve-devel] [PATCH ceph 0/2] Build rocksdb in non-debug mode

[DERUMIER, Alexandre]

>>
>>I managed to port it over, and in the second patch I squashed both
>>changes into one diff hunk, those two hunks touching the same file
>>confused me slightly initially ^^

oh, sorry, it was 2 patches from the initial github PR, and I have 
merged them in the same patch manually, not cleanly

Re: [pve-devel] [PATCH ceph 1/2] patch: debian/rules: fix build type

[Fabian Grünbichler]

On January 9, 2024 3:50 pm, Alexandre Derumier wrote:
> source: https://github.com/ceph/ceph/pull/54891

this was not merged upstream, and I don't think it's needed as per the
discussions in both PRs if the other patch is pulled in.

> build packages with 'RelWithDebInfo' to avoid to build rocksdb in debug
> 
> This is already the default in ubuntu packages
> https://bugs.launchpad.net/ubuntu/+source/ceph/+bug/1894453
> ---
>  patches/0021-debian-rules-fix-buildtype.patch | 22 +++++++++++++++++++
>  patches/series                                |  1 +
>  2 files changed, 23 insertions(+)
>  create mode 100644 patches/0021-debian-rules-fix-buildtype.patch
> 
> diff --git a/patches/0021-debian-rules-fix-buildtype.patch b/patches/0021-debian-rules-fix-buildtype.patch
> new file mode 100644
> index 000000000..8b6ef6b56
> --- /dev/null
> +++ b/patches/0021-debian-rules-fix-buildtype.patch
> @@ -0,0 +1,22 @@
> +From 1f4b106d49fc916994d97e273599f75caa904c3b Mon Sep 17 00:00:00 2001
> +From: Mark Nelson <mark.nelson@clyso.com>
> +Date: Thu, 14 Dec 2023 05:19:46 +0000
> +Subject: [PATCH] debian/rules: Fix build_type for massive performance gain
> +
> +Signed-off-by: Mark Nelson <mark.nelson@clyso.com>
> +---
> + debian/rules | 1 +
> + 1 file changed, 1 insertion(+)
> +
> +diff --git a/debian/rules b/debian/rules
> +index ed7f4a255ed4b..b28abb7d62788 100755
> +--- a/debian/rules
> ++++ b/debian/rules
> +@@ -29,6 +29,7 @@ extraopts += -DWITH_PYTHON3=3
> + extraopts += -DWITH_CEPHFS_JAVA=ON
> + extraopts += -DWITH_CEPHFS_SHELL=ON
> + extraopts += -DWITH_SYSTEMD=ON -DCEPH_SYSTEMD_ENV_DIR=/etc/default
> ++extraopts += -DCMAKE_BUILD_TYPE=RelWithDebInfo
> + extraopts += -DWITH_GRAFANA=ON
> + ifeq ($(DEB_HOST_ARCH), amd64)
> +   extraopts += -DWITH_RBD_RWL=ON
> diff --git a/patches/series b/patches/series
> index 67a52ae7c..d9a3ff8f4 100644
> --- a/patches/series
> +++ b/patches/series
> @@ -13,3 +13,4 @@
>  0016-d-rules-fix-no-restart-on-upgrade.patch
>  0017-python3.10-pep-620.patch
>  0020-fix-4759-run-ceph-crash-daemon-with-www-data-group-f.patch
> +0021-debian-rules-fix-buildtype.patch
> \ No newline at end of file
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>