Merge lp:~vkolesnikov/maria/maria-pbxt-bug-439889 into lp:~maria-captains/maria/5.1-converting

[Vladimir Kolesnikov]

[Vladimir Kolesnikov (vkolesnikov)]

The original problem happened because pbxt had dependencies on mysql structures in several sources but macro MYSQL_PLUGIN_DEPENDS_ON_MYSQL_INTERNALS did not work for > 1 files. So some of the dependent source were built with wrong options. After fixing this problem wrong object files were still used. This was because of the following: libmysqld build script extracts object files from all plugin libraries and puts them into a single temporary archive. Next, object code of the files that depend on mysql internals are put into that same archive in replacement mode, so that if we already have e.g. ha_pbxt.o (compiled for normal server) in the archive then it will be replaced with the correct version built for embedded server. The problem was that pbxt used CXXFLAGS automake variable in its Makefile.am and in such case automake automatically adds target-specific prefix to the output object file to avoid conflicts when the same source is built several times with different options. In our case we got something like libpbxt_a-ha_pbxt.o. The result was that the old ha_pbxt.o was not replaced in the archive and linker used symbols from there (by a chance I guess). The solution was not to use CXXFLAGS (we didn't really need them) but this is an undocumented limitation and might be a problem in future for other plugins too.

[Kristian Nielsen (knielsen)]

Looks good, thanks!

I suggest some changes, mainly to make the mysql-test-run --embedded test
suite work. I pushed these changes to a branch on Launchpad, please check them
out and see what you think:

    lp:~knielsen/maria/mariadb-pbxt-mbug439889

1. In embedded server, max_connections is set to 1. This causes problems since
   PBXT uses max_connections to create a default value for pbxt_max_threads. I
   suggest a small hack in the above branch to solve this case, but lets
   discuss if you have a better suggestion.

2. I changed a number of printf(...) in PBXT to fprintf(stderr,
   ...). Otherwise the output of mysql-test-run --embedded gets flooded with
   lots ouf PBXT output. It seems to me better in any case to use stderr for
   this, but again let me know if you have a better suggestion.

3. Update some .test and .result files for --embedded (two tests I disabled in
   --embedded due to SHOW PROCESSLIST and mysqldump usage).

4. I moved mysql-test/suite/pbxt/t/load_unique_error1.inc into
   mysql-test/std_data/, to make it usable in both normal and embedded tests
   (it seems embedded tests run with a different current directory). Do you
   think that is ok?

If you think these changes look ok (or have better suggestions), I will merge
them into MariaDB.

review: Approve

[Paul McCullagh (paul-mccullagh)]

Hi Kristian,

On Dec 22, 2009, at 1:04 PM, Kristian Nielsen wrote:

> Review: Approve
> Looks good, thanks!
>
> I suggest some changes, mainly to make the mysql-test-run --embedded
> test
> suite work. I pushed these changes to a branch on Launchpad, please
> check them
> out and see what you think:
>
> lp:~knielsen/maria/mariadb-pbxt-mbug439889
>
> 1. In embedded server, max_connections is set to 1. This causes
> problems since
> PBXT uses max_connections to create a default value for
> pbxt_max_threads. I
> suggest a small hack in the above branch to solve this case, but
> lets
> discuss if you have a better suggestion.

This fix should be OK for the moment. This problem is correctly solved
in PBXT 1.1 where the thread list has been made dynamic, and
pbxt_max_threads is no longer needed.

> 2. I changed a number of printf(...) in PBXT to fprintf(stderr,
> ...). Otherwise the output of mysql-test-run --embedded gets
> flooded with
> lots ouf PBXT output. It seems to me better in any case to use
> stderr for
> this, but again let me know if you have a better suggestion.

It does not make any difference to me, as long as the output is
directed to the .err log, when running in the usual server environment.

> 3. Update some .test and .result files for --embedded (two tests I
> disabled in
> --embedded due to SHOW PROCESSLIST and mysqldump usage).

OK.

> 4. I moved mysql-test/suite/pbxt/t/load_unique_error1.inc into
> mysql-test/std_data/, to make it usable in both normal and
> embedded tests
> (it seems embedded tests run with a different current directory).
> Do you
> think that is ok?

I guess there is no better way to do this at the moment.

> If you think these changes look ok (or have better suggestions), I
> will merge
> them into MariaDB.

Yes, please go ahead.

> --
> https://code.launchpad.net/~vkolesnikov/maria/maria-pbxt-
> bug-439889/+merge/15899
> Your team Maria-captains is subscribed to branch lp:maria.

--
Paul McCullagh
PrimeBase Technologies
www.primebase.org
www.blobstreaming.org
pbxt.blogspot.com

[Kristian Nielsen (knielsen)]

I have merged this into lp:~maria-captains/maria/5.1-release.
(This will go into the MariaDB 5.1.41 release and will later be merged into main lp:maria).

Thanks for the fix!

=== modified file 'config/ac-macros/plugins.m4'
--- config/ac-macros/plugins.m4	2009-04-25 10:05:32 +0000
+++ config/ac-macros/plugins.m4	2009-12-09 21:58:15 +0000
@@ -463,11 +463,13 @@
       mysql_plugin_defs="$mysql_plugin_defs, [builtin_]$2[_plugin]"
       [with_plugin_]$2=yes
       AC_MSG_RESULT([yes])
-      m4_ifdef([$11],[
-       condition_dependent_plugin_modules="$condition_dependent_plugin_modules m4_bregexp($11, [[^/]+$], [\&])"
-       condition_dependent_plugin_objects="$condition_dependent_plugin_objects m4_bregexp($11, [[^/]+\.], [\&o])"
-       condition_dependent_plugin_links="$condition_dependent_plugin_links $6/$11"
-       condition_dependent_plugin_includes="$condition_dependent_plugin_includes -I[\$(top_srcdir)]/$6/m4_bregexp($11, [^.+[/$]], [\&])"
+      m4_ifdef([$11], [
+        m4_foreach([plugin], [$11], [
+           condition_dependent_plugin_modules="$condition_dependent_plugin_modules m4_bregexp(plugin, [[^/]+$], [\&])"
+           condition_dependent_plugin_objects="$condition_dependent_plugin_objects m4_bregexp(plugin, [[^/]+\.], [\&o])"
+           condition_dependent_plugin_links="$condition_dependent_plugin_links $6/plugin"
+           condition_dependent_plugin_includes="$condition_dependent_plugin_includes -I[\$(top_srcdir)]/$6/m4_bregexp(plugin, [^.+[/$]], [\&])"
+        ])
       ])
     fi
   fi

=== modified file 'sql/sql_plugin.cc'
--- sql/sql_plugin.cc	2009-12-03 11:19:05 +0000
+++ sql/sql_plugin.cc	2009-12-09 21:58:15 +0000
@@ -1179,9 +1179,9 @@
            embedded server with different options than the regular server,
            the only way was to disable PBXT from here.
       */
-      if (!my_strnncoll(&my_charset_latin1, (const uchar*) plugin->name,
-                        4, (const uchar*) "PBXT", 4))
-        continue;
+      //if (!my_strnncoll(&my_charset_latin1, (const uchar*) plugin->name,
+      //                  4, (const uchar*) "PBXT", 4))
+      //  continue;
 
 #endif
       bzero(&tmp, sizeof(tmp));

=== modified file 'storage/pbxt/plug.in'
--- storage/pbxt/plug.in	2009-05-12 06:44:01 +0000
+++ storage/pbxt/plug.in	2009-12-09 21:58:15 +0000
@@ -5,3 +5,4 @@
 MYSQL_PLUGIN_ACTIONS(pbxt,  [
 #               AC_CONFIG_FILES(storage/pbxt/src/Makefile)
                ])
+MYSQL_PLUGIN_DEPENDS_ON_MYSQL_INTERNALS(pbxt, [[src/ha_pbxt.cc],[src/myxt_xt.cc],[src/discover_xt.cc]])

=== modified file 'storage/pbxt/src/Makefile.am'
--- storage/pbxt/src/Makefile.am	2009-10-07 07:40:56 +0000
+++ storage/pbxt/src/Makefile.am	2009-12-09 21:58:15 +0000
@@ -46,7 +46,5 @@
 EXTRA_LIBRARIES =		libpbxt.a
 noinst_LIBRARIES = 		libpbxt.a
 libpbxt_a_SOURCES =		$(libpbxt_la_SOURCES)
-libpbxt_a_CXXFLAGS =	$(AM_CXXFLAGS)
-libpbxt_a_CFLAGS =		$(AM_CFLAGS) -std=c99
 
 EXTRA_DIST =			pbms_enabled.cc win_inttypes.h