retupmoca / P6-Net-AMQP

MIT License
6 stars 4 forks source link

When cancel method will be available? #21

Closed bbkr closed 5 years ago

bbkr commented 5 years ago

Hi

Do you have any timeline when Queue::cancel method will be available?

jonathanstowe commented 5 years ago

Well, Not all that difficult to implement really, I'll take a look in the next day or so, it's just no-one wanted it until now :)

bbkr commented 5 years ago

Awesome, thanks!

bbkr commented 5 years ago

After looking into code i think current interface design will make cancel a bit difficult / tricky. There is no direct connection between consumer tag (required for cancel) and Supply used to receive messages. So - there is no easy way to "un-tap" the $q.message-supply Supply.

Example:

$q.message-supply.tap( { ... } );
$q.consume( consumer-tag => "foo" );
$q.consume( consumer-tag => "bar" );

After cancelling both tags there will be no easy way to gracefully finish Supply because there is no easy access to Supplier to call done() on it. And that will leave supply in memory. Am I right?

Maybe message supply tied with specific consumer tag should be returned from consume promise:

$q.consume( consumer-tag => "foo" ).result.tap( {...} );
$q.consume( consumer-tag => "bar" ).result.tap( {...} );
$q.cancel( consumer-tag => "foo" ); # sends done() only to one Supply

?

bbkr commented 5 years ago

Ah, Consumer tag is queue attribute. So there is no way to consume the same queue twice from single Queue object instance. Still - question about Supply un-taping stays valid.

jonathanstowe commented 5 years ago

Well, I think that it would be make the Supplier behind the supply-message a private attribute rather than simply closing over a lexical variable as it does now, and the call done on it when the cancel-ok message is received by the client. This would also make it easier to handle the RabbitMQ extension whereby the broker can send a cancel .

I also have a feeling that there should be a cancellation promise that gets kept when either a cancel-ok or broker cancel is received so other parts of an application can react to the consumer being cancelled..

jonathanstowe commented 5 years ago

I'm going to use this excuse to make some various improvements to the code as well as this change so I'll probably be done later in the week.

bbkr commented 5 years ago

thanks!