diff --git a/CHANGELOG.md b/CHANGELOG.md index b06ddcff8b..0296891144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ ___ - NEW: LiveQuery support for $and, $nor, $containedBy, $geoWithin, $geoIntersects queries [#7113](https://github.com/parse-community/parse-server/pull/7113). Thanks to [dplewis](https://github.com/dplewis) - NEW: Supporting patterns in LiveQuery server's config parameter `classNames` [#7131](https://github.com/parse-community/parse-server/pull/7131). Thanks to [Nes-si](https://github.com/Nes-si) - NEW: `requireAnyUserRoles` and `requireAllUserRoles` for Parse Cloud validator. [#7097](https://github.com/parse-community/parse-server/pull/7097). Thanks to [dblythy](https://github.com/dblythy) +- IMPROVE: Retry transactions on MongoDB when it fails due to transient error [#7187](https://github.com/parse-community/parse-server/pull/7187). Thanks to [Antonio Davi Macedo Coelho de Castro](https://github.com/davimacedo). - IMPROVE: Bump tests to use Mongo 4.4.4 [#7184](https://github.com/parse-community/parse-server/pull/7184). Thanks to [Antonio Davi Macedo Coelho de Castro](https://github.com/davimacedo). - IMPROVE: Added new account lockout policy option `accountLockout.unlockOnPasswordReset` to automatically unlock account on password reset. [#7146](https://github.com/parse-community/parse-server/pull/7146). Thanks to [Manuel Trezza](https://github.com/mtrezza). - IMPROVE: Parse Server is from now on continuously tested against all recent MongoDB versions that have not reached their end-of-life support date. Added MongoDB compatibility table to Parse Server docs. [7161](https://github.com/parse-community/parse-server/pull/7161). Thanks to [Manuel Trezza](https://github.com/mtrezza). diff --git a/spec/ParseServerRESTController.spec.js b/spec/ParseServerRESTController.spec.js index fcf8281aff..e2f49c0bbe 100644 --- a/spec/ParseServerRESTController.spec.js +++ b/spec/ParseServerRESTController.spec.js @@ -197,7 +197,7 @@ describe('ParseServerRESTController', () => { .then(() => { spyOn(databaseAdapter, 'createObject').and.callThrough(); - RESTController.request('POST', 'batch', { + return RESTController.request('POST', 'batch', { requests: [ { method: 'POST', @@ -218,11 +218,14 @@ describe('ParseServerRESTController', () => { expect(response[1].success.objectId).toBeDefined(); expect(response[1].success.createdAt).toBeDefined(); const query = new Parse.Query('MyObject'); - query.find().then(results => { - expect(databaseAdapter.createObject.calls.count()).toBe(2); - expect(databaseAdapter.createObject.calls.argsFor(0)[3]).toBe( - databaseAdapter.createObject.calls.argsFor(1)[3] - ); + return query.find().then(results => { + expect(databaseAdapter.createObject.calls.count() % 2).toBe(0); + expect(databaseAdapter.createObject.calls.count() > 0).toEqual(true); + for (let i = 0; i + 1 < databaseAdapter.createObject.calls.length; i = i + 2) { + expect(databaseAdapter.createObject.calls.argsFor(i)[3]).toBe( + databaseAdapter.createObject.calls.argsFor(i + 1)[3] + ); + } expect(results.map(result => result.get('key')).sort()).toEqual([ 'value1', 'value2', @@ -230,7 +233,8 @@ describe('ParseServerRESTController', () => { done(); }); }); - }); + }) + .catch(done.fail); }); it('should not save anything when one operation fails in a transaction', done => { @@ -513,18 +517,18 @@ describe('ParseServerRESTController', () => { const results3 = await query3.find(); expect(results3.map(result => result.get('key')).sort()).toEqual(['value1', 'value2']); - expect(databaseAdapter.createObject.calls.count()).toBe(13); + expect(databaseAdapter.createObject.calls.count() >= 13).toEqual(true); let transactionalSession; let transactionalSession2; let myObjectDBCalls = 0; let myObject2DBCalls = 0; let myObject3DBCalls = 0; - for (let i = 0; i < 13; i++) { + for (let i = 0; i < databaseAdapter.createObject.calls.count(); i++) { const args = databaseAdapter.createObject.calls.argsFor(i); switch (args[0]) { case 'MyObject': myObjectDBCalls++; - if (!transactionalSession) { + if (!transactionalSession || (myObjectDBCalls - 1) % 2 === 0) { transactionalSession = args[3]; } else { expect(transactionalSession).toBe(args[3]); @@ -535,7 +539,7 @@ describe('ParseServerRESTController', () => { break; case 'MyObject2': myObject2DBCalls++; - if (!transactionalSession2) { + if (!transactionalSession2 || (myObject2DBCalls - 1) % 9 === 0) { transactionalSession2 = args[3]; } else { expect(transactionalSession2).toBe(args[3]); @@ -550,9 +554,12 @@ describe('ParseServerRESTController', () => { break; } } - expect(myObjectDBCalls).toEqual(2); - expect(myObject2DBCalls).toEqual(9); - expect(myObject3DBCalls).toEqual(2); + expect(myObjectDBCalls % 2).toEqual(0); + expect(myObjectDBCalls > 0).toEqual(true); + expect(myObject2DBCalls % 9).toEqual(0); + expect(myObject2DBCalls > 0).toEqual(true); + expect(myObject3DBCalls % 2).toEqual(0); + expect(myObject3DBCalls > 0).toEqual(true); }); }); } diff --git a/spec/batch.spec.js b/spec/batch.spec.js index db284d0ce4..07dc955664 100644 --- a/spec/batch.spec.js +++ b/spec/batch.spec.js @@ -228,10 +228,13 @@ describe('batch', () => { expect(response.data[1].success.createdAt).toBeDefined(); const query = new Parse.Query('MyObject'); query.find().then(results => { - expect(databaseAdapter.createObject.calls.count()).toBe(2); - expect(databaseAdapter.createObject.calls.argsFor(0)[3]).toBe( - databaseAdapter.createObject.calls.argsFor(1)[3] - ); + expect(databaseAdapter.createObject.calls.count() % 2).toBe(0); + expect(databaseAdapter.createObject.calls.count() > 0).toEqual(true); + for (let i = 0; i + 1 < databaseAdapter.createObject.calls.length; i = i + 2) { + expect(databaseAdapter.createObject.calls.argsFor(i)[3]).toBe( + databaseAdapter.createObject.calls.argsFor(i + 1)[3] + ); + } expect(results.map(result => result.get('key')).sort()).toEqual([ 'value1', 'value2', @@ -542,18 +545,18 @@ describe('batch', () => { const results3 = await query3.find(); expect(results3.map(result => result.get('key')).sort()).toEqual(['value1', 'value2']); - expect(databaseAdapter.createObject.calls.count()).toBe(13); + expect(databaseAdapter.createObject.calls.count() >= 13).toEqual(true); let transactionalSession; let transactionalSession2; let myObjectDBCalls = 0; let myObject2DBCalls = 0; let myObject3DBCalls = 0; - for (let i = 0; i < 13; i++) { + for (let i = 0; i < databaseAdapter.createObject.calls.count(); i++) { const args = databaseAdapter.createObject.calls.argsFor(i); switch (args[0]) { case 'MyObject': myObjectDBCalls++; - if (!transactionalSession) { + if (!transactionalSession || (myObjectDBCalls - 1) % 2 === 0) { transactionalSession = args[3]; } else { expect(transactionalSession).toBe(args[3]); @@ -564,7 +567,7 @@ describe('batch', () => { break; case 'MyObject2': myObject2DBCalls++; - if (!transactionalSession2) { + if (!transactionalSession2 || (myObject2DBCalls - 1) % 9 === 0) { transactionalSession2 = args[3]; } else { expect(transactionalSession2).toBe(args[3]); @@ -579,9 +582,12 @@ describe('batch', () => { break; } } - expect(myObjectDBCalls).toEqual(2); - expect(myObject2DBCalls).toEqual(9); - expect(myObject3DBCalls).toEqual(2); + expect(myObjectDBCalls % 2).toEqual(0); + expect(myObjectDBCalls > 0).toEqual(true); + expect(myObject2DBCalls % 9).toEqual(0); + expect(myObject2DBCalls > 0).toEqual(true); + expect(myObject3DBCalls % 2).toEqual(0); + expect(myObject3DBCalls > 0).toEqual(true); }); }); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index cac2f6614c..94c2ca4039 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -1046,9 +1046,20 @@ export class MongoStorageAdapter implements StorageAdapter { } commitTransactionalSession(transactionalSection: any): Promise { - return transactionalSection.commitTransaction().then(() => { - transactionalSection.endSession(); - }); + const commit = retries => { + return transactionalSection + .commitTransaction() + .catch(error => { + if (error && error.hasErrorLabel('TransientTransactionError') && retries > 0) { + return commit(retries - 1); + } + throw error; + }) + .then(() => { + transactionalSection.endSession(); + }); + }; + return commit(5); } abortTransactionalSession(transactionalSection: any): Promise { diff --git a/src/ParseServerRESTController.js b/src/ParseServerRESTController.js index c678f99ea3..8293b94a85 100644 --- a/src/ParseServerRESTController.js +++ b/src/ParseServerRESTController.js @@ -48,44 +48,60 @@ function ParseServerRESTController(applicationId, router) { } if (path === '/batch') { - let initialPromise = Promise.resolve(); - if (data.transaction === true) { - initialPromise = config.database.createTransactionalSession(); - } - return initialPromise.then(() => { - const promises = data.requests.map(request => { - return handleRequest(request.method, request.path, request.body, options, config).then( - response => { - if (options.returnStatus) { - const status = response._status; - delete response._status; - return { success: response, _status: status }; + const batch = transactionRetries => { + let initialPromise = Promise.resolve(); + if (data.transaction === true) { + initialPromise = config.database.createTransactionalSession(); + } + return initialPromise.then(() => { + const promises = data.requests.map(request => { + return handleRequest(request.method, request.path, request.body, options, config).then( + response => { + if (options.returnStatus) { + const status = response._status; + delete response._status; + return { success: response, _status: status }; + } + return { success: response }; + }, + error => { + return { + error: { code: error.code, error: error.message }, + }; } - return { success: response }; - }, - error => { - return { - error: { code: error.code, error: error.message }, - }; - } - ); - }); - return Promise.all(promises).then(result => { - if (data.transaction === true) { - if (result.find(resultItem => typeof resultItem.error === 'object')) { - return config.database.abortTransactionalSession().then(() => { - return Promise.reject(result); - }); - } else { - return config.database.commitTransactionalSession().then(() => { + ); + }); + return Promise.all(promises) + .then(result => { + if (data.transaction === true) { + if (result.find(resultItem => typeof resultItem.error === 'object')) { + return config.database.abortTransactionalSession().then(() => { + return Promise.reject(result); + }); + } else { + return config.database.commitTransactionalSession().then(() => { + return result; + }); + } + } else { return result; - }); - } - } else { - return result; - } + } + }) + .catch(error => { + if ( + error && + error.find( + errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251 + ) && + transactionRetries > 0 + ) { + return batch(transactionRetries - 1); + } + throw error; + }); }); - }); + }; + return batch(5); } let query; diff --git a/src/batch.js b/src/batch.js index 607c668a76..58c23ccab6 100644 --- a/src/batch.js +++ b/src/batch.js @@ -83,49 +83,66 @@ function handleBatch(router, req) { req.config.publicServerURL ); - let initialPromise = Promise.resolve(); - if (req.body.transaction === true) { - initialPromise = req.config.database.createTransactionalSession(); - } - - return initialPromise.then(() => { - const promises = req.body.requests.map(restRequest => { - const routablePath = makeRoutablePath(restRequest.path); - - // Construct a request that we can send to a handler - const request = { - body: restRequest.body, - config: req.config, - auth: req.auth, - info: req.info, - }; - - return router.tryRouteRequest(restRequest.method, routablePath, request).then( - response => { - return { success: response.response }; - }, - error => { - return { error: { code: error.code, error: error.message } }; - } - ); - }); + const batch = transactionRetries => { + let initialPromise = Promise.resolve(); + if (req.body.transaction === true) { + initialPromise = req.config.database.createTransactionalSession(); + } - return Promise.all(promises).then(results => { - if (req.body.transaction === true) { - if (results.find(result => typeof result.error === 'object')) { - return req.config.database.abortTransactionalSession().then(() => { - return Promise.reject({ response: results }); - }); - } else { - return req.config.database.commitTransactionalSession().then(() => { + return initialPromise.then(() => { + const promises = req.body.requests.map(restRequest => { + const routablePath = makeRoutablePath(restRequest.path); + + // Construct a request that we can send to a handler + const request = { + body: restRequest.body, + config: req.config, + auth: req.auth, + info: req.info, + }; + + return router.tryRouteRequest(restRequest.method, routablePath, request).then( + response => { + return { success: response.response }; + }, + error => { + return { error: { code: error.code, error: error.message } }; + } + ); + }); + + return Promise.all(promises) + .then(results => { + if (req.body.transaction === true) { + if (results.find(result => typeof result.error === 'object')) { + return req.config.database.abortTransactionalSession().then(() => { + return Promise.reject({ response: results }); + }); + } else { + return req.config.database.commitTransactionalSession().then(() => { + return { response: results }; + }); + } + } else { return { response: results }; - }); - } - } else { - return { response: results }; - } + } + }) + .catch(error => { + if ( + error && + error.response && + error.response.find( + errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251 + ) && + transactionRetries > 0 + ) { + return batch(transactionRetries - 1); + } + throw error; + }); }); - }); + }; + return batch(5); } module.exports = {