You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2022/04/29 18:10:38 UTC
[GitHub] [activemq] lucastetreault opened a new pull request, #835: Use inclusive terminology for network connector transports
lucastetreault opened a new pull request, #835:
URL: https://github.com/apache/activemq/pull/835
This [tweet](https://twitter.com/owenblacker/status/1517156221207212032) raised the issue of non-inclusive terminology in the [AWS docs](https://docs.aws.amazon.com/amazon-mq/latest/developer-guide/amazon-mq-creating-configuring-network-of-brokers.html#creating-configuring-network-of-brokers-configure-network-connectors) and suggested that we should replace masterslave with an more inclusive name for the network connector transport. The AWS docs refer to a feature of ActiveMQ that is a convenience discovery agent: https://activemq.apache.org/networks-of-brokers#masterslave-discovery
Replacing master/slave nomenclature in ActiveMQ was raised in July 2020 [AMQ-7514](https://issues.apache.org/jira/browse/AMQ-7514) and there have been some attempts at making some changes (https://github.com/apache/activemq/pull/679, https://github.com/apache/activemq/pull/714, https://github.com/apache/activemq/pull/788) however we have not been able to come to an agreement on nomenclature so these efforts seem to have stalled out.
If we are able to come to an agreement on nomenclature in this PR, we can move forward with removing more non-inclusive terminology on the website (I will follow up with some PRs to the website), in discussions with the community and of course in this codebase. This will remove adoption barriers and make ActiveMQ a more approachable and inclusive project for everyone! Other Apache projects such as Solr and Kafka have moved from master/slave to leader/follower. Leader/follower is also recommended by the [IETF](https://tools.ietf.org/id/draft-knodel-terminology-02.html) and [inclusivenaming.org](https://inclusivenaming.org/word-lists/tier-1/) which is supported by companies such as Cisco, Intel, and RedHat.
If we can't come to an agreement on Leader/Follower or some other nomenclature I will, at the very least, create a follow up PR to remove the masterslave transport since it is just a convenience method to use static+failover with ?randomize=false&maxReconnectAttempts=0.
This change leaves the masterslave: transport in place but provides a new alias leaderfollower: for now but we can easily remove it in 5.18.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMQ-8593] Use inclusive terminology for network connector transports [activemq]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1772133082
Auto closed: it will be superseded by full terminology change for ActiveMQ 7.0.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
gemmellr commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1116019688
> @gemmellr
>
> > The other config type is "static":
> > ```
> > <networkConnector uri="static:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > The one under discussion differs in using "failover" transports. So what about something like simply "staticfailover":
> > ```
> > <networkConnector uri="staticfailover:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
> > ```
>
> Why need to make a new name at all? why is simply a nested of existing not suitable here: e.g.
>
> static:(failover:(uri1,uri2)?randomize=false&maxReconnectAttempts=0)
Maybe it would be, though apparently in the past it was considered not to be and this convenience was created instead. At this point I dont see a particular benefit in trying to unwind the convenience, so just changing the prefix seems the simplest to explain to upgraders and the simplest for them to action and so is what I would do. As it actually seemed to have nothing in particular to do with topology or its naming it seemed odd to get caught up on that here at all, and so I suggested an obvious very literal name that fits what its used for.
I'm not against just removing it if everyone else wants to unwind the convenience entirely and then cover things as needed for ungraders, but I thing the rename is simpler for everyone. I dont plan to spend more more time on this than I have already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1219051071
@jbonofre I see you're merging a ton of PRs and I assume you're preparing for the next release. It would be great to see this included in the next version!
It seems like everyone is happy with this PR at this point. Are there any blockers?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
jbonofre commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1219054447
@lucastetreault yes, I will take a look on this one. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1113627234
@mattrpav this PR actually doesn't remove the masterslave transport: activemq-client/src/main/resources/META-INF/services/org/apache/activemq/transport/discoveryagent/masterslave is updated to use the renamed LeaderFollowerDiscoveryAgentFactory.
I can open a Jira to go along with this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1118189121
Thanks for all the feedback folks. I have updated the PR to use the `staticfailover` as proposed by @gemmellr which seems to be right balance of removing non-inclusive language and convenience for users. I plan to email the mailing list for a formal vote on nomenclature as suggested by @michaelpearce-gain on slack 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a diff in pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
gemmellr commented on code in PR #835:
URL: https://github.com/apache/activemq/pull/835#discussion_r863632302
##########
activemq-client/src/main/java/org/apache/activemq/transport/discovery/masterslave/MasterSlaveDiscoveryAgentFactory.java:
##########
@@ -27,6 +27,7 @@
import java.net.URI;
import java.util.Map;
+@Deprecated
Review Comment:
Ditto
##########
activemq-client/src/main/java/org/apache/activemq/transport/discovery/masterslave/MasterSlaveDiscoveryAgent.java:
##########
@@ -24,7 +24,10 @@
/**
* A static DiscoveryAgent that supports connecting to a Master / Slave tuple
* of brokers.
+ *
+ * @deprecated This class is superseded by HADiscoveryAgent and will be removed in 5.18.0.
*/
+@Deprecated
Review Comment:
Can it use the 'for removal' flag? If so it seems like it should. Though i realise most things arent going to touch the code itself so it would really only be for descriptive purposes.
(I've yet to use it myself so not clear when it came in)
##########
activemq-unit-tests/src/test/java/org/apache/activemq/network/NetworkConnectionsTest.java:
##########
@@ -5,9 +5,9 @@
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
Review Comment:
Seems like this should be unwound...but if a change actually needs made, I'd make the licence header a comment rather than Javadoc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMQ-8593] Use inclusive terminology for network connector transports [activemq]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre closed pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
URL: https://github.com/apache/activemq/pull/835
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMQ-8593] Use inclusive terminology for network connector transports [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1852321975
Re-opening-- This is additive, and can be merged without having to wait for the openwire terminology changes (aka v7.0.0)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
gemmellr commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115923343
Since the docs say this mechanism is essentially an alternative means of specifying static "failover:" transport usages with some unnamed options, and the code seemingly confirms that from looking like just a loop for creating a "failover://(looped-given-servers)?randomize=false&maxReconnectAttempts=0" string, why not just name it more literally about what the config actually does rather than trying to come up with some new abstractive topology name it seems people have a hard time agreeing on.
The other config type is "static":
```
<networkConnector uri="static:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
```
The one under discussion differs in using "failover" transports. So what about something like simply "staticfailover":
```
<networkConnector uri="staticfailover:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1113870651
@mattrpav I pushed another commit to address your comments. Let me know if there's anything else 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
mattrpav commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1114951122
@lucastetreault thanks for updating to include deprecated for masterslave.
In terms of naming-- I think "ha://" is more accurate (and shorter too!). 'leaderfollower' is not great here, since there is no need to name an architecture. This transport handler really is just 'failover'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115956606
> Def needs a deprecation log entry if old config is read up, so that users are alerted maybe if still using old config. A
How about: `LOG.warn("masterSlave is deprecated and will be removed in a future release. Use {newname} instead.");`
> The one under discussion differs in using "failover" transports. So what about something like simply "staticfailover":
I like that. Very literal which is probably good in this case.
I still think it would be valuable to try to get to a consensus on what terms should replace master/slave. Any thoughts on how we can go about that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] michaelpearce-gain commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115963086
>
> The other config type is "static":
>
> ```
> <networkConnector uri="static:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
> ```
>
> The one under discussion differs in using "failover" transports. So what about something like simply "staticfailover":
>
> ```
> <networkConnector uri="staticfailover:(tcp://host1:61616,tcp://host2:61616,tcp://..)"/>
> ```
Why need to make a new name at all? why is simply a nested of existing not suitable here: e.g.
static:(failover:(uri1,uri2)?randomize=false&maxReconnectAttempts=0)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
mattrpav commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1116119107
+1 'staticfailover' makes sense as the uri prefix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1113856652
@mattrpav Jira for this change: https://issues.apache.org/jira/browse/AMQ-8593
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1120343751
I created a PR for the corresponding doc changes assuming this will be merged and available in 5.17.2 when it is released: https://github.com/apache/activemq-website/pull/83
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] michaelpearce-gain commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115859305
Def needs a deprecation log entry if old config is read up, so that users are alerted maybe if still using old config. A
lso im a little -1 using terminology "ha" ha = highly available of which there is many ways to be "highly available" which leader/follower primary/secondary is just one way.... also on naming i believe was front runners for replacement of master/slave terminology.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
gemmellr commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115911687
> I assumed we could merge this in to a 5.17.X release and then it would be fair game to remove in 5.18.0. Is there some written guidance on due notice for this type of change that I could refer to?
5.17.x and 5.18.x wont necessarily be coexisting for as long (or maybe at all) as some previous cases have, and it seems likely some folks may update to one of the earlier 5.17.x releases already existing and stick for a while and not see this in any later 5.17.x releases before e.g trying 5.18.0. Overall, may be better to consider some period of time where the deprecated option availability overlaps the new one rather than just a specific version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #835: Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
mattrpav commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1113607746
I don't think we should just 'drop' masterslave. It will break compatibility for other users.
Can you please create a JIRA to discuss?
I think we should:
1. Add a new discovery agent
2. Mark masterslave as deprecated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #835: Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
mattrpav commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1113639442
@lucastetreault at the very least, it would help to have unit tests showing the old name still works, vs refactoring that test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] michaelpearce-gain commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115965841
> I still think it would be valuable to try to get to a consensus on what terms should replace master/slave. Any thoughts on how we can go about that?
essentially requires a vote on dev mail list if you need to have a decision, for the community and pmc to cast their votes.
This said, for this change, is it even needed? Why not go with simply nested of existing names/url transports per same comment to robbie and which i believe you even highlighted in slack yourself?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115978234
> > I still think it would be valuable to try to get to a consensus on what terms should replace master/slave. Any thoughts on how we can go about that?
>
> essentially requires a vote on dev mail list if you need to have a decision, for the community and pmc to cast their votes.
>
> This said, for this change, is it even needed we make a new name.....? Why not go with simply nested of existing names/url transports per same comment to robbie and which i believe you even highlighted in slack yourself? Are we really saving anything from telling users to put :
>
> (already supported) static:(failover:(uri1,uri2....
>
> vs
>
> staticfailover(uri1,uri2.....
I'm fine if the solution in the codebase is to simply remove the convenience method. Even so, I think we need to agree on new terms. For example, to update https://activemq.apache.org/networks-of-brokers#masterslave-discovery with new instructions, how do I refer to the pair of brokers using shared storage if not master/slave? I know Matt proposed some wording on slack that dances around the problem, but there are lots of other places in the docs that could use updating as well :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] michaelpearce-gain commented on pull request #835: [AMQ-8593] Use inclusive terminology for network connector transports
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on PR #835:
URL: https://github.com/apache/activemq/pull/835#issuecomment-1115984626
So to replace wording or master/slave with either primary/secondary or leader/follower as i stated requires a proposal and a vote in dev mailing list plain and simple there is no way to dance around it, in slack or PR's.
But for this PR i think just deprecation (not yet removal) of convenience method is simplest and cleanest. IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org