[3/3] kudu git commit: thirdparty: patch cmake on SLES 12 SP0 to avoid hanging

[ad...@apache.org]

thirdparty: patch cmake on SLES 12 SP0 to avoid hanging

There's an odd cmake bug [1] that causes it to hang from time to time. So
far we've only seen it manifest during Impala's native toolchain build
(which includes Kudu) on SLES 12 SP0 build machines. I myself wasn't able to
reproduce it using the same AMI. Nor has it manifested in any other build
environments (including SLES 12 SP1), to our knowledge.

The workaround is to disable the use of select() when reading from
subprocess' pipes. I'm not thrilled by this; it means cmake will poll those
pipes for data, adding to its overall time. On my laptop, the unpatched cmake
takes 10s to process the Kudu source tree and 15s with this patch. That's
not much compared to the time spent in make/ninja (even with a hot ccache),
but anecdotally it feels like it hurts the "interactivity" of the build.

To limit the damage, the patch is only applied if SLES 12 SP0 is detected.
To my knowledge, no one is developing for Kudu on that platform, so the net
outcome is a more robust Impala build without a real hit to productivity.

1. https://gitlab.kitware.com/cmake/cmake/issues/15873

Change-Id: I33596a36c1f974078140360cb62b01f58211b44c
Reviewed-on: http://gerrit.cloudera.org:8080/5857
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f3ec97bd
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f3ec97bd
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f3ec97bd

Branch: refs/heads/master
Commit: f3ec97bd16d99052420bd3b92ce7cdf92aa7fbe7
Parents: 00cf84c
Author: Adar Dembo <ad...@cloudera.com>
Authored: Wed Feb 1 14:00:46 2017 -0800
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Fri Feb 3 22:54:52 2017 +0000

----------------------------------------------------------------------
 thirdparty/download-thirdparty.sh               | 31 ++++++++++++++++++++
 .../cmake-issue-15873-dont-use-select.patch     | 14 +++++++++
 2 files changed, 45 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/f3ec97bd/thirdparty/download-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index c5c0b16..ff16ddf 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -147,8 +147,39 @@ if [ ! -d $PROTOBUF_SOURCE ]; then
   popd
 fi
 
+# Returns 0 if cmake should be patched to work around this bug [1].
+#
+# Currently only SLES 12 SP0 is known to be vulnerable, and since the workaround
+# hurts cmake performance, we apply it only if absolutely necessary.
+#
+# 1. https://gitlab.kitware.com/cmake/cmake/issues/15873.
+needs_patched_cmake() {
+  if [ ! -e /etc/SuSE-release ]; then
+    # Not a SUSE distro.
+    return 1
+  fi
+  if ! grep -q "SUSE Linux Enterprise Server 12" /etc/SuSE-release; then
+    # Not SLES 12.
+    return 1
+  fi
+  if ! grep -q "PATCHLEVEL = 0" /etc/SuSE-release; then
+    # Not SLES 12 SP0.
+    return 1
+  fi
+  return 0
+}
+
+CMAKE_PATCHLEVEL=1
+delete_if_wrong_patchlevel $CMAKE_SOURCE $CMAKE_PATCHLEVEL
 if [ ! -d $CMAKE_SOURCE ]; then
   fetch_and_expand cmake-${CMAKE_VERSION}.tar.gz
+  pushd $CMAKE_SOURCE
+  if needs_patched_cmake; then
+    patch -p1 < $TP_DIR/patches/cmake-issue-15873-dont-use-select.patch
+  fi
+  # Write the patchlevel file even if the patch was skipped, so as to simplify
+  # future patch numbering.
+  touch patchlevel-$CMAKE_PATCHLEVEL
 fi
 
 if [ ! -d $SNAPPY_SOURCE ]; then

http://git-wip-us.apache.org/repos/asf/kudu/blob/f3ec97bd/thirdparty/patches/cmake-issue-15873-dont-use-select.patch
----------------------------------------------------------------------
diff --git a/thirdparty/patches/cmake-issue-15873-dont-use-select.patch b/thirdparty/patches/cmake-issue-15873-dont-use-select.patch
new file mode 100644
index 0000000..c5d4c7f
--- /dev/null
+++ b/thirdparty/patches/cmake-issue-15873-dont-use-select.patch
@@ -0,0 +1,14 @@
+diff --git a/Source/kwsys/ProcessUNIX.c b/Source/kwsys/ProcessUNIX.c
+index 1be6d02..525217a 100644
+--- a/Source/kwsys/ProcessUNIX.c
++++ b/Source/kwsys/ProcessUNIX.c
+@@ -110,7 +110,8 @@ static inline void kwsysProcess_usleep(unsigned int msec)
+  * without select().
+  */
+ #if !defined(__BEOS__) && !defined(__VMS) && !defined(__MINT__)
+-# define KWSYSPE_USE_SELECT 1
++// Don't use select() due to https://cmake.org/Bug/view.php?id=15873
++# define KWSYSPE_USE_SELECT 0
+ #endif
+ 
+ /* Some platforms do not have siginfo on their signal handlers.  */