build, libglusterfs, tests: use system xxhash if installed #3127

dmantipov · 2022-01-11T10:26:11Z

Use installed xxhash library and xxhsum binary if found,
fallback to use everything from contrib/xxhash otherwise,
adjust gfid2path FUSE and NFS tests accordingly.

Signed-off-by: Dmitry Antipov dantipov@cloudlinux.com
Updates: #1000

gluster-ant · 2022-01-11T10:43:15Z

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index f01134315..793f9f69e 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -69,7 +69,7 @@
 #else /* not a convenient LP64 ABI */
 #define XXH64_FMT "llx"
 #endif /* ABI selection */
-#else /* use from contrib */
+#else  /* use from contrib */
 #define XXH64_FMT "llx"
 #endif /* HAVE_LIBXXHASH */
 
@@ -303,7 +303,8 @@ gf_gfid_generate_from_xxh64(uuid_t gfid, char *key)
     }
 
     gf_msg_debug(this->name, 0,
-                 "gfid generated is %s (hash1: %" XXH64_FMT ") "
+                 "gfid generated is %s (hash1: %" XXH64_FMT
+                 ") "
                  "hash2: %" XXH64_FMT ", xxh64_1: %s xxh64_2: %s",
                  uuid_utoa(gfid), hash_1, hash_2, xxh64_1, xxh64_2);
 
@@ -5474,7 +5475,7 @@ gf_pipe(int fd[2], int flags)
     int ret = 0;
 #if defined(HAVE_PIPE2)
     ret = pipe2(fd, flags);
-#else /* not HAVE_PIPE2 */
+#else  /* not HAVE_PIPE2 */
     ret = pipe(fd);
     if (ret < 0)
         return ret;

dmantipov · 2022-01-11T11:24:10Z

/run regression

amarts · 2022-01-11T12:44:59Z

@sac ☝️

sac · 2022-01-11T13:04:57Z

libglusterfs/src/common-utils.c

@@ -59,6 +63,16 @@
 #undef BIT_SET
 #endif

+#if defined(HAVE_LIBXXHASH)


Why not just use PRIx64? Both %ld and %lld are handled by PRIx64.

That's kinda weird. PRIx64 is %lx (unsigned long) on a convenient 64-bit system but contrib/xxhash/xxhash.h defines XXH64_hash_t as unsigned long long everywhere (so %llx is needed everywhere as well). And never (as of >= 0.8) xxhash defines XXH64_hash_t as unsigned long on 64-bit system and unsigned long long on a 32-bit one.

But, still may be simplified to:

#if defined(HAVE_LIBXXHASH) #define XXH64_FMT PRIx64 #else /* use from contrib */ #define XXH64_FMT "llx" #endif /* HAVE_LIBXXHASH */

sac · 2022-01-11T14:16:55Z

libglusterfs/src/common-utils.c

@@ -50,7 +50,11 @@
 #include "glusterfs/syscall.h"
 #include "glusterfs/globals.h"
 #define XXH_INLINE_ALL


Which version of xxhash are you using? With xxhash-8.0 on ubuntu compile fails if XXH_NAMESPACE and XXH_INLINE_ALL are both defined. You may have to remove XXH_NAMESPACE.

xxhash.h
116 # ifdef XXH_NAMESPACE
117 # error "XXH_INLINE_ALL with XXH_NAMESPACE is not supported"
118 /*

0.8.1. Don't see any issues with or without XXH_NAMESPACE defined. Please share your error log.

Making all in src make[1]: Entering directory '/tmp/glusterfs/libglusterfs/src' make all-am make[2]: Entering directory '/tmp/glusterfs/libglusterfs/src' CC libglusterfs_la-common-utils.lo In file included from common-utils.c:54: /usr/include/xxhash.h:127:6: error: #error "XXH_INLINE_ALL with XXH_NAMESPACE is not supported" 127 | # error "XXH_INLINE_ALL with XXH_NAMESPACE is not supported" | ^~~~~ make[2]: *** [Makefile:1105: libglusterfs_la-common-utils.lo] Error 1 make[2]: Leaving directory '/tmp/glusterfs/libglusterfs/src' make[1]: *** [Makefile:894: all] Error 2 make[1]: Leaving directory '/tmp/glusterfs/libglusterfs/src' make: *** [Makefile:467: all-recursive] Error 1 sac@kadalu:/tmp/glusterfs/libglusterfs$

And my package:

sac@kadalu:/tmp/glusterfs/libglusterfs$ apt-cache policy libxxhash0 libxxhash0: Installed: 0.8.0-2build1 Candidate: 0.8.0-2build1 Version table: *** 0.8.0-2build1 500 500 http://in.archive.ubuntu.com/ubuntu impish/main amd64 Packages 100 /var/lib/dpkg/status sac@kadalu:/tmp/glusterfs/libglusterfs$

Hmm... there is no such #error neither in 0.8.1 nor in https://github.com/Cyan4973/xxHash/blob/dev/xxhash.h. It was removed in Cyan4973/xxHash@81d343d, so I would suggest to update to 0.8.1 and consider 0.8.0 as broken in this area.

Interesting, good find.

libglusterfs/src/common-utils.c

gluster-ant · 2022-01-11T15:37:28Z

1 test(s) failed
./tests/gfid2path/gfid2path_fuse.t

0 test(s) generated core

5 test(s) needed retry
./tests/000-flaky/basic_afr_split-brain-favorite-child-policy.t
./tests/000-flaky/basic_mount-nfs-auth.t
./tests/000-flaky/bugs_nfs_bug-1116503.t
./tests/bugs/glusterd/brick-order-check-add-brick.t
./tests/gfid2path/gfid2path_fuse.t

2 flaky test(s) marked as success even though they failed
./tests/000-flaky/basic_mount-nfs-auth.t
./tests/000-flaky/bugs_nfs_bug-1116503.t
https://build.gluster.org/job/gh_centos7-regression/2026/

sac · 2022-01-11T17:28:41Z

Otherwise everything looks good.

gluster-ant · 2022-01-11T17:33:50Z

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 316520545..56d6159d9 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -295,7 +295,8 @@ gf_gfid_generate_from_xxh64(uuid_t gfid, char *key)
     }
 
     gf_msg_debug(this->name, 0,
-                 "gfid generated is %s (hash1: %" XXH64_FMT ") "
+                 "gfid generated is %s (hash1: %" XXH64_FMT
+                 ") "
                  "hash2: %" XXH64_FMT ", xxh64_1: %s xxh64_2: %s",
                  uuid_utoa(gfid), hash_1, hash_2, xxh64_1, xxh64_2);
 
@@ -5466,7 +5467,7 @@ gf_pipe(int fd[2], int flags)
     int ret = 0;
 #if defined(HAVE_PIPE2)
     ret = pipe2(fd, flags);
-#else /* not HAVE_PIPE2 */
+#else  /* not HAVE_PIPE2 */
     ret = pipe(fd);
     if (ret < 0)
         return ret;

dmantipov · 2022-01-12T07:03:12Z

/run regression

gluster-ant · 2022-01-12T11:12:28Z

1 test(s) failed
./tests/gfid2path/gfid2path_fuse.t

0 test(s) generated core

3 test(s) needed retry
./tests/000-flaky/basic_afr_split-brain-favorite-child-policy.t
./tests/000-flaky/glusterd-restart-shd-mux.t
./tests/gfid2path/gfid2path_fuse.t

1 flaky test(s) marked as success even though they failed
./tests/000-flaky/glusterd-restart-shd-mux.t
https://build.gluster.org/job/gh_centos7-regression/2034/

gluster-ant · 2022-01-12T13:23:28Z

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 316520545..56d6159d9 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -295,7 +295,8 @@ gf_gfid_generate_from_xxh64(uuid_t gfid, char *key)
     }
 
     gf_msg_debug(this->name, 0,
-                 "gfid generated is %s (hash1: %" XXH64_FMT ") "
+                 "gfid generated is %s (hash1: %" XXH64_FMT
+                 ") "
                  "hash2: %" XXH64_FMT ", xxh64_1: %s xxh64_2: %s",
                  uuid_utoa(gfid), hash_1, hash_2, xxh64_1, xxh64_2);
 
@@ -5466,7 +5467,7 @@ gf_pipe(int fd[2], int flags)
     int ret = 0;
 #if defined(HAVE_PIPE2)
     ret = pipe2(fd, flags);
-#else /* not HAVE_PIPE2 */
+#else  /* not HAVE_PIPE2 */
     ret = pipe(fd);
     if (ret < 0)
         return ret;

dmantipov · 2022-01-12T13:59:19Z

/run regression

dmantipov · 2022-01-13T07:33:13Z

/run regression

dmantipov · 2022-01-14T11:02:43Z

/run regression

gluster-ant · 2022-01-14T15:24:26Z

1 test(s) failed
./tests/gfid2path/gfid2path_fuse.t

0 test(s) generated core

6 test(s) needed retry
./tests/000-flaky/basic_mount-nfs-auth.t
./tests/000-flaky/bugs_nfs_bug-1116503.t
./tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t
./tests/basic/ec/ec-1468261.t
./tests/bugs/distribute/bug-1099890.t
./tests/gfid2path/gfid2path_fuse.t

2 flaky test(s) marked as success even though they failed
./tests/000-flaky/basic_mount-nfs-auth.t
./tests/000-flaky/bugs_nfs_bug-1116503.t
https://build.gluster.org/job/gh_centos7-regression/2044/

dmantipov · 2022-01-14T17:24:21Z

This patch assumes that both xxhash development headers/libraries/symlinks etc. and xxhsum binary are installed. System(s) running regression tests seems has no xxhsum binary in PATH:

14:04:01 configure: WARNING: install 'xxhsum' binary if you plan to run the tests.

mykaul · 2022-01-14T17:35:42Z

This patch assumes that both xxhash development headers/libraries/symlinks etc. and xxhsum binary are installed. System(s) running regression tests seems has no xxhsum binary in PATH:
14:04:01 configure: WARNING: install 'xxhsum' binary if you plan to run the tests.

We'll need an issue / send a patch to https://github.com/gluster/project-infrastructure

sac · 2022-01-15T10:09:53Z

This patch assumes that both xxhash development headers/libraries/symlinks etc. and xxhsum binary are installed. System(s) running regression tests seems has no xxhsum binary in PATH:
14:04:01 configure: WARNING: install 'xxhsum' binary if you plan to run the tests.
We'll need an issue / send a patch to https://github.com/gluster/project-infrastructure

I have already raised a request for that and @mscherer has fixed the build machines. Please take a look at the issue and update the issue if you need anything more.
Ref: gluster/project-infrastructure#156

Should you be adding the xxhash to the install list of packages in tests/vagrant/vagrant-template-centos6/roles/install-pkgs/tasks/main.yml and tests/vagrant/vagrant-template-fedora/roles/install-pkgs/tasks/main.yml ?

mscherer · 2022-01-16T09:37:25Z

So I can confirm that xxhash-devel is installed:

[root@builder-c7-4 ~]# rpm -qa | grep -i xxhas
xxhash-devel-0.8.1-1.el7.x86_64
xxhash-libs-0.8.1-1.el7.x86_64

However, what is missing is xxhash directly. I am adding it.

mscherer · 2022-01-16T10:33:25Z

/run regression

Use installed xxhash library and xxhsum binary if found, fallback to use everything from contrib/xxhash otherwise, adjust gfid2path FUSE and NFS tests accordingly. Signed-off-by: Dmitry Antipov <dantipov@cloudlinux.com> Updates: gluster#1000

gluster-ant · 2022-01-17T09:59:56Z

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 316520545..56d6159d9 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -295,7 +295,8 @@ gf_gfid_generate_from_xxh64(uuid_t gfid, char *key)
     }
 
     gf_msg_debug(this->name, 0,
-                 "gfid generated is %s (hash1: %" XXH64_FMT ") "
+                 "gfid generated is %s (hash1: %" XXH64_FMT
+                 ") "
                  "hash2: %" XXH64_FMT ", xxh64_1: %s xxh64_2: %s",
                  uuid_utoa(gfid), hash_1, hash_2, xxh64_1, xxh64_2);
 
@@ -5466,7 +5467,7 @@ gf_pipe(int fd[2], int flags)
     int ret = 0;
 #if defined(HAVE_PIPE2)
     ret = pipe2(fd, flags);
-#else /* not HAVE_PIPE2 */
+#else  /* not HAVE_PIPE2 */
     ret = pipe(fd);
     if (ret < 0)
         return ret;

dmantipov · 2022-01-17T10:28:41Z

/run regression

tests/gfid2path/gfid2path_fuse.t

sac suggested changes Jan 11, 2022

View reviewed changes

dmantipov force-pushed the 1000-build-libglusterfs-tests-use-system-xxhash branch from 960f903 to 17b29ee Compare January 11, 2022 17:08

sac previously approved these changes Jan 12, 2022

View reviewed changes

dmantipov dismissed sac’s stale review via 21002ef January 12, 2022 13:05

dmantipov force-pushed the 1000-build-libglusterfs-tests-use-system-xxhash branch from 17b29ee to 21002ef Compare January 12, 2022 13:05

dmantipov force-pushed the 1000-build-libglusterfs-tests-use-system-xxhash branch from 21002ef to adae433 Compare January 17, 2022 09:42

sac approved these changes Jan 18, 2022

View reviewed changes

sac reviewed Jan 18, 2022

View reviewed changes

tests/gfid2path/gfid2path_fuse.t Show resolved Hide resolved

sac approved these changes Jan 18, 2022

View reviewed changes

amarts approved these changes Jan 20, 2022

View reviewed changes

amarts merged commit 1161d23 into gluster:devel Jan 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build, libglusterfs, tests: use system xxhash if installed #3127

build, libglusterfs, tests: use system xxhash if installed #3127

dmantipov commented Jan 11, 2022 •

edited

gluster-ant commented Jan 11, 2022

dmantipov commented Jan 11, 2022

amarts commented Jan 11, 2022

sac Jan 11, 2022

dmantipov Jan 11, 2022

dmantipov Jan 11, 2022

sac Jan 11, 2022

dmantipov Jan 11, 2022

sac Jan 11, 2022

dmantipov Jan 12, 2022

sac Jan 12, 2022

gluster-ant commented Jan 11, 2022

sac commented Jan 11, 2022

gluster-ant commented Jan 11, 2022

dmantipov commented Jan 12, 2022

gluster-ant commented Jan 12, 2022

gluster-ant commented Jan 12, 2022

dmantipov commented Jan 12, 2022

dmantipov commented Jan 13, 2022

dmantipov commented Jan 14, 2022

gluster-ant commented Jan 14, 2022

dmantipov commented Jan 14, 2022

mykaul commented Jan 14, 2022

sac commented Jan 15, 2022

mscherer commented Jan 16, 2022

mscherer commented Jan 16, 2022

gluster-ant commented Jan 17, 2022

dmantipov commented Jan 17, 2022

build, libglusterfs, tests: use system xxhash if installed #3127

build, libglusterfs, tests: use system xxhash if installed #3127

Conversation

dmantipov commented Jan 11, 2022 • edited

gluster-ant commented Jan 11, 2022

dmantipov commented Jan 11, 2022

amarts commented Jan 11, 2022

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

gluster-ant commented Jan 11, 2022

sac commented Jan 11, 2022

gluster-ant commented Jan 11, 2022

dmantipov commented Jan 12, 2022

gluster-ant commented Jan 12, 2022

gluster-ant commented Jan 12, 2022

dmantipov commented Jan 12, 2022

dmantipov commented Jan 13, 2022

dmantipov commented Jan 14, 2022

gluster-ant commented Jan 14, 2022

dmantipov commented Jan 14, 2022

mykaul commented Jan 14, 2022

sac commented Jan 15, 2022

mscherer commented Jan 16, 2022

mscherer commented Jan 16, 2022

gluster-ant commented Jan 17, 2022

dmantipov commented Jan 17, 2022

dmantipov commented Jan 11, 2022 •

edited