trac-admin hotcopy failing with mysql backend on 1.2.3

33 views
Skip to first unread message

Dan

unread,
Oct 28, 2018, 6:09:31 AM10/28/18
to Trac Users
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?

Peter Suter

unread,
Oct 28, 2018, 7:33:02 AM10/28/18
to trac-...@googlegroups.com
On 28.10.2018 02:12, Dan wrote:
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].
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.
It seems --no-defaults is relatively new in Trac: https://trac.edgewall.org/ticket/12880#comment:4
The interaction with read_default_file / --defaults-file was maybe not considered / encountered yet.

Jun Omae

unread,
Oct 29, 2018, 8:48:46 AM10/29/18
to trac-...@googlegroups.com
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 <jun...@gmail.com> (大前 潤)

RjOllos

unread,
Nov 15, 2018, 12:12:16 AM11/15/18
to Trac Users
Reply all
Reply to author
Forward
0 new messages