Skip to content

Redis New Implementation#585

Merged
makasim merged 7 commits intomasterfrom
redis
Oct 30, 2018
Merged

Redis New Implementation#585
makasim merged 7 commits intomasterfrom
redis

Conversation

@ASKozienko
Copy link
Member

@ASKozienko ASKozienko commented Oct 26, 2018

  • Upd docs

Supports

  • Redelivery
  • Delaying
  • Expiration

/**
* @return int
*/
public function getRetryDelay(): ?int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redeliveryDelay

* @param Redis $redis
* @param RedisDestination[] $queues
*/
public function __construct(Redis $redis, array $queues)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move array $queues to receiveXXX methods. I think it would be better to make it as stateless as possible

*/
public function getRetryDelay(): int
{
return $this->retryDelay;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we pass it as an argument of receiveXXX methods? S


namespace Enqueue\Redis;

class RedisQueueConsumer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is misleading IMO. Could it be named RedisInternalConsumer or RedisGenericConsumer.


private function initQueueConsumer(): void
{
if (null === $this->queueConsumer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we make a RedisQueueConsumer stateless we could move it to the context and pass to real consumers as a dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The init would not be needed.

{
$this->retryDelay = $retryDelay;

if ($this->queueConsumer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think setting retryDelay property is enough. No need to set same value on queue consumer too. (It will be passed as argument to receiveXXX methods).


$now = time();

if ($expiresAt = $message->getHeader('expires_at')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we managed to processes a message before expiration time, but failed and as a result the message was redelivered, and this condition reject redelivered message as expired. I dont think this is desired behavior.

return $message;
}

private function pushQueueNameBack($queueName): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string $queueName

InvalidMessageException::assertMessageInstanceOf($message, RedisMessage::class);

$this->redis->lpush($destination->getName(), json_encode($message));
$message->setMessageId(UUID::generate());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require ramnsy uuid and use it.

@makasim makasim merged commit 66a1774 into master Oct 30, 2018
@makasim makasim deleted the redis branch October 30, 2018 08:40
ASKozienko pushed a commit that referenced this pull request Nov 2, 2018
@makasim makasim added this to the 0.9 milestone Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants