Skip to content
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

Is AbstractConnectionFactory need to register itself as an AMQConnection's ShutdownListener when a new connection was created? #2891

Closed
welsper-young opened this issue Nov 8, 2024 · 1 comment

Comments

@welsper-young
Copy link

welsper-young commented Nov 8, 2024

Thus we can receive an AMQConnection shutdown signal.
For CachingConnectionFactory, it does do this thing for AMQChannel creation at line 712:

private Channel doCreateBareChannel(ChannelCachingConnectionProxy conn, boolean transactional) {
Channel channel = conn.createBareChannel(transactional);
if (!ConfirmType.NONE.equals(this.confirmType)) {
try {
channel.confirmSelect();
}
catch (IOException e) {
logger.error("Could not configure the channel to receive publisher confirms", e);
}
}
if ((ConfirmType.CORRELATED.equals(this.confirmType) || this.publisherReturns)
&& !(channel instanceof PublisherCallbackChannelImpl)) {
channel = this.publisherChannelFactory.createChannel(channel, getChannelsExecutor());
}
channel.addShutdownListener(this);
return channel; // NOSONAR - Simple connection throws exception
}

We can do this within below codes maybe?
For example, add rabbitConnection.addShutdownListener(this); under line 584.

protected final Connection createBareConnection() {
try {
String connectionName = this.connectionNameStrategy.obtainNewConnectionName(this);
com.rabbitmq.client.Connection rabbitConnection = connect(connectionName);
Connection connection = new SimpleConnection(rabbitConnection, this.closeTimeout,
this.connectionCreatingBackOff == null ? null : this.connectionCreatingBackOff.start());
if (rabbitConnection instanceof AutorecoveringConnection auto) {
auto.addRecoveryListener(new RecoveryListener() {
@Override
public void handleRecoveryStarted(Recoverable recoverable) {
handleRecovery(recoverable);
}
@Override
public void handleRecovery(Recoverable recoverable) {
try {
connection.close();
}
catch (Exception e) {
AbstractConnectionFactory.this.logger.error("Failed to close auto-recover connection", e);
}
}
});
}
if (this.logger.isInfoEnabled()) {
this.logger.info("Created new connection: " + connectionName + "/" + connection);
}
if (this.recoveryListener != null && rabbitConnection instanceof AutorecoveringConnection auto) {
auto.addRecoveryListener(this.recoveryListener);
}
if (this.applicationEventPublisher != null) {
connection
.addBlockedListener(new ConnectionBlockedListener(connection, this.applicationEventPublisher));
}
return connection;
}
catch (IOException | TimeoutException ex) {
RuntimeException converted = RabbitExceptionTranslator.convertRabbitAccessException(ex);
this.connectionListener.onFailed(ex);
throw converted;
}
}

Currently, I do this at ConnectionListener's onCreate method instead which is not straightforward.

abstractConnectionFactory.addConnectionListener(new ConnectionListener() {
    @Override
    public void onCreate(Connection connection) {
        // Do something.
        connection.getDelegate().addShutdownListener(new ShutdownListener() {
            @Override
            public void shutdownCompleted(ShutdownSignalException cause) {
                // Do something when the AMQConnection shutdown.
            }
        });
    }

    @Override
    public void onClose(Connection connection) {
        // Do something.
    }

    @Override
    public void onShutDown(ShutdownSignalException signal) {
        // Do something.
    }

    @Override
    public void onFailed(Exception exception) {
        // Do something.
    }
 });
@artembilan
Copy link
Member

The channel.addShutdownListener(this); and rabbitConnection.addShutdownListener(this); are two different entities.
We just cannot compare them or replace one with another if I understood your request correctly.

I'm also not sure in your connection.getDelegate() since it feels like something else is missed in your logic.

I agree that there is no easy way to implement your requirement with out-of-the-box AbstractConnectionFactory implementations.

I'll go ahead and add the mentioned rabbitConnection.addShutdownListener(this); into the AbstractConnectionFactory#L580.
Looks like AbstractConnectionFactory.shutdownCompleted() is aware of connection shutdowns as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants