search

JackAdams / meteor-transactions

App level transactions for Meteor + Mongo
http://transactions.taonova.com/
MIT License
113 stars 16 forks source link

Force Transaction failure #43

Closed dheerajbhaskar closed 8 years ago

dheerajbhaskar commented 8 years ago

Is there any way to force transaction failure so as check if the rollback steps are appropriate?

To be more specific:

tx.start("delete post");
Posts.remove({_id: post_id}, {tx: true});
Comments.find({post_id: post_id}).forEach(function (comment) {
  Comments.remove({_id: comment._id}, {tx: true});
});
tx.forceFail(); // <<<<<<<<< This or something like that is the request I am making
// more statements...
tx.commit();
JackAdams commented 8 years ago

tx.rollback() should work for that purpose.

dheerajbhaskar commented 8 years ago

Hey could I trouble you to link to a sample usage?

On Sat 7 Nov, 2015 5:16 pm Brent Abrahams notifications@github.com wrote:

tx.rollback() should work for that purpose.

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154694548 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

I mean, in the code sample above, replace tx.forceFail(); with tx.rollback().

dheerajbhaskar commented 8 years ago

Thanks for the quick reply, Brent. Will try this out.

On Sat 7 Nov, 2015 5:46 pm Brent Abrahams notifications@github.com wrote:

I mean, in the code sample above, replace tx.forceFail(); with tx.rollback().

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154695651 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

Actually, I think tx.cancel() would work as a better substitute for tx.forceFail(). The difference between tx.cancel() and tx.rollback() is that the cancel allows all the subsequent operations to complete and guarantees a rollback at the end. tx.rollback() rolls back immediately, so any further operations (after the rollback) would still be attempted. (I think -- just had a quick glance over the code and it seems that that's what would happen.)

dheerajbhaskar commented 8 years ago

If a transaction fails at some point (without these "force fail" like statements) what does the package do? Does it use rollback or cancel. I just want to check that the rollbacks happen properly and the behavior is as expected.

On Sat 7 Nov, 2015 5:58 pm Brent Abrahams notifications@github.com wrote:

Actually, I think tx.cancel() would work as a better substitute for tx.forceFail(). The difference between tx.cancel() and tx.rollback() is that the cancel allows all the subsequent operations to complete and guarantees a rollback at the end. tx.rollback() rolls back immediately, so any further operations (after the rollback) would still be attempted. (I think -- just had a quick glance over the code and it seems that that's what would happen.)

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154696435 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

It does the cancel behaviour if there is a problem halfway through the set of actions; tx.rollback() is called, at the end, after all the actions in the transaction have been queued (so the queue is not actually processed - i.e. db writes are not made).

There is currently a bug in the actual implementation of the rollback if an error occurs half way through processing a queue of actions (i.e. making the actual writes to the db), but that will be fixed asap. There are open issues regarding this problem and it is on the "fairly-urgent" list, as the package is kind of missing the whole point until that particular part works as expected.

So, long story short, there's a serious bug that will be fixed soon. I'm working on it tonight, but I don't know when I'll actually get all the tests passing and the next release out.

I've been holding off an announcement of this package on crater.io and forums.meteor.com because I've never felt it's quite production ready (even though I've been using it successfully in a big, complex production app for a couple of years).

dheerajbhaskar commented 8 years ago

Given the number of stars (ignoring the version number), we've also integrated this into a production app. It would be great if you could release a fix and let us know.

Thanks for taking the time to explain the behavior.

Would it be right to manually call tx.cancel as production behavior(when the transaction didn't go as expected) ?

Would it be okay to call a return in a function right after tx.cancel? (we're using this in an api endpoint via restivus package)

On Sat 7 Nov, 2015 6:24 pm Brent Abrahams notifications@github.com wrote:

It does the cancel behaviour if there is a problem halfway through the set of actions; tx.rollback() is called, at the end, after all the actions in the transaction have been queued (so the queue is not actually processed - i.e. db writes are not made).

There is currently a bug in the actual implementation of the rollback if an error occurs half way through processing a queue of actions (i.e. making the actual writes to the db), but that will be fixed asap. There are open issues regarding this problem and this is on the "fairly-urgent" list, as the package is kind of missing the whole point until it is working as expected.

So, long story short, there's a serious bug that will be fixed soon. I'm working on it tonight, but I don't know when I'll actually get all the tests passing and the next release out.

I've been holding off an announcement of this package on crater.io and forums.meteor.com because I've never felt it's quite production ready (even though I've been using it successfully in a big, complex production app for a couple of years).

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154698573 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

I occasionally use tx.cancel() in my production app, when I discover something halfway through the transaction that means it's not going to work out properly after all (usually when performing checks while iterating through a loop).

On the server, if the transaction isn't explicitly committed, it will time out and self commit after 5 seconds (by default) of being idle. If you want to return immediately, I'd suggest:

tx.cancel();
tx.commit();
return;
dheerajbhaskar commented 8 years ago

That seems like a good idea tx.cancel(); tx.commit(); return;

Since I couldn't see the failed transactions in the collection, I've just replaced this with precondition logic so that transaction is not even initiated.

Are you using 0.6 in prod or latest release? The reason I ask is that 0.6, given that it's more battle tested, it might be suitable for our prod deployment as well.

On Mon 9 Nov, 2015 4:53 am Brent Abrahams notifications@github.com wrote:

I occasionally use tx.cancel() in my production app, when I discover something halfway through the transaction that means it's not going to work out properly after all (usually when performing checks while iterating through a loop).

On the server, if the transaction isn't explicitly committed, it will time out and self commit after 5 seconds (by default) of being idle. If you want to return immediately, I'd suggest:

tx.cancel(); tx.commit(); return;

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154886142 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

I'm using 0.7.x in production. 0.6.x had no recoverability - everything had to go as expected or there would be data corruption. At least 0.7.x holds out a hope of recovery and, more importantly, it has a growing automated test suite, which is absolutely essential for a package of this nature. 0.6.x had no tests except by constant usage. 0.7.x has been in production long enough now that I'm pretty confident that it performs its base functions well. I'll be rolling out more tests and fixing the rollbacks within the week.

Sent from my iPhone

On Nov 9, 2015, at 6:38 PM, Dheeraj Bhaskar notifications@github.com wrote:

That seems like a good idea tx.cancel(); tx.commit(); return;

Since I couldn't see the failed transactions in the collection, I've just replaced this with precondition logic so that transaction is not even initiated.

Are you using 0.6 in prod or latest release? The reason I ask is that 0.6, given that it's more battle tested, it might be suitable for our prod deployment as well.

On Mon 9 Nov, 2015 4:53 am Brent Abrahams notifications@github.com wrote:

I occasionally use tx.cancel() in my production app, when I discover something halfway through the transaction that means it's not going to work out properly after all (usually when performing checks while iterating through a loop).

On the server, if the transaction isn't explicitly committed, it will time out and self commit after 5 seconds (by default) of being idle. If you want to return immediately, I'd suggest:

tx.cancel(); tx.commit(); return;

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154886142 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.--- — Reply to this email directly or view it on GitHub.

dheerajbhaskar commented 8 years ago

Oh okay, sounds good.

On Mon 9 Nov, 2015 4:15 pm Brent Abrahams notifications@github.com wrote:

I'm using 0.7.x in production. 0.6.x had no recoverability - everything had to go as expected or there would be data corruption. At least 0.7.x holds out a hope of recovery and, more importantly, it has a growing automated test suite, which is absolutely essential for a package of this nature. 0.6.x had no tests except by constant usage. 0.7.x has been in production long enough now that I'm pretty confident that it performs its base functions well. I'll be rolling out more tests and fixing the rollbacks within the week.

Sent from my iPhone

On Nov 9, 2015, at 6:38 PM, Dheeraj Bhaskar notifications@github.com wrote:

That seems like a good idea tx.cancel(); tx.commit(); return;

Since I couldn't see the failed transactions in the collection, I've just replaced this with precondition logic so that transaction is not even initiated.

Are you using 0.6 in prod or latest release? The reason I ask is that 0.6, given that it's more battle tested, it might be suitable for our prod deployment as well.

On Mon 9 Nov, 2015 4:53 am Brent Abrahams notifications@github.com wrote:

I occasionally use tx.cancel() in my production app, when I discover something halfway through the transaction that means it's not going to work out properly after all (usually when performing checks while iterating through a loop).

On the server, if the transaction isn't explicitly committed, it will time out and self commit after 5 seconds (by default) of being idle. If you want to return immediately, I'd suggest:

tx.cancel(); tx.commit(); return;

— Reply to this email directly or view it on GitHub < https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-154886142

.

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.--- — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-155026055 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

This is just about done. I need to write a couple more tests and I should be able to release on atmosphere. The version on Github should be pretty much okay right now.

dheerajbhaskar commented 8 years ago

I shall wait for the atmosphere release rather than vendoring this dependency. Thanks Brent for keeping at it.

On Sun, Nov 15, 2015 at 10:51 PM Brent Abrahams notifications@github.com wrote:

This is just about done. I need to write a couple more tests and I should be able to release on atmosphere. The version on Github should be pretty much okay right now.

— Reply to this email directly or view it on GitHub https://github.com/JackAdams/meteor-transactions/issues/43#issuecomment-156832329 .

--- Disclaimer --- The information in this mail is confidential and is intended solely for addressee. Access to this mail by anyone else is unauthorised. Copying or further distribution beyond the original recipient may be unlawful. Any opinion expressed in this mail is that of sender and does not necessarily reflect that of OSW Technologies Pvt Ltd.---

JackAdams commented 8 years ago

Just released 0.7.5 on Atmosphere.