Skip to content

Commit 003aabc

Browse files
schmodsushantdhiman
authored andcommitted
fix(mysql/mariadb): treat deadlocked transactions as rollback (#11074)
1 parent 4e0ce38 commit 003aabc

File tree

6 files changed

+90
-12
lines changed

6 files changed

+90
-12
lines changed

lib/dialects/mariadb/query.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ class Query extends AbstractQuery {
3434

3535
run(sql, parameters) {
3636
this.sql = sql;
37-
const { connection } = this;
37+
const { connection, options } = this;
3838

3939
const showWarnings = this.sequelize.options.showWarnings
40-
|| this.options.showWarnings;
40+
|| options.showWarnings;
4141

4242
const complete = this._logQuery(sql, debug);
4343

@@ -56,6 +56,11 @@ class Query extends AbstractQuery {
5656
return results;
5757
})
5858
.catch(err => {
59+
// MariaDB automatically rolls-back transactions in the event of a deadlock
60+
if (options.transaction && err.errno === 1213) {
61+
options.transaction.finished = 'rollback';
62+
}
63+
5964
complete();
6065

6166
err.sql = sql;

lib/dialects/mysql/query.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ class Query extends AbstractQuery {
2929

3030
run(sql, parameters) {
3131
this.sql = sql;
32-
const { connection } = this;
32+
const { connection, options } = this;
3333

3434
//do we need benchmark for this query execution
35-
const showWarnings = this.sequelize.options.showWarnings || this.options.showWarnings;
35+
const showWarnings = this.sequelize.options.showWarnings || options.showWarnings;
3636

3737
const complete = this._logQuery(sql, debug);
3838

@@ -41,6 +41,10 @@ class Query extends AbstractQuery {
4141
complete();
4242

4343
if (err) {
44+
// MySQL automatically rolls-back transactions in the event of a deadlock
45+
if (options.transaction && err.errno === 1213) {
46+
options.transaction.finished = 'rollback';
47+
}
4448
err.sql = sql;
4549

4650
reject(this.formatError(err));

lib/query-interface.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,7 +1417,8 @@ class QueryInterface {
14171417

14181418
options = Object.assign({}, options, {
14191419
transaction: transaction.parent || transaction,
1420-
supportsSearchPath: false
1420+
supportsSearchPath: false,
1421+
completesTransaction: true
14211422
});
14221423

14231424
const sql = this.QueryGenerator.commitTransactionQuery(transaction);
@@ -1435,7 +1436,8 @@ class QueryInterface {
14351436

14361437
options = Object.assign({}, options, {
14371438
transaction: transaction.parent || transaction,
1438-
supportsSearchPath: false
1439+
supportsSearchPath: false,
1440+
completesTransaction: true
14391441
});
14401442
options.transaction.name = transaction.parent ? transaction.name : undefined;
14411443
const sql = this.QueryGenerator.rollbackTransactionQuery(transaction);

lib/sequelize.js

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -614,24 +614,30 @@ class Sequelize {
614614
[sql, bindParameters] = this.dialect.Query.formatBindParameters(sql, options.bind, this.options.dialect);
615615
}
616616

617+
const checkTransaction = () => {
618+
if (options.transaction && options.transaction.finished && !options.completesTransaction) {
619+
const error = new Error(`${options.transaction.finished} has been called on this transaction(${options.transaction.id}), you can no longer use it. (The rejected query is attached as the 'sql' property of this error)`);
620+
error.sql = sql;
621+
throw error;
622+
}
623+
};
624+
617625
const retryOptions = Object.assign({}, this.options.retry, options.retry || {});
618626

619627
return Promise.resolve(retry(() => Promise.try(() => {
620628
if (options.transaction === undefined && Sequelize._cls) {
621629
options.transaction = Sequelize._cls.get('transaction');
622630
}
623-
if (options.transaction && options.transaction.finished) {
624-
const error = new Error(`${options.transaction.finished} has been called on this transaction(${options.transaction.id}), you can no longer use it. (The rejected query is attached as the 'sql' property of this error)`);
625-
error.sql = sql;
626-
throw error;
627-
}
631+
632+
checkTransaction();
628633

629634
return options.transaction
630635
? options.transaction.connection
631636
: this.connectionManager.getConnection(options);
632637
}).then(connection => {
633638
const query = new this.dialect.Query(connection, this, options);
634639
return this.runHooks('beforeQuery', options, query)
640+
.then(() => checkTransaction())
635641
.then(() => query.run(sql, bindParameters))
636642
.finally(() => this.runHooks('afterQuery', options, query))
637643
.finally(() => {

test/integration/dialects/mysql/errors.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ if (dialect === 'mysql') {
7070
reltype: 'child'
7171
}));
7272
});
73-
7473
});
7574
});
7675
}

test/integration/transaction.test.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,68 @@ if (current.dialect.supports.transactions) {
366366
});
367367
});
368368

369+
if (dialect === 'mysql' || dialect === 'mariadb') {
370+
describe('deadlock handling', () => {
371+
it('should treat deadlocked transaction as rollback', function() {
372+
const Task = this.sequelize.define('task', {
373+
id: {
374+
type: Sequelize.INTEGER,
375+
primaryKey: true
376+
}
377+
});
378+
379+
// This gets called twice simultaneously, and we expect at least one of the calls to encounter a
380+
// deadlock (which effectively rolls back the active transaction).
381+
// We only expect createTask() to insert rows if a transaction is active. If deadlocks are handled
382+
// properly, it should only execute a query if we're actually inside a real transaction. If it does
383+
// execute a query, we expect the newly-created rows to be destroyed when we forcibly rollback by
384+
// throwing an error.
385+
// tl;dr; This test is designed to ensure that this function never inserts and commits a new row.
386+
const update = (from, to) => this.sequelize.transaction(transaction => {
387+
return Task.findAll({
388+
where: {
389+
id: {
390+
[Sequelize.Op.eq]: from
391+
}
392+
},
393+
lock: 'UPDATE',
394+
transaction
395+
})
396+
.then(() => Promise.delay(10))
397+
.then(() => {
398+
return Task.update({ id: to }, {
399+
where: {
400+
id: {
401+
[Sequelize.Op.ne]: to
402+
}
403+
},
404+
lock: transaction.LOCK.UPDATE,
405+
transaction
406+
});
407+
})
408+
.catch(e => { console.log(e.message); })
409+
.then(() => Task.create({ id: 2 }, { transaction }))
410+
.catch(e => { console.log(e.message); })
411+
.then(() => { throw new Error('Rollback!'); });
412+
}).catch(() => {});
413+
414+
return this.sequelize.sync({ force: true })
415+
.then(() => Task.create({ id: 0 }))
416+
.then(() => Task.create({ id: 1 }))
417+
.then(() => Promise.all([
418+
update(1, 0),
419+
update(0, 1)
420+
]))
421+
.then(() => {
422+
return Task.count().then(count => {
423+
// If we were actually inside a transaction when we called `Task.create({ id: 2 })`, no new rows should be added.
424+
expect(count).to.equal(2, 'transactions were fully rolled-back, and no new rows were added');
425+
});
426+
});
427+
});
428+
});
429+
}
430+
369431
if (dialect === 'sqlite') {
370432
it('provides persistent transactions', () => {
371433
const sequelize = new Support.Sequelize('database', 'username', 'password', { dialect: 'sqlite' }),

0 commit comments

Comments
 (0)