amqp-node / amqplib

AMQP 0-9-1 library and client for Node.JS
https://amqp-node.github.io/amqplib/
Other
3.69k stars 475 forks source link

Supporting transactions #76

Open fuson opened 10 years ago

fuson commented 10 years ago

It's a very usefull feature for production processing :)

squaremo commented 10 years ago

Oh, I thought everyone had given up on transactions in AMQP, since they have none of the properties that "transactions" are usually considered to have. What are you using them for?

fuson commented 10 years ago

We use it in case when we must have guarantee what if message finally dequeued then it processed, cause in case of server process crashe or restart we cant lost dequeued but not finally processed massages. Only transactions may helps. :(

squaremo commented 10 years ago

Perhaps I have misunderstood, but acknowledgements would be adequate for that, wouldn't they?

  1. Dequeue message
  2. Process message
  3. Acknowledge message

If the server or the client crashes before 3., the message will get redelivered. Transactions don't make any difference.

squaremo commented 10 years ago

It wouldn't be hard to add "transaction" channels to the client API. But I wouldn't want to encourage people to use them, since they don't do what the name suggests.

Did you figure out if you really need them?

cookie-bytes commented 9 years ago

:-1:

manchicken commented 8 years ago

I've seen transactions used quite well to manage dead-letter queues which have manual processing requirements. Either way, it's part of AMQP 0.9.1, should one really limit what's in the API just because they can't think of how they would use it, or should the implement as true to the spec as reasonable?

squaremo commented 8 years ago

Either way, it's part of AMQP 0.9.1, should one really limit what's in the API just because they can't think of how they would use it,

I appreciate this point, and there's an element of that. I was more concerned that it would invite people to rely on transactions in RabbitMQ, which don't deserve the name, and thereby cause far more problems than it solves.

But I'm not dead set against them. With careful use they can make some failures a bit more recoverable.

manchicken commented 8 years ago

I appreciate this point, and there's an element of that. I was more concerned that it would invite people to rely on transactions in RabbitMQ, which don't deserve the name, and thereby cause far more problems than it solves.

But I'm not dead set against them. With careful use they can make some failures a bit more recoverable.

As someone who maintains an AMQP library (Net::AMQP::RabbitMQ on CPAN), I appreciate your concern. There are some wrong ways to use the protocol, but I don't really think that the library should intentionally have gaps to prevent people from engaging in poor practices.

Transactions are in the spec because they have value, they're useful. I argue that we should have access to them in the library since they're in the spec, and that those who do silly things with the library are going to find ways to break things with or without the help of this library (you know this to be true).

crussell52 commented 6 years ago

@squaremo Any 2018 thoughts on this? The AMQP libs that I've encountered in other languages all support the transaction semantic. In some of your comments on this thread, I see a declaration of relative simplicity and an acknowledgement of value:

"It wouldn't be hard to add "transaction" channels to the client API..."

"But I'm not dead set against them. With careful use they can make some failures a bit more recoverable."

If nothing else, implementing would fall under the "principle of least surprise". It is unfortunate to build an app based on such a good library only and be caught off-guard later when you find out that a feature defined by the core spec is not supported. Anybody coming from another language where TX is supported may be similarly surprised. 😞

manchicken commented 6 years ago

My thoughts are the same as they were before: it's part of the spec, I still think it makes sense to add this functionality.

cyrilchapon commented 5 years ago

In here we're building an API that receives queries from a frontend.

We have workers, and want to publish from our API to RMQ to handle certain queries asynchroneously.

The main trouble we encounter is that we want to keep the API "stateful" in order to show some spinners or stuff on the frontend while the message is being processed.

That "state" cannot be handled inside RMQ itself and requires a database for this. We have a Postgres database and we'd like to do, safely, something like :

sqlTransaction(t => {
  await sql.create(whatEverState, { transaction: t })
  await rmq.publish(theMessage)
})

The above just works actually, but only handles half that we're trying to handle :

If now I do :

sqlTransaction(t => {
  await rmq.publish(theMessage)
  await sql.create(whatEverState, { transaction: t })
})

The closest pattern would be :

sqlTransaction(sqlT => {
  rmqTransaction(rmqT => {
    await sql.create(whatEverState, { transaction: sqlT })
    await rmq.publish(theMessage, { someKindOfTransaction: rmqT })
  })
})

Thus, in that case, we would need RMQ transactions :)

cressie176 commented 5 years ago

@cyrilchapon that wouldn't help. Your application could crash / restart between committing the rmqTransaction and committing the sqlTransaction. What you are looking for are distributed transactions or 2 phase commit, but I don't think that RabbitMQ supports it.

2 phase commit gives you "exactly once" semantics, but there are many situations where this is unachievable (imagine if you were sending an email instead of publishing a message to a broker). Instead I try to work out which of "at most once" or "at least once" semantics is preferable if something does go wrong, then take reasonable steps to minimise the impact and facility recovery.

Since you're using PostgreSQL an alternative way of managing background work is a combination of SELECT FOR UPDATE, SKIP LOCKED and LIMIT, e.g. the following query would retrieve the next available row from the 'todo' table.

SELECT * FROM todo FOR UPDATE SKIP LOCKED
WHERE completed_on IS NULL
ORDER BY received_on ASC
LIMIT 1

There's obviously a little more to it than this, but it might mean you can use a single transaction after all.

cyrilchapon commented 5 years ago

@cressie176 thanks for your help, great pièces of information, and references.

Your application could crash / restart between committing the rmqTransaction and committing the sqlTransaction.

That's a risk I'm aware of, and that I'm reasonably going to accept for now.

imagine if you were sending an email instead of publishing a message to a broker

That's something I'm going to prohibit in this stack. Actualy my API is just one slave among others that publish work on the broker. I have some (stateless) webhook receivers that also do so. If I had to send an email, I'd rather publish that to the exchange. I'm kind of writing directly to RMQ from my API to avoid creating and calling some webhook mecanism from here too (which would have the same atomicity / resiliency / commit requirements, but would be even more dangerous through HTTP or would require some retry mecanisms backed by some... message queue... so...)

Anyway that's not the exact place to talk about this but I'd be glad to get more design feedback anywhere else if you're up to 🙂

Transactions inside RMQ, in my special case, would add a little more flexibility and safety (to a non-perfect strategy, I confess)