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

ARTEMIS-1647 - Add plugin support for binding creation and removal #1829

Merged
merged 1 commit into from Feb 1, 2018

Conversation

cshannon
Copy link
Contributor

Adds callbacks to ActiveMQServerPlugin for binding add and remove events

@@ -571,6 +571,8 @@ public AddressInfo getAddressInfo(SimpleString addressName) {
// even though failover is complete
@Override
public synchronized void addBinding(final Binding binding) throws Exception {
server.callBrokerPlugins(server.hasBrokerPlugins() ? plugin -> plugin.beforeAddBinding(binding) : null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Due to the default uses on callBrokerPlugins and other wierd reasons dependent by how the JIT works with compilations/inlining/interface optimization would be better at least to turn it into:

if(server.hasBrokerPlugins()){
 ....
}

I've noticed it is used on most "cold path" of the broker so it is fine to left as it is in this place too: it doesn't seem a hot path. My suggestion is just for wider puporses 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change it everywhere the plugins are called. There are some hot paths where the plugins are called such as during message routing so if that change will improve performance then that is fine.

Adds callbacks to ActiveMQServerPlugin for binding add and remove events
@cshannon
Copy link
Contributor Author

cshannon commented Feb 1, 2018

@franz1981 - I updated the PR, is this ok to merge now?

@franz1981
Copy link
Contributor

@cshannon Perfect, thanks!!!! 👍

@cshannon
Copy link
Contributor Author

cshannon commented Feb 1, 2018

@franz1981 - can you do the PR merge?

@asfgit asfgit merged commit 51cb5df into apache:master Feb 1, 2018
asfgit pushed a commit that referenced this pull request Feb 1, 2018
@cshannon cshannon deleted the ARTEMIS-1647 branch February 11, 2020 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants