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

Changed ClientProperty after recovery in connection.created message #861

Open
mheidt opened this issue Oct 11, 2022 · 6 comments
Open

Changed ClientProperty after recovery in connection.created message #861

mheidt opened this issue Oct 11, 2022 · 6 comments

Comments

@mheidt
Copy link

mheidt commented Oct 11, 2022

Hello,

We are using this version
com.rabbitmq
amqp-client
5.15.0

But 3.16.0 hasn't changed in the following regards.

We are facing the issue that we need a unique identifier that should change after each connection incl. automatic recovery.
That's why we added this UUID into the clientProperties of the connection via ConnectionFactory.
Everything works but the initial connection.created message the rabbit server is sending after a recovery.
Here we see the old UUID still.
We tried to change the property in the recoveryStarted call.

But when I look into the source code, we don't have any chance to do this.

AutorecoveringConnection stores params, which are not accessible from outside but in parts through a delegate that is created as RecoveryAwareAMQConnection using this params.
Unfortunately they are stored as a new HashMap in AMQConnection:
this._clientProperties = new HashMap(params.getClientProperties());
As well as whole as params in RecoveryAwareAMQConnectionFactory.

The AutorecoveringConnection.recoverConnection is calling the internal ConnectionFactory (not ours) to create a newConnection which is using the stored params, which is not accessible for us.

As I previously said, the first call of recoveryStart is too late for us to make any modification, because we need this changed clientProperty in the first communication (connection.created message) that is made before any channel activity.

Or is there a way to do this anyhow?
In my small opinion the clientProperties of the delegate of the current connection should be used for the new connection instead of the initially stored params of the factory.

Kind regards,
Markus Heidt

I know, I hate those cases as well :)

@michaelklishin
Copy link
Member

If the intent is to observe a reconnection event using a property of the connection object we can introduce an "epoch" that would be incremented with every recovery and that's it. No need to mess with client properties which are not meant to be a transient store applications modify.

@mheidt
Copy link
Author

mheidt commented Oct 12, 2022

Yes, to sum it up in my words: It changes for each connection/recovery and the EPOCH is accessible in the RecoveryListener methods, in the client calls and in the connection.read message as well.
That would be nice, if you could provide that.

@acogoluegnes
Copy link
Contributor

These suggestions would require some API changes. Connection has already an ID with the accessors. It's used by the MetricsCollector in newConnection. It would be worth trying to hack something, the newConnection method is called just after the connection initialization (open AMQP method). Maybe with a custom MetricsCollector that decorates calls to a (real) delegate, this way you can intercept newConnection.

@mheidt
Copy link
Author

mheidt commented Oct 14, 2022

@michaelklishin
Is the MetricsCollector-Delegate hack an option or are you planning to provide the EPOCH?

@michaelklishin
Copy link
Member

@mheidt this is open source software. You can contribute what you need. I would not be a favor of any hacks if all we need is a counter atomically incremented between connection recoveries.

@acogoluegnes
Copy link
Contributor

The MetricsCollector solution is a form of hack, or at least it means using an API not only in the way it is intended to be used. The epoch is a good and simple solution, but it implies giving access to the epoch somehow. The most obvious place is in the Connection interface, which means breaking the contract unless we provide a default implementation.

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

No branches or pull requests

3 participants