Percona Server moved to https://jira.percona.com/projects/PS

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

  1. 5.5-BT32840-ps-bug1196460
  2. Merge into 5.5
Proposed by Sergei Glushchenko
Status: Merged
Approved by: Laurynas Biveinis
Approved revision: no longer in the source branch.
Merged at revision: 553
Proposed branch: lp:~sergei.glushchenko/percona-server/5.5-BT32840-ps-bug1196460
Merge into: lp:percona-server/5.5
Diff against target: 179 lines (+32/-15)
11 files modified
Percona-Server/CMakeLists.txt (+1/-0)
Percona-Server/cmake/dtrace.cmake (+4/-3)
Percona-Server/cmake/plugin.cmake (+11/-4)
Percona-Server/storage/archive/CMakeLists.txt (+2/-1)
Percona-Server/storage/blackhole/CMakeLists.txt (+2/-1)
Percona-Server/storage/csv/CMakeLists.txt (+2/-1)
Percona-Server/storage/example/CMakeLists.txt (+2/-1)
Percona-Server/storage/federated/CMakeLists.txt (+2/-1)
Percona-Server/storage/heap/CMakeLists.txt (+2/-1)
Percona-Server/storage/myisam/CMakeLists.txt (+2/-1)
Percona-Server/storage/myisammrg/CMakeLists.txt (+2/-1)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-server/5.5-BT32840-ps-bug1196460
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Laurynas Biveinis (community) Approve
Review via email: mp+172579@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

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

review: Needs Information
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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.

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

What about adding -DENABLE_DTRACE=OFF to cmake?

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

Revision history for this message
Sergei Glushchenko (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.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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.

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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?

Revision history for this message
Sergei Glushchenko (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.

Thanks,
Sergei

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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?

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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.

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Changes: ENABLE_DTRACE instead of DISABLE_DTRACE.

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

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
Revision history for this message
Alexey Kopytov (akopytov) wrote :

Looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
170 SET(WITHOUT_DYNAMIC_PLUGINS 1)170 SET(WITHOUT_DYNAMIC_PLUGINS 1)
171ENDIF()171ENDIF()
172OPTION(ENABLED_PROFILING "Enable profiling" ON)172OPTION(ENABLED_PROFILING "Enable profiling" ON)
173OPTION(ENABLE_DTRACE "Include support for DTrace probes" OFF)
173OPTION(CYBOZU "" OFF)174OPTION(CYBOZU "" OFF)
174OPTION(BACKUP_TEST "" OFF)175OPTION(BACKUP_TEST "" OFF)
175OPTION(WITHOUT_SERVER OFF)176OPTION(WITHOUT_SERVER OFF)
176177
=== 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 @@
35 MARK_AS_ADVANCED(DTRACE)35 MARK_AS_ADVANCED(DTRACE)
3636
37 # On FreeBSD, dtrace does not handle userland tracing yet37 # On FreeBSD, dtrace does not handle userland tracing yet
38 IF(DTRACE AND NOT CMAKE_SYSTEM_NAME MATCHES "FreeBSD"38 IF(ENABLE_DTRACE AND (CMAKE_SYSTEM_NAME MATCHES "FreeBSD"
39 AND NOT BUGGY_GCC_NO_DTRACE_MODULES)39 OR BUGGY_GCC_NO_DTRACE_MODULES OR NOT DTRACE))
40 SET(ENABLE_DTRACE ON CACHE BOOL "Enable dtrace")40 MESSAGE(FATAL_ERROR "dtrace is not supported on this system")
41 ENDIF()41 ENDIF()
42
42 SET(HAVE_DTRACE ${ENABLE_DTRACE})43 SET(HAVE_DTRACE ${ENABLE_DTRACE})
43 IF(CMAKE_SYSTEM_NAME MATCHES "SunOS")44 IF(CMAKE_SYSTEM_NAME MATCHES "SunOS")
44 IF(CMAKE_SIZEOF_VOID_P EQUAL 4)45 IF(CMAKE_SIZEOF_VOID_P EQUAL 4)
4546
=== 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 @@
21# [STORAGE_ENGINE]21# [STORAGE_ENGINE]
22# [MANDATORY|DEFAULT]22# [MANDATORY|DEFAULT]
23# [STATIC_ONLY|DYNAMIC_ONLY]23# [STATIC_ONLY|DYNAMIC_ONLY]
24# [DTRACE_INSTRUMENTED]
24# [MODULE_OUTPUT_NAME module_name]25# [MODULE_OUTPUT_NAME module_name]
25# [STATIC_OUTPUT_NAME static_name]26# [STATIC_OUTPUT_NAME static_name]
26# [RECOMPILE_FOR_EMBEDDED]27# [RECOMPILE_FOR_EMBEDDED]
@@ -47,7 +48,7 @@
47MACRO(MYSQL_ADD_PLUGIN)48MACRO(MYSQL_ADD_PLUGIN)
48 MYSQL_PARSE_ARGUMENTS(ARG49 MYSQL_PARSE_ARGUMENTS(ARG
49 "LINK_LIBRARIES;DEPENDENCIES;MODULE_OUTPUT_NAME;STATIC_OUTPUT_NAME"50 "LINK_LIBRARIES;DEPENDENCIES;MODULE_OUTPUT_NAME;STATIC_OUTPUT_NAME"
50 "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED"51 "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED;DTRACE_INSTRUMENTED"
51 ${ARGN}52 ${ARGN}
52 )53 )
53 54
@@ -116,7 +117,9 @@
116 IF (WITH_${plugin} AND NOT ARG_MODULE_ONLY)117 IF (WITH_${plugin} AND NOT ARG_MODULE_ONLY)
117 ADD_LIBRARY(${target} STATIC ${SOURCES})118 ADD_LIBRARY(${target} STATIC ${SOURCES})
118 SET_TARGET_PROPERTIES(${target} PROPERTIES COMPILE_DEFINITONS "MYSQL_SERVER")119 SET_TARGET_PROPERTIES(${target} PROPERTIES COMPILE_DEFINITONS "MYSQL_SERVER")
119 DTRACE_INSTRUMENT(${target})120 IF (ARG_DTRACE_INSTRUMENTED)
121 DTRACE_INSTRUMENT(${target})
122 ENDIF()
120 ADD_DEPENDENCIES(${target} GenError ${ARG_DEPENDENCIES})123 ADD_DEPENDENCIES(${target} GenError ${ARG_DEPENDENCIES})
121 IF(WITH_EMBEDDED_SERVER)124 IF(WITH_EMBEDDED_SERVER)
122 # Embedded library should contain PIC code and be linkable125 # Embedded library should contain PIC code and be linkable
@@ -124,7 +127,9 @@
124 IF(ARG_RECOMPILE_FOR_EMBEDDED OR NOT _SKIP_PIC)127 IF(ARG_RECOMPILE_FOR_EMBEDDED OR NOT _SKIP_PIC)
125 # Recompile some plugins for embedded128 # Recompile some plugins for embedded
126 ADD_CONVENIENCE_LIBRARY(${target}_embedded ${SOURCES})129 ADD_CONVENIENCE_LIBRARY(${target}_embedded ${SOURCES})
127 DTRACE_INSTRUMENT(${target}_embedded) 130 IF (ARG_DTRACE_INSTRUMENTED)
131 DTRACE_INSTRUMENT(${target}_embedded)
132 ENDIF()
128 IF(ARG_RECOMPILE_FOR_EMBEDDED)133 IF(ARG_RECOMPILE_FOR_EMBEDDED)
129 SET_TARGET_PROPERTIES(${target}_embedded 134 SET_TARGET_PROPERTIES(${target}_embedded
130 PROPERTIES COMPILE_DEFINITIONS "MYSQL_SERVER;EMBEDDED_LIBRARY")135 PROPERTIES COMPILE_DEFINITIONS "MYSQL_SERVER;EMBEDDED_LIBRARY")
@@ -170,7 +175,9 @@
170175
171 ADD_VERSION_INFO(${target} MODULE SOURCES)176 ADD_VERSION_INFO(${target} MODULE SOURCES)
172 ADD_LIBRARY(${target} MODULE ${SOURCES}) 177 ADD_LIBRARY(${target} MODULE ${SOURCES})
173 DTRACE_INSTRUMENT(${target})178 IF (ARG_DTRACE_INSTRUMENTED)
179 DTRACE_INSTRUMENT(${target})
180 ENDIF()
174 SET_TARGET_PROPERTIES (${target} PROPERTIES PREFIX ""181 SET_TARGET_PROPERTIES (${target} PROPERTIES PREFIX ""
175 COMPILE_DEFINITIONS "MYSQL_DYNAMIC_PLUGIN")182 COMPILE_DEFINITIONS "MYSQL_DYNAMIC_PLUGIN")
176 TARGET_LINK_LIBRARIES (${target} mysqlservices)183 TARGET_LINK_LIBRARIES (${target} mysqlservices)
177184
=== 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 @@
14# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA14# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
1515
16SET(ARCHIVE_SOURCES azio.c ha_archive.cc ha_archive.h)16SET(ARCHIVE_SOURCES azio.c ha_archive.cc ha_archive.h)
17MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE LINK_LIBRARIES ${ZLIB_LIBRARY})17MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE
18 LINK_LIBRARIES ${ZLIB_LIBRARY} DTRACE_INSTRUMENTED)
1819
1920
=== 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 @@
14# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA14# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
1515
16SET(BLACKHOLE_SOURCES ha_blackhole.cc ha_blackhole.h)16SET(BLACKHOLE_SOURCES ha_blackhole.cc ha_blackhole.h)
17MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE)17MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE
18 DTRACE_INSTRUMENTED)
1819
=== 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 @@
17SET(CSV_PLUGIN_MANDATORY TRUE)17SET(CSV_PLUGIN_MANDATORY TRUE)
1818
19SET(CSV_SOURCES ha_tina.cc ha_tina.h transparent_file.cc transparent_file.h)19SET(CSV_SOURCES ha_tina.cc ha_tina.h transparent_file.cc transparent_file.h)
20MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE MANDATORY)20MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE
21 MANDATORY DTRACE_INSTRUMENTED)
2122
=== 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 @@
1515
16SET(EXAMPLE_PLUGIN_DYNAMIC "ha_example")16SET(EXAMPLE_PLUGIN_DYNAMIC "ha_example")
17SET(EXAMPLE_SOURCES ha_example.cc)17SET(EXAMPLE_SOURCES ha_example.cc)
18MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY)18MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY
19 DTRACE_INSTRUMENTED)
1920
=== 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 @@
21 # mysqld and are optimized away by the linker.21 # mysqld and are optimized away by the linker.
22 SET(FEDERATED_SOURCES ${FEDERATED_SOURCES} ${CMAKE_SOURCE_DIR}/mysys/string.c)22 SET(FEDERATED_SOURCES ${FEDERATED_SOURCES} ${CMAKE_SOURCE_DIR}/mysys/string.c)
23ENDIF()23ENDIF()
24MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE)24MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE
25 DTRACE_INSTRUMENTED)
2526
=== 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 @@
23 hp_dspace.c hp_record.c23 hp_dspace.c hp_record.c
24 hp_rrnd.c hp_rsame.c hp_scan.c hp_static.c hp_update.c hp_write.c)24 hp_rrnd.c hp_rsame.c hp_scan.c hp_static.c hp_update.c hp_write.c)
2525
26MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)26MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY
27 RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)
2728
28ADD_EXECUTABLE(hp_test1 hp_test1.c)29ADD_EXECUTABLE(hp_test1 hp_test1.c)
29TARGET_LINK_LIBRARIES(hp_test1 mysys heap dbug strings)30TARGET_LINK_LIBRARIES(hp_test1 mysys heap dbug strings)
3031
=== 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 @@
29MYSQL_ADD_PLUGIN(myisam ${MYISAM_SOURCES} 29MYSQL_ADD_PLUGIN(myisam ${MYISAM_SOURCES}
30 STORAGE_ENGINE 30 STORAGE_ENGINE
31 MANDATORY 31 MANDATORY
32 RECOMPILE_FOR_EMBEDDED)32 RECOMPILE_FOR_EMBEDDED
33 DTRACE_INSTRUMENTED)
3334
34TARGET_LINK_LIBRARIES(myisam mysys)35TARGET_LINK_LIBRARIES(myisam mysys)
3536
3637
=== 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 @@
20 myrg_rprev.c myrg_rrnd.c myrg_rsame.c myrg_static.c myrg_update.c20 myrg_rprev.c myrg_rrnd.c myrg_rsame.c myrg_static.c myrg_update.c
21 myrg_write.c myrg_records.c)21 myrg_write.c myrg_records.c)
2222
23MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)23MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY
24 RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)

Subscribers

People subscribed via source and target branches