Skip to content

[Messenger (Redis transport)] Connection should use custom Redis instance #45057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
OO00O0O opened this issue Jan 18, 2022 · 2 comments
Closed

Comments

@OO00O0O
Copy link

OO00O0O commented Jan 18, 2022

Description

If I provide custom Redis instance to Symfony\Component\Messenger\Bridge\Redis\Transport\Connection and it's already connected, Redis goes "Server went away". Thats because(probably) its already connected.

Problem is in symfony/redis-messenger/Transport/Connection.php:76. There should be another elseif with elseif ($redis && $redis->isConnected()) { $this->connection = $redis; }

Example

class MessengerTransportFactory extends RedisTransportFactory
{
    public function createTransport(string $dsn, array $options, SerializerInterface $serializer): TransportInterface
    {
         return new RedisTransport(
            new Connection([], [], [], $CUSTOM_REDIS_INSTANCE),
            $serializer
        );
    }
}
@chalasr
Copy link
Member

chalasr commented Jan 19, 2022

Sounds like a bug. PR with test case welcome. Please target the lowest branch where it applies.

@nicolas-grekas
Copy link
Member

PR welcome indeed @OO00O0O. Closing meanwhile.

fabpot added a commit that referenced this issue Dec 8, 2023
…BusterNeece)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Avoid reconnecting active Redis connections.

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #45057
| License       | MIT

With the Messenger component's internal Redis Connection class, if you pass it a \Redis or \Relay instance that's already connected, it will still try to reconnect that instance using the host/port/etc. that's passed to the connection object's "options" array in the constructor.

Not only does this cause problems when the Redis instance is already connected, but it also prevents you from, say, using socket connections instead (as the only connection method supported by the class itself is IP-based connections) and just passing in a ready-to-go Redis instance.

This PR is a resubmission of #52811 targeting 5.4 as it's a bug fix.

Commits
-------

650f153 bug #45057 [Messenger] Avoid reconnecting active Redis connections.
xabbuh added a commit that referenced this issue Dec 10, 2023
* 5.4:
  [String] Skip a test when an issue is detected in PCRE2
  [Mailer] Stop using the (local) AWS shared configuration in the PHPUnit tests.
  detect colors on not windows
  fix xterm detection
  refactor: hyper check
  Missing translations for Slovak (sk) #51954
  properly handle SYMFONY_DOTENV_VARS being the empty string
  Avoid incompatibility with symfony/console 7
  bug #45057 [Messenger] Avoid reconnecting active Redis connections.
  [Serializer] fix regression where nullable int cannot be serialized
  do not overwrite an application's default serialization context
xabbuh added a commit that referenced this issue Dec 10, 2023
* 6.3:
  fix merge
  [VarDumper] Test intl formatter broken since dumper does not replace the nnbsp character by standard space
  [WebProfilerBundle] Fix intercept external redirects
  [Webhook] Added missing XML attribute in config XSD
  [String] Skip a test when an issue is detected in PCRE2
  [ExpressionLanguage] Fix null coalescing propagation
  [Mailer] Stop using the (local) AWS shared configuration in the PHPUnit tests.
  detect colors on not windows
  fix xterm detection
  refactor: hyper check
  Missing translations for Slovak (sk) #51954
  Remove redundant PHPdoc line
  properly handle SYMFONY_DOTENV_VARS being the empty string
  Avoid incompatibility with symfony/console 7
  bug #45057 [Messenger] Avoid reconnecting active Redis connections.
  [HttpKernel] Catch `TypeError` if the wrong type is used in `BackedEnumValueResolver`
  [Serializer] fix regression where nullable int cannot be serialized
  do not overwrite an application's default serialization context
xabbuh added a commit that referenced this issue Dec 10, 2023
* 6.4:
  fix merge
  [VarDumper] Test intl formatter broken since dumper does not replace the nnbsp character by standard space
  [WebProfilerBundle] Fix intercept external redirects
  [Webhook] Added missing XML attribute in config XSD
  [String] Skip a test when an issue is detected in PCRE2
  [ExpressionLanguage] Fix null coalescing propagation
  [Mailer] Stop using the (local) AWS shared configuration in the PHPUnit tests.
  detect colors on not windows
  fix xterm detection
  refactor: hyper check
  Missing translations for Slovak (sk) #51954
  Remove redundant PHPdoc line
  properly handle SYMFONY_DOTENV_VARS being the empty string
  Avoid incompatibility with symfony/console 7
  bug #45057 [Messenger] Avoid reconnecting active Redis connections.
  [HttpKernel] Catch `TypeError` if the wrong type is used in `BackedEnumValueResolver`
  [Serializer] fix regression where nullable int cannot be serialized
  do not overwrite an application's default serialization context
xabbuh added a commit that referenced this issue Dec 10, 2023
* 7.0:
  fix merge
  [VarDumper] Test intl formatter broken since dumper does not replace the nnbsp character by standard space
  [WebProfilerBundle] Fix intercept external redirects
  [Webhook] Added missing XML attribute in config XSD
  [String] Skip a test when an issue is detected in PCRE2
  [ExpressionLanguage] Fix null coalescing propagation
  [Mailer] Stop using the (local) AWS shared configuration in the PHPUnit tests.
  detect colors on not windows
  fix xterm detection
  refactor: hyper check
  Missing translations for Slovak (sk) #51954
  Remove redundant PHPdoc line
  properly handle SYMFONY_DOTENV_VARS being the empty string
  Avoid incompatibility with symfony/console 7
  bug #45057 [Messenger] Avoid reconnecting active Redis connections.
  [HttpKernel] Catch `TypeError` if the wrong type is used in `BackedEnumValueResolver`
  [Serializer] fix regression where nullable int cannot be serialized
  do not overwrite an application's default serialization context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants