RabbitTemplate with SIMPLE correlation causes uncached channels

[bjoernhaeuser]

Hello everyone,

I hope this discussion is at the right place.

We are using a CachingConnectionFactory with CachingConnectionFactory.ConfirmType.SIMPLE as publisher confirm type setting.

Then we are using the RabbitTemplate something like this:

this.amqpTemplate.invoke(operations -> {
    // send messages

    // operations.waitForConfirmsOrDie(10_000);
}   

I thought this reuses channels, but after looking at the RabbitMQ stats I see a relatively high channel churn (in our case 50-100 channels/sec) and digging into this it looks like the RabbitTemplate is creating a new channel for every .invoke() call.

After a debugging session through the code I saw that the RabbitTemplate::invoke method calls RabbitUtils.setPhysicalCloseRequired(channel, true); Reference, when the connectionFactory says that publisher confirms are deactivated.

Right now the connectionFactory.isPublisherConfirms() would only return true when the ConfirmType is set to CORRELATED Reference.

I am not familiar enough to judge if this is the right condition, but it looks to be that it should be changed to say return !ConfirmType.NONE.equals(this.confirmType).

Changing the value to be of confirmType CORRELATED changes the behaviour and channels are properly cached. Can someone please advise if my findings are correct and/or if I am using it simply wrong?

Thank you very much!

[artembilan]

The SIMPLE feature was introduced here: #2357
However its RabbitUtils.setPhysicalCloseRequired(channel, true); was not guarded by the condition.
That one was fixed by this commit: 2e99acd

But unfortunately no any explanation in the commit message.

We can ask @garyrussell for insight, but looks like that condition in the RabbitTemplate must be like this:

				if (!connectionFactory.isPublisherConfirms() && !connectionFactory.isSimplePublisherConfirms()) {
					RabbitUtils.setPhysicalCloseRequired(channel, true);
				}

If we are on the same page, then I don't see why this discussion cannot be converted to issue with respective fix and so.

Thanks

[garyrussell]

Agreed.

[bjoernhaeuser]

First of all thank you very much for the quick response! I am not sure what are the next steps now, would you folks take it from here? Would you expect a PR? (I am not sure what the expected changes are though).

Thank you! 🙇

[artembilan]

Created respective issue: #2572.

But our plans is to look into that in the next major/minor version.
Probably already after holidays.

Feel free to raise a PR though!
Not promising a quick review either way 😉

[bjoernhaeuser]

Hi there!

Just wanted to revive this a bit and was looking through the discussion again. I would like to offer a PR, though I am not sure what's the result of the discussion how this should be handled.

Should this behaviour be removed from the invoke method or should be handled differently?

Thank you! 🙇

[artembilan]

We don't have plans for major/minor version yet, so the fix (if any) is not going to be accepted yet.
You are feel free to propose some PR and we can go from there with the planning and so on.

Thanks