On Monday, October 29, 2018 at 5:48:46 AM UTC-7, Jun Omae wrote: > > On Sun, Oct 28, 2018 at 7:09 PM Dan <[email protected]> wrote: > > > > To preface, the MySqlDb documentation for Trac states the following: > > > >> Tables must be created as InnoDB or TokuDB type tables, because Trac > uses a transaction mechanism that is not supported by MyISAM tables > > > > > > I've created all my tables using InnoDB and installed Trac 1.2.3. > > > > During trac-admin hotcopy, just before files are copied, trac-admin > executes the following to lock the database: > > > > trac/env.py:1056: # Bogus statement to lock the database while > copying files > > trac/env.py-1057- with self.env.db_transaction as db: > > trac/env.py-1058- db("UPDATE system SET name=NULL WHERE name > IS NULL") > > > > After the file copy completes, the database is still locked with the > statement above, and it starts the mysqldump; building the mysqldump > command line from the database URL. The initial value is set here: > > > > trac/db/mysql_backend.py:243: args = [self.mysqldump_path, > '--no-defaults'] > > > > Note the '--no-defaults' in args[1]. > > > > With InnoDB, mysqldump should be run with '--single-transaction', which > is set in the appropriate my.cnf, but since '--no-defaults' is hard-coded, > all my.cnf files are ignored by default. > > > > As a result, mysqldump tries to lock the database before running the > dump. However, the earlier statement is still holding the lock. This never > exists and you have to manually kill the dump or the session holding the > earlier lock. > > > > In the DatabaseBackend documentation for MySQL, there is mention of a > parameter 'read_default_file', which you can include in your 'database' > URL, which should then tell mysqldump to read that my.cnf. > > > > However, when set, it is added to the same constructed mysqldump command > line started in the code above with the following: > > > > trac/db/mysql_backend.py-255- elif name == > 'read_default_file': # Must be first > > trac/db/mysql_backend.py:256: args.insert(1, > '--defaults-file=' + value) > > > > This correctly makes '--defaults-file' first, but it pushes the original > first argument ('--no-defaults') to the second argument. These arguments > are mutually exclusive with any version of mysqldump I could find, > resulting in the confusing error: > > > > $ mysqldump --defaults-file=trac/conf/my.cnf --no-defaults > > mysqldump: unknown option '--no-defaults' > > > > I had to apply the following patch to resolve this: > > > > --- db/mysql_backend.py.orig 2018-10-27 15:19:15.285092905 -0700 > > +++ db/mysql_backend.py 2018-10-27 15:11:36.138342337 -0700 > > @@ -255,3 +255,3 @@ > > elif name == 'read_default_file': # Must be first > > - args.insert(1, '--defaults-file=' + value) > > + args[1] = '--defaults-file=' + value > > elif name == 'unix_socket': > > > > The first argument should be replaced, not shifted. This patch and > including 'read_default_file' the database URL resolves the issue. > > > > However, it seems to me that if Trac requires a transactional database, > it should, by default, work with that transactional database. Since the > database is already locked by the earlier statement, Trac could even just > disable all mysqldump locking by default, but ideally the initial lock > should be released once the mysqldump transaction starts to avoid lengthy > and unnecessary database locks. > > > > I also question the use of '--no-defaults' at all. I have 'mysqldump' > working in all other situations, but Trac creates an unexpected situation > where both ~/.my.cnf and /etc/my.cnf are ignored, resulting in obscure > failures. > > > > Has anyone experienced a similar issue or had this work without issue / > patching? > > Ok. You're right. > > It seems that --no-defaults option with the read_default_file > parameter in #12880 is not tested. > Also, I consider we should add --single-transaction and --quick > options by default. > > Could you please create new ticket with the details via > https://trac.edgewall.org/newticket? > > ==== > diff --git a/trac/db/mysql_backend.py b/trac/db/mysql_backend.py > index f607bce30..01cb4fc1d 100644 > --- a/trac/db/mysql_backend.py > +++ b/trac/db/mysql_backend.py > @@ -240,7 +240,8 @@ class MySQLConnector(Component): > db_params = db_prop.setdefault('params', {}) > db_name = os.path.basename(db_prop['path']) > > - args = [self.mysqldump_path, '--no-defaults'] > + defaults_opt = '--no-defaults' > + args = [] > if 'host' in db_prop: > args.extend(['-h', db_prop['host']]) > if 'port' in db_prop: > @@ -252,14 +253,15 @@ class MySQLConnector(Component): > args.append('--compress') > elif name == 'named_pipe' and as_int(value, 0): > args.append('--protocol=pipe') > - elif name == 'read_default_file': # Must be first > - args.insert(1, '--defaults-file=' + value) > + elif name == 'read_default_file': > + defaults_opt = '--defaults-file=' + value > elif name == 'unix_socket': > args.extend(['--protocol=socket', '--socket=' + value]) > elif name not in ('init_command', 'read_default_group'): > self.log.warning("Invalid connection string parameter > '%s'", > name) > - args.extend(['-r', dest_file, db_name]) > + args = [self.mysqldump_path, defaults_opt, > '--single-transaction', > + '--quick'] + args + ['-r', dest_file, db_name] > > environ = os.environ.copy() > if 'password' in db_prop: > ==== > > -- > Jun Omae <[email protected]> (大前 潤) >
Created: https://trac.edgewall.org/ticket/13104 - Ryan -- You received this message because you are subscribed to the Google Groups "Trac Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/trac-users. For more options, visit https://groups.google.com/d/optout.
