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

Proposed by Sergei Glushchenko
Status: Merged
Approved by: Laurynas Biveinis
Approved revision: 545
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.

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

review: Needs Information

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.

What about adding -DENABLE_DTRACE=OFF to cmake?

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

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.

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) 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?

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) 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?

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.

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

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.

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.

Changes: ENABLE_DTRACE instead of DISABLE_DTRACE.

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) wrote :

Looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/CMakeLists.txt'
2--- Percona-Server/CMakeLists.txt 2013-06-26 07:01:13 +0000
3+++ Percona-Server/CMakeLists.txt 2013-07-08 12:39:32 +0000
4@@ -170,6 +170,7 @@
5 SET(WITHOUT_DYNAMIC_PLUGINS 1)
6 ENDIF()
7 OPTION(ENABLED_PROFILING "Enable profiling" ON)
8+OPTION(ENABLE_DTRACE "Include support for DTrace probes" OFF)
9 OPTION(CYBOZU "" OFF)
10 OPTION(BACKUP_TEST "" OFF)
11 OPTION(WITHOUT_SERVER OFF)
12
13=== modified file 'Percona-Server/cmake/dtrace.cmake'
14--- Percona-Server/cmake/dtrace.cmake 2012-06-15 01:35:09 +0000
15+++ Percona-Server/cmake/dtrace.cmake 2013-07-08 12:39:32 +0000
16@@ -35,10 +35,11 @@
17 MARK_AS_ADVANCED(DTRACE)
18
19 # On FreeBSD, dtrace does not handle userland tracing yet
20- IF(DTRACE AND NOT CMAKE_SYSTEM_NAME MATCHES "FreeBSD"
21- AND NOT BUGGY_GCC_NO_DTRACE_MODULES)
22- SET(ENABLE_DTRACE ON CACHE BOOL "Enable dtrace")
23+ IF(ENABLE_DTRACE AND (CMAKE_SYSTEM_NAME MATCHES "FreeBSD"
24+ OR BUGGY_GCC_NO_DTRACE_MODULES OR NOT DTRACE))
25+ MESSAGE(FATAL_ERROR "dtrace is not supported on this system")
26 ENDIF()
27+
28 SET(HAVE_DTRACE ${ENABLE_DTRACE})
29 IF(CMAKE_SYSTEM_NAME MATCHES "SunOS")
30 IF(CMAKE_SIZEOF_VOID_P EQUAL 4)
31
32=== modified file 'Percona-Server/cmake/plugin.cmake'
33--- Percona-Server/cmake/plugin.cmake 2013-02-26 05:35:17 +0000
34+++ Percona-Server/cmake/plugin.cmake 2013-07-08 12:39:32 +0000
35@@ -21,6 +21,7 @@
36 # [STORAGE_ENGINE]
37 # [MANDATORY|DEFAULT]
38 # [STATIC_ONLY|DYNAMIC_ONLY]
39+# [DTRACE_INSTRUMENTED]
40 # [MODULE_OUTPUT_NAME module_name]
41 # [STATIC_OUTPUT_NAME static_name]
42 # [RECOMPILE_FOR_EMBEDDED]
43@@ -47,7 +48,7 @@
44 MACRO(MYSQL_ADD_PLUGIN)
45 MYSQL_PARSE_ARGUMENTS(ARG
46 "LINK_LIBRARIES;DEPENDENCIES;MODULE_OUTPUT_NAME;STATIC_OUTPUT_NAME"
47- "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED"
48+ "STORAGE_ENGINE;STATIC_ONLY;MODULE_ONLY;MANDATORY;DEFAULT;DISABLED;RECOMPILE_FOR_EMBEDDED;DTRACE_INSTRUMENTED"
49 ${ARGN}
50 )
51
52@@ -116,7 +117,9 @@
53 IF (WITH_${plugin} AND NOT ARG_MODULE_ONLY)
54 ADD_LIBRARY(${target} STATIC ${SOURCES})
55 SET_TARGET_PROPERTIES(${target} PROPERTIES COMPILE_DEFINITONS "MYSQL_SERVER")
56- DTRACE_INSTRUMENT(${target})
57+ IF (ARG_DTRACE_INSTRUMENTED)
58+ DTRACE_INSTRUMENT(${target})
59+ ENDIF()
60 ADD_DEPENDENCIES(${target} GenError ${ARG_DEPENDENCIES})
61 IF(WITH_EMBEDDED_SERVER)
62 # Embedded library should contain PIC code and be linkable
63@@ -124,7 +127,9 @@
64 IF(ARG_RECOMPILE_FOR_EMBEDDED OR NOT _SKIP_PIC)
65 # Recompile some plugins for embedded
66 ADD_CONVENIENCE_LIBRARY(${target}_embedded ${SOURCES})
67- DTRACE_INSTRUMENT(${target}_embedded)
68+ IF (ARG_DTRACE_INSTRUMENTED)
69+ DTRACE_INSTRUMENT(${target}_embedded)
70+ ENDIF()
71 IF(ARG_RECOMPILE_FOR_EMBEDDED)
72 SET_TARGET_PROPERTIES(${target}_embedded
73 PROPERTIES COMPILE_DEFINITIONS "MYSQL_SERVER;EMBEDDED_LIBRARY")
74@@ -170,7 +175,9 @@
75
76 ADD_VERSION_INFO(${target} MODULE SOURCES)
77 ADD_LIBRARY(${target} MODULE ${SOURCES})
78- DTRACE_INSTRUMENT(${target})
79+ IF (ARG_DTRACE_INSTRUMENTED)
80+ DTRACE_INSTRUMENT(${target})
81+ ENDIF()
82 SET_TARGET_PROPERTIES (${target} PROPERTIES PREFIX ""
83 COMPILE_DEFINITIONS "MYSQL_DYNAMIC_PLUGIN")
84 TARGET_LINK_LIBRARIES (${target} mysqlservices)
85
86=== modified file 'Percona-Server/storage/archive/CMakeLists.txt'
87--- Percona-Server/storage/archive/CMakeLists.txt 2011-06-30 15:46:53 +0000
88+++ Percona-Server/storage/archive/CMakeLists.txt 2013-07-08 12:39:32 +0000
89@@ -14,5 +14,6 @@
90 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
91
92 SET(ARCHIVE_SOURCES azio.c ha_archive.cc ha_archive.h)
93-MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE LINK_LIBRARIES ${ZLIB_LIBRARY})
94+MYSQL_ADD_PLUGIN(archive ${ARCHIVE_SOURCES} STORAGE_ENGINE
95+ LINK_LIBRARIES ${ZLIB_LIBRARY} DTRACE_INSTRUMENTED)
96
97
98=== modified file 'Percona-Server/storage/blackhole/CMakeLists.txt'
99--- Percona-Server/storage/blackhole/CMakeLists.txt 2011-06-30 15:46:53 +0000
100+++ Percona-Server/storage/blackhole/CMakeLists.txt 2013-07-08 12:39:32 +0000
101@@ -14,4 +14,5 @@
102 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
103
104 SET(BLACKHOLE_SOURCES ha_blackhole.cc ha_blackhole.h)
105-MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE)
106+MYSQL_ADD_PLUGIN(blackhole ${BLACKHOLE_SOURCES} STORAGE_ENGINE
107+ DTRACE_INSTRUMENTED)
108
109=== modified file 'Percona-Server/storage/csv/CMakeLists.txt'
110--- Percona-Server/storage/csv/CMakeLists.txt 2011-06-30 15:46:53 +0000
111+++ Percona-Server/storage/csv/CMakeLists.txt 2013-07-08 12:39:32 +0000
112@@ -17,4 +17,5 @@
113 SET(CSV_PLUGIN_MANDATORY TRUE)
114
115 SET(CSV_SOURCES ha_tina.cc ha_tina.h transparent_file.cc transparent_file.h)
116-MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE MANDATORY)
117+MYSQL_ADD_PLUGIN(csv ${CSV_SOURCES} STORAGE_ENGINE
118+ MANDATORY DTRACE_INSTRUMENTED)
119
120=== modified file 'Percona-Server/storage/example/CMakeLists.txt'
121--- Percona-Server/storage/example/CMakeLists.txt 2011-06-30 15:46:53 +0000
122+++ Percona-Server/storage/example/CMakeLists.txt 2013-07-08 12:39:32 +0000
123@@ -15,4 +15,5 @@
124
125 SET(EXAMPLE_PLUGIN_DYNAMIC "ha_example")
126 SET(EXAMPLE_SOURCES ha_example.cc)
127-MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY)
128+MYSQL_ADD_PLUGIN(example ${EXAMPLE_SOURCES} STORAGE_ENGINE MODULE_ONLY
129+ DTRACE_INSTRUMENTED)
130
131=== modified file 'Percona-Server/storage/federated/CMakeLists.txt'
132--- Percona-Server/storage/federated/CMakeLists.txt 2011-06-30 15:46:53 +0000
133+++ Percona-Server/storage/federated/CMakeLists.txt 2013-07-08 12:39:32 +0000
134@@ -21,4 +21,5 @@
135 # mysqld and are optimized away by the linker.
136 SET(FEDERATED_SOURCES ${FEDERATED_SOURCES} ${CMAKE_SOURCE_DIR}/mysys/string.c)
137 ENDIF()
138-MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE)
139+MYSQL_ADD_PLUGIN(federated ${FEDERATED_SOURCES} STORAGE_ENGINE
140+ DTRACE_INSTRUMENTED)
141
142=== modified file 'Percona-Server/storage/heap/CMakeLists.txt'
143--- Percona-Server/storage/heap/CMakeLists.txt 2012-04-18 23:26:28 +0000
144+++ Percona-Server/storage/heap/CMakeLists.txt 2013-07-08 12:39:32 +0000
145@@ -23,7 +23,8 @@
146 hp_dspace.c hp_record.c
147 hp_rrnd.c hp_rsame.c hp_scan.c hp_static.c hp_update.c hp_write.c)
148
149-MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)
150+MYSQL_ADD_PLUGIN(heap ${HEAP_SOURCES} STORAGE_ENGINE MANDATORY
151+ RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)
152
153 ADD_EXECUTABLE(hp_test1 hp_test1.c)
154 TARGET_LINK_LIBRARIES(hp_test1 mysys heap dbug strings)
155
156=== modified file 'Percona-Server/storage/myisam/CMakeLists.txt'
157--- Percona-Server/storage/myisam/CMakeLists.txt 2011-06-30 15:46:53 +0000
158+++ Percona-Server/storage/myisam/CMakeLists.txt 2013-07-08 12:39:32 +0000
159@@ -29,7 +29,8 @@
160 MYSQL_ADD_PLUGIN(myisam ${MYISAM_SOURCES}
161 STORAGE_ENGINE
162 MANDATORY
163- RECOMPILE_FOR_EMBEDDED)
164+ RECOMPILE_FOR_EMBEDDED
165+ DTRACE_INSTRUMENTED)
166
167 TARGET_LINK_LIBRARIES(myisam mysys)
168
169
170=== modified file 'Percona-Server/storage/myisammrg/CMakeLists.txt'
171--- Percona-Server/storage/myisammrg/CMakeLists.txt 2011-06-30 15:46:53 +0000
172+++ Percona-Server/storage/myisammrg/CMakeLists.txt 2013-07-08 12:39:32 +0000
173@@ -20,4 +20,5 @@
174 myrg_rprev.c myrg_rrnd.c myrg_rsame.c myrg_static.c myrg_update.c
175 myrg_write.c myrg_records.c)
176
177-MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY RECOMPILE_FOR_EMBEDDED)
178+MYSQL_ADD_PLUGIN(myisammrg ${MYISAMMRG_SOURCES} STORAGE_ENGINE MANDATORY
179+ RECOMPILE_FOR_EMBEDDED DTRACE_INSTRUMENTED)

Subscribers

People subscribed via source and target branches