Merge lp:~sergei.glushchenko/percona-server/5.5-BT32840-ps-bug1196460 into lp:percona-server/5.5

[Sergei Glushchenko]

1. Introduce cmake option DISABLE_DTRACE
2. Invoke dtrace -G only for plugins and static libraries which need it

[Sergei Glushchenko (sergei.glushchenko)]

http://jenkins.percona.com/job/percona-server-5.5-param/783/

[Laurynas Biveinis (laurynas-biveinis)]

Why there is need to add -DDISABLE_DTRACE option in addition to the already-existing -DENABLE_DTRACE one?

review: Needs Information

[Sergei Glushchenko (sergei.glushchenko)]

Passing --enable-dtrace to cmake/configure won't take any effect. Moreover, providing --enable-dtrace would mean that dtrace is turned off by default, which changes current behavior. Currently it is turned ON by default. Current meaning of ENABLE_DTRACE variable is more appropriate to name HAVE_DTRACE.

[Laurynas Biveinis (laurynas-biveinis)]

What about adding -DENABLE_DTRACE=OFF to cmake?

Let's for a moment not consider the configure wrapper with --options.

[Sergei Glushchenko (sergei.glushchenko)]

What default value would ENABLE_DTRACE have?
If ON - we need to force it to OFF if dtrace is not supported.
If OFF - we will break current behavior.

[Sergei Glushchenko (sergei.glushchenko)]

I mean, the default behavior is to link dtrace wherever we find support for it. To produce dtrace-free binary even if support is present, I add DISABLE_DTRACE. I think it is reasonable.

[Alexey Kopytov (akopytov)]

Hi Sergei,

On Wed, 03 Jul 2013 13:12:27 -0000, Sergei Glushchenko wrote:
> What default value would ENABLE_DTRACE have?
> If ON - we need to force it to OFF if dtrace is not supported.
> If OFF - we will break current behavior.
>

The former I think is more reasonable than adding DISABLE_DTRACE. I
don't even understand what goal does -DDISABLE_DTRACE tries to achieve.
I.e. what's wrong with cmake -DENABLE_DTRACE=0?

[Sergei Glushchenko (sergei.glushchenko)]

Alexey, Laurynas,

Could you please provide me desired behavior for following combinations:
1. ENABLE_DTRACE=ON, dtrace support not found
2. ENABLE_DTRACE=ON, dtrace support found
3. ENABLE_DTRACE=OFF, dtrace support not found
4. ENABLE_DTRACE=OFF, dtrace support found
5. ENABLE_DTRACE not specified, dtrace support found
6. ENABLE_DTRACE not specified, dtrace support not found

I really don't know which behavior is desired for some of them.

Thanks,
Sergei

[Alexey Kopytov (akopytov)]

Hi Sergei,

On Fri, 05 Jul 2013 10:53:24 -0000, Sergei Glushchenko wrote:
> Alexey, Laurynas,
>
> Could you please provide me desired behavior for following combinations:
> 1. ENABLE_DTRACE=ON, dtrace support not found
> 2. ENABLE_DTRACE=ON, dtrace support found
> 3. ENABLE_DTRACE=OFF, dtrace support not found
> 4. ENABLE_DTRACE=OFF, dtrace support found
> 5. ENABLE_DTRACE not specified, dtrace support found
> 6. ENABLE_DTRACE not specified, dtrace support not found
>
> I really don't know which behavior is desired for some of them.
>

Build with DTrace support when possible (there's DTrace support) *and*
ENABLE_DTRACE=ON. When not specified, use the default value?

[Sergei Glushchenko (sergei.glushchenko)]

Yes, Alexey, this looks reasonable, but the question was mostly about which value is default? Doc states it is OFF, but the behavior is like I'd specified ENABLE_DTRACE=ON.

[Sergei Glushchenko (sergei.glushchenko)]

So, when I implement ENABLE_DTRACE=ON/OFF switch for user, should I set it OFF by default?

[Alexey Kopytov (akopytov)]

Hi Sergei,

On Mon, 08 Jul 2013 08:46:28 -0000, Sergei Glushchenko wrote:
> So, when I implement ENABLE_DTRACE=ON/OFF switch for user, should I set it OFF by default?
>

Yes, I think it should be OFF by default. But cmake should also fail
with an error when building with -DENABLE_DTRACE=ON and the dtrace
binary is not found.

[Sergei Glushchenko (sergei.glushchenko)]

Thank you, this is good for me. My concerns was about that by implementing this we breaking current behavior, which is undocumented, but somebody may already rely on it. If it is OK by you, it is fine by me as well.

[Sergei Glushchenko (sergei.glushchenko)]

Changes: ENABLE_DTRACE instead of DISABLE_DTRACE.

[Laurynas Biveinis (laurynas-biveinis)]

Approving. I'll hold off merging it for a couple of days, in case Alexey or somebody else has objections.

Sergei, consider submitting the patch under OCA to Oracle.

review: Approve

[Alexey Kopytov (akopytov)]

Looks good to me.

review: Approve

=== modified file 'Percona-Server/CMakeLists.txt'
--- Percona-Server/CMakeLists.txt	2013-06-26 07:01:13 +0000
+++ Percona-Server/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -170,6 +170,7 @@
   SET(WITHOUT_DYNAMIC_PLUGINS 1)
 ENDIF()
 OPTION(ENABLED_PROFILING "Enable profiling" ON)
+OPTION(ENABLE_DTRACE "Include support for DTrace probes" OFF)
 OPTION(CYBOZU "" OFF)
 OPTION(BACKUP_TEST "" OFF)
 OPTION(WITHOUT_SERVER OFF)

=== modified file 'Percona-Server/cmake/dtrace.cmake'
--- Percona-Server/cmake/dtrace.cmake	2012-06-15 01:35:09 +0000
+++ Percona-Server/cmake/dtrace.cmake	2013-07-08 12:39:32 +0000
@@ -35,10 +35,11 @@
  MARK_AS_ADVANCED(DTRACE)
 
  # On FreeBSD, dtrace does not handle userland tracing yet
- IF(DTRACE AND NOT CMAKE_SYSTEM_NAME MATCHES "FreeBSD"
-     AND NOT BUGGY_GCC_NO_DTRACE_MODULES)
-   SET(ENABLE_DTRACE ON CACHE BOOL "Enable dtrace")
+ IF(ENABLE_DTRACE AND (CMAKE_SYSTEM_NAME MATCHES "FreeBSD"
+    OR BUGGY_GCC_NO_DTRACE_MODULES OR NOT DTRACE))
+  MESSAGE(FATAL_ERROR "dtrace is not supported on this system")
  ENDIF()
+
  SET(HAVE_DTRACE ${ENABLE_DTRACE})
  IF(CMAKE_SYSTEM_NAME MATCHES "SunOS")
    IF(CMAKE_SIZEOF_VOID_P EQUAL 4)

=== modified file 'Percona-Server/cmake/plugin.cmake'
--- Percona-Server/cmake/plugin.cmake	2013-02-26 05:35:17 +0000
+++ Percona-Server/cmake/plugin.cmake	2013-07-08 12:39:32 +0000
@@ -21,6 +21,7 @@
 # [STORAGE_ENGINE]
 # [MANDATORY|DEFAULT]
 # [STATIC_ONLY|DYNAMIC_ONLY]
+# [DTRACE_INSTRUMENTED]
 # [MODULE_OUTPUT_NAME module_name]
 # [STATIC_OUTPUT_NAME static_name]
 # [RECOMPILE_FOR_EMBEDDED]
@@ -47,7 +48,7 @@
 MACRO(MYSQL_ADD_PLUGIN)
   MYSQL_PARSE_ARGUMENTS(ARG
     "LINK_LIBRARIES;DEPENDENCIES;MODULE_OUTPUT_NAME;STATIC_OUTPUT_NAME"
-    "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED"
+    "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED;DTRACE_INSTRUMENTED"
     ${ARGN}
   )
   
@@ -116,7 +117,9 @@
   IF (WITH_${plugin} AND NOT ARG_MODULE_ONLY)
     ADD_LIBRARY(${target} STATIC ${SOURCES})
     SET_TARGET_PROPERTIES(${target} PROPERTIES COMPILE_DEFINITONS "MYSQL_SERVER")
-    DTRACE_INSTRUMENT(${target})
+    IF (ARG_DTRACE_INSTRUMENTED)
+      DTRACE_INSTRUMENT(${target})
+    ENDIF()
     ADD_DEPENDENCIES(${target} GenError ${ARG_DEPENDENCIES})
     IF(WITH_EMBEDDED_SERVER)
       # Embedded library should contain PIC code and be linkable
@@ -124,7 +127,9 @@
       IF(ARG_RECOMPILE_FOR_EMBEDDED OR NOT _SKIP_PIC)
         # Recompile some plugins for embedded
         ADD_CONVENIENCE_LIBRARY(${target}_embedded ${SOURCES})
-        DTRACE_INSTRUMENT(${target}_embedded)   
+        IF (ARG_DTRACE_INSTRUMENTED)
+          DTRACE_INSTRUMENT(${target}_embedded)
+        ENDIF()
         IF(ARG_RECOMPILE_FOR_EMBEDDED)
           SET_TARGET_PROPERTIES(${target}_embedded 
             PROPERTIES COMPILE_DEFINITIONS "MYSQL_SERVER;EMBEDDED_LIBRARY")
@@ -170,7 +175,9 @@
 
     ADD_VERSION_INFO(${target} MODULE SOURCES)
     ADD_LIBRARY(${target} MODULE ${SOURCES}) 
-    DTRACE_INSTRUMENT(${target})
+    IF (ARG_DTRACE_INSTRUMENTED)
+      DTRACE_INSTRUMENT(${target})
+    ENDIF()
     SET_TARGET_PROPERTIES (${target} PROPERTIES PREFIX ""
       COMPILE_DEFINITIONS "MYSQL_DYNAMIC_PLUGIN")
     TARGET_LINK_LIBRARIES (${target} mysqlservices)

=== modified file 'Percona-Server/storage/archive/CMakeLists.txt'
--- Percona-Server/storage/archive/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/archive/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -14,5 +14,6 @@
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
 SET(ARCHIVE_SOURCES  azio.c ha_archive.cc ha_archive.h)
-MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE LINK_LIBRARIES ${ZLIB_LIBRARY})
+MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE
+                 LINK_LIBRARIES ${ZLIB_LIBRARY} DTRACE_INSTRUMENTED)
 

=== modified file 'Percona-Server/storage/blackhole/CMakeLists.txt'
--- Percona-Server/storage/blackhole/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/blackhole/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -14,4 +14,5 @@
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
 SET(BLACKHOLE_SOURCES  ha_blackhole.cc ha_blackhole.h)
-MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE)
+MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE
+                 DTRACE_INSTRUMENTED)

=== modified file 'Percona-Server/storage/csv/CMakeLists.txt'
--- Percona-Server/storage/csv/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/csv/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -17,4 +17,5 @@
 SET(CSV_PLUGIN_MANDATORY TRUE)
 
 SET(CSV_SOURCES  ha_tina.cc ha_tina.h transparent_file.cc transparent_file.h)
-MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE MANDATORY)
+MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE
+                 MANDATORY DTRACE_INSTRUMENTED)

=== modified file 'Percona-Server/storage/example/CMakeLists.txt'
--- Percona-Server/storage/example/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/example/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -15,4 +15,5 @@
 
 SET(EXAMPLE_PLUGIN_DYNAMIC "ha_example")
 SET(EXAMPLE_SOURCES ha_example.cc)
-MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY)
+MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY
+                 DTRACE_INSTRUMENTED)

=== modified file 'Percona-Server/storage/federated/CMakeLists.txt'
--- Percona-Server/storage/federated/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/federated/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -21,4 +21,5 @@
  # mysqld and are optimized away by the linker.
  SET(FEDERATED_SOURCES ${FEDERATED_SOURCES} ${CMAKE_SOURCE_DIR}/mysys/string.c)
 ENDIF()
-MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE)
+MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE
+                 DTRACE_INSTRUMENTED)

=== modified file 'Percona-Server/storage/heap/CMakeLists.txt'
--- Percona-Server/storage/heap/CMakeLists.txt	2012-04-18 23:26:28 +0000
+++ Percona-Server/storage/heap/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -23,7 +23,8 @@
 				hp_dspace.c hp_record.c
 				hp_rrnd.c hp_rsame.c hp_scan.c hp_static.c hp_update.c hp_write.c)
 
-MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)
+MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY
+                 RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)
 
 ADD_EXECUTABLE(hp_test1 hp_test1.c)
 TARGET_LINK_LIBRARIES(hp_test1 mysys heap dbug strings)

=== modified file 'Percona-Server/storage/myisam/CMakeLists.txt'
--- Percona-Server/storage/myisam/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/myisam/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -29,7 +29,8 @@
 MYSQL_ADD_PLUGIN(myisam ${MYISAM_SOURCES} 
   STORAGE_ENGINE 
   MANDATORY 
-  RECOMPILE_FOR_EMBEDDED)
+  RECOMPILE_FOR_EMBEDDED
+  DTRACE_INSTRUMENTED)
 
 TARGET_LINK_LIBRARIES(myisam mysys)
 

=== modified file 'Percona-Server/storage/myisammrg/CMakeLists.txt'
--- Percona-Server/storage/myisammrg/CMakeLists.txt	2011-06-30 15:46:53 +0000
+++ Percona-Server/storage/myisammrg/CMakeLists.txt	2013-07-08 12:39:32 +0000
@@ -20,4 +20,5 @@
 				myrg_rprev.c myrg_rrnd.c myrg_rsame.c myrg_static.c myrg_update.c
 				myrg_write.c myrg_records.c)
 
-MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)
+MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY
+                 RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)