On Wednesday 27 January 2010 11:40:33 Gary R. Schmidt wrote: > Kern Sibbald wrote: > [SNIP] > > > The purpose of SQL_LIB is to be able to determine if Batch insert will > > work or not. SQL_LIB should *never* be returned pointing to a .so file. > > If it does, batch insert will be turned off in *all* cases. So any patch > > that returns a .so for SQL_LIB will break Bacula for all platforms as did > > the orginally submitted patch that you mention above. > > Sigh. > > There is no .a library supplied with the WebStack MySQL. As of Solaris > 10 there are no static system libraries. WebStack is a Sun delivery, so > it follows the internal rules of only supplying shared objects.
Then there is no way for us to check 100% that it is thread save, which means that with the current, conservative logic there is no way to turn on batch insert. Theoretically if the library name ends in _r.so, it should be thread safe, but we currently do not have any code to check for that. If I am not mistaken, we now have runtime code that checks whether or not the library is thread safe (absolutely required for batch insert), but I think it is only implemented for Postgesql. Eric can confirm this. If we have valid runtime code for all libraries, we could remove the current checks during configure time that require the .a. > > > Batch insert works very well on all versions of MySQL and PostgreSQL, > > except for one or two MySQL versions that are broken. Unfortunately it > > seems to be one of the more common versions used on Solaris. > > And it works perfectly well on Solaris 10 with the .so library. At the current time, we have no way to verify that without the .a. If someone forgets to make it thread safe, sooner or later with batch insert enabled you will end up with a broken database, which for backup software is a real catastrophe. > > > If you want to disable batch insert, the proper way to do it is by adding > > a .configure option. > > Enable, enable, enable. Not disable, not disable, not disable. You can do that if you want, which is one of the great advantages of open source, but we do not plan to ship code that we know can destroy databases because it uses multiple threads and the sql library does not have them enabled. > > A fix to allow Bacula to enable the batch insert facility where MySQL > has only supplied a shared object is represented in this context diff: There are several problems with the patch below: 1. Configure is a script that is generated from configure.in, so any change to it must be made in configure.in (or one of the .m4 routines that it calls). 2. I only read unified diffs (i.e. you must put the -u option) otherwise, the context is not clear, which is the case below, so I was unable to understand what you are proposing. 3. Patches must be sent in attachements to avoid the fatal consequences of email program line wrapping. > *** bacula-5.0.0-orig/configure Mon Jan 25 18:56:28 2010 > --- bacula-5.0.0/configure Wed Jan 27 21:23:13 2010 > *************** > *** 23019,23032 **** > fi > fi > SQL_INCLUDE=-I$MYSQL_INCDIR > ! if test -f $MYSQL_LIBDIR/libmysqlclient_r.a \ > ! -o -f $MYSQL_LIBDIR/libmysqlclient_r.so; then > SQL_LFLAGS="-L$MYSQL_LIBDIR -lmysqlclient_r -lz" > $as_echo "#define HAVE_THREAD_SAFE_MYSQL 1" >>confdefs.h > ! > fi > SQL_BINDIR=$MYSQL_BINDIR > - SQL_LIB=$MYSQL_LIBDIR/libmysqlclient_r.a > > > $as_echo "#define HAVE_MYSQL 1" >>confdefs.h > --- 23019,23036 ---- > fi > fi > SQL_INCLUDE=-I$MYSQL_INCDIR > ! if test -f $MYSQL_LIBDIR/libmysqlclient_r.a; then > SQL_LFLAGS="-L$MYSQL_LIBDIR -lmysqlclient_r -lz" > + SQL_LIB=$MYSQL_LIBDIR/libmysqlclient_r.a > $as_echo "#define HAVE_THREAD_SAFE_MYSQL 1" >>confdefs.h > ! elif test -f $MYSQL_LIBDIR/libmysqlclient_r.so; then > ! SQL_LFLAGS="-L$MYSQL_LIBDIR -lmysqlclient_r -lz" > ! SQL_LIB=$MYSQL_LIBDIR/libmysqlclient_r.so > ! $as_echo "#define HAVE_THREAD_SAFE_MYSQL 1" >>confdefs.h > ! else > ! SQL_LIB=$MYSQL_LIBDIR/libmysqlclient.a > fi > SQL_BINDIR=$MYSQL_BINDIR > > > $as_echo "#define HAVE_MYSQL 1" >>confdefs.h > *************** > > The change is required because of the following lines: > 23665:A=`test -f $SQL_LIB && nm $SQL_LIB | grep pthread_mutex_lock` > 23866:A=`test -f $SQL_LIB && nm $SQL_LIB | grep pthread_mutex_lock` > 23871:A=`test -f $SQL_LIB && nm $DB_PROG_LIB | grep pthread_mutex_lock` > > The test for batch insert (re-?)occurs *after* SQL_LIB has been set. > > Thinking about it, if this particular mistake in configuration occurs in > the other database sections of the configure file, it will bite on any > database that is supplied with only a shared object. The current code avoids a catastropic situation. If you have a valid fix that does not disable the check for thread safe libraries we will be happy to take it, but if it turns off the check and thus totally disables batch insert as the originally submitted patch did, or if it allows turning on batch insert without sufficient checks, then the patch is not acceptable. Best regards, Kern ------------------------------------------------------------------------------ The Planet: dedicated and managed hosting, cloud storage, colocation Stay online with enterprise data centers and the best network in the business Choose flexible plans and management services without long-term contracts Personal 24x7 support from experience hosting pros just a phone call away. http://p.sf.net/sfu/theplanet-com _______________________________________________ Bacula-devel mailing list Bacula-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bacula-devel
