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]> (大前 潤)

-- 
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.

Reply via email to