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 2021/07/08 19:24:18 UTC
[GitHub] [activemq] ehossack-aws opened a new pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
ehossack-aws opened a new pull request #679:
URL: https://github.com/apache/activemq/pull/679
This is the first in a series of changes that are planned to phase out non-inclusive terminology throughout the config, and codebase.
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666471550
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Active doesnt sound like a good replacement master
Typically we would refer to master or slave being the current active node.
Active = state
Master / slave was what role the node took.
I think a different word to replace master should be found e.g.(primary/standby) (primary/replica)
--
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] jbertram commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666642146
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
I agree with @michaelandrepearce. I've added a comment to AMQ-7514 to push along the discussion.
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r667162581
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
I did a 'find references' search and yeah, you're right @mattrpav. I think this is also relevant in the discussion about active/passive/standby. It seems reasonable in the scope of this change rather than renaming to actually remove those methods/concepts. Then there is no need to consider the 'existing' 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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too. Especially when leveldb (i think the wording used then was "pure master slave") and replicated kahadb was done. So some legacy nomiculture there with what master slave meant along with active and passive.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
Im also keen we align with Artemis broker for consistency sakes on the way forward here.
Happy for others to chip in here. As this is one of those things that once released its a done deal...
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r667162869
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
try {
stop();
} catch (Exception e) {
- LOG.error("Failed to stop for master failure", e);
+ LOG.error("Failed to stop for active failure", e);
Review comment:
How about in the scope of this PR we add both messages, and then the subtask is to remove the non-inclusive messages? That would support a transition period 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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
This is down to replication state so primary replica should be used
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666486518
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -152,10 +152,15 @@
private boolean useShutdownHook = true;
private boolean useLoggingForShutdownErrors;
private boolean shutdownOnMasterFailure;
+ private boolean shutdownOnActiveFailure;
Review comment:
Hmm good point, my brain didn't catch that as my first pass was assuming spring looked at the actual variable names.
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666486518
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -152,10 +152,15 @@
private boolean useShutdownHook = true;
private boolean useLoggingForShutdownErrors;
private boolean shutdownOnMasterFailure;
+ private boolean shutdownOnActiveFailure;
Review comment:
Hmm good point, my brain didn't catch that as my first pass was assuming spring looked at the actual variable names.
edit: xbean
--
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 a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
mattrpav commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666927309
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
try {
stop();
} catch (Exception e) {
- LOG.error("Failed to stop for master failure", e);
+ LOG.error("Failed to stop for active failure", e);
Review comment:
Updating log messages is tricky, since it can break monitoring in not-so-easy-to-upgrade ways. I think this should be a sub-task conversation on the parent ticket.
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
This group of methods is never read anywhere by the code base. My understanding is that they are legacy hold-over from early replication approach that has been removed long ago. I do not see any reason to deprecate them. Given that we have agreed to make major breaking changes in 5.17.x (remove leveldb), I believe we can safely remove these methods in this pass.
We should not be adding new terms or concepts that do not apply to the code base in this pass.
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -2896,51 +2922,142 @@ public void setSslContext(SslContext sslContext) {
this.sslContext = sslContext;
}
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public boolean isShutdownOnSlaveFailure() {
return shutdownOnSlaveFailure;
}
/**
+ * Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param shutdownOnSlaveFailure
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setShutdownOnSlaveFailure(boolean shutdownOnSlaveFailure) {
this.shutdownOnSlaveFailure = shutdownOnSlaveFailure;
+ this.shutdownOnStandbyFailure = shutdownOnSlaveFailure;
+ LOG.warn("Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.");
}
+ public boolean isShutdownOnStandbyFailure() {
+ return shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
+ */
+ public void setShutdownOnStandbyFailure(boolean shutdownOnStandbyFailure) {
+ this.shutdownOnSlaveFailure = shutdownOnStandbyFailure;
+ this.shutdownOnStandbyFailure = shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
Review comment:
This method is never called in the code base. My understanding is that it was kept to not break configurations from the long-gone active-active replication. I would contend that since we have agreed to remove leveldb from 5.17.x and there are no planned usages for these metods, that this method (and related ones) could safely be removed instead of 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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666492182
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
I disagree with the ticket then
Active is state though. Either master or slave can be active. We will end up with very confusing terminology.
https://www.theserverside.com/opinion/Master-slave-terminology-alternatives-you-can-use-right-now
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r693215288
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
as per @mattrpav 's AMQ-8316, moved the two concretely deprecated messages out of this PR
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
as per @mattrpav 's AMQ-8316, moved the two concretely deprecated messages out of this PR into https://github.com/apache/activemq/pull/700
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666492182
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
I disagree with the ticket then
Active is state though. Either master or slave can be active. We will end up with very confusing terminology.
https://www.theserverside.com/opinion/Master-slave-terminology-alternatives-you-can-use-right-now
Also i would appreciate the discussion is not based on because aws does it. This is an apache project not an aws one.
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666489299
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
The [ticket](https://issues.apache.org/jira/browse/AMQ-7514) mentions active/standby and I'd tend to agree that it's the clearest (it's also what we use at AWS).
Consider:
- of the nodes, only 1 is _active_ serving requests
- the other nodes are not pure replicas, as they have shared storage, rather they are on standby over the same FS
It's possible that in the previous replicated leveldb one could think of replica as a good term here, but I think it's slightly disingenuous. (especially if the storage used isn't actually replicated...)
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666469864
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -152,10 +152,15 @@
private boolean useShutdownHook = true;
private boolean useLoggingForShutdownErrors;
private boolean shutdownOnMasterFailure;
+ private boolean shutdownOnActiveFailure;
Review comment:
Its a private field no need to duplicate just rename the original field. Just need to keep method compatibility
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too. Especially when leveldb (i think the wording used then was "pure master slave") and replicated kahadb was done. So some legacy nomiculture there with what master slave meant along with active and passive.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica (or primary standby) should maybe be used
Im also keen we align with Artemis broker for consistency sakes on the way forward here.
Happy for others to chip in here. As this is one of those things that once released its a done deal...
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
Im also keen we align with Artemis broker for consistency sakes on the way forward here.
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666471550
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Active doesnt sound like a good replacement for master
Typically we would refer to master or slave and which one is serving being the current active node.
Active = state
Master / slave was what role the node took.
I think a different word to replace master should be found e.g.(primary/standby) (primary/replica)
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666470343
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -152,10 +152,15 @@
private boolean useShutdownHook = true;
private boolean useLoggingForShutdownErrors;
private boolean shutdownOnMasterFailure;
+ private boolean shutdownOnActiveFailure;
Review comment:
This comment applies generally. No need for me to repeat on every occurance
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
Im also keen we align with Artemis broker for consistency sakes on the way forward here.
Happy for others to chip in here. As this is one of those things that once released its a done deal...
--
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 a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
mattrpav commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r667972064
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
try {
stop();
} catch (Exception e) {
- LOG.error("Failed to stop for master failure", e);
+ LOG.error("Failed to stop for active failure", e);
Review comment:
2 messages could yield 2 alerts. I think we break this up into some sub-tasks and add a broker config param to flip b/w the log messages using deprecated terms.
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
try {
stop();
} catch (Exception e) {
- LOG.error("Failed to stop for master failure", e);
+ LOG.error("Failed to stop for active failure", e);
Review comment:
2 messages could yield 2 alerts. I think we break this up into some sub-tasks and add a broker config param to flip b/w the log messages using deprecated terms. I'll update the JIRA with some sub-tasks.
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -2896,51 +2922,142 @@ public void setSslContext(SslContext sslContext) {
this.sslContext = sslContext;
}
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public boolean isShutdownOnSlaveFailure() {
return shutdownOnSlaveFailure;
}
/**
+ * Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param shutdownOnSlaveFailure
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setShutdownOnSlaveFailure(boolean shutdownOnSlaveFailure) {
this.shutdownOnSlaveFailure = shutdownOnSlaveFailure;
+ this.shutdownOnStandbyFailure = shutdownOnSlaveFailure;
+ LOG.warn("Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.");
}
+ public boolean isShutdownOnStandbyFailure() {
+ return shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
+ */
+ public void setShutdownOnStandbyFailure(boolean shutdownOnStandbyFailure) {
+ this.shutdownOnSlaveFailure = shutdownOnStandbyFailure;
+ this.shutdownOnStandbyFailure = shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public boolean isWaitForSlave() {
return waitForSlave;
}
/**
+ * Configuration `waitForSlave` is deprecated and will be removed in a future release. Use `waitForStandby` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param waitForSlave
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setWaitForSlave(boolean waitForSlave) {
this.waitForSlave = waitForSlave;
+ this.waitForStandby = waitForSlave;
+ LOG.warn("Configuration `waitForSlave` is deprecated and will be removed in a future release. Use `waitForStandby` instead.");
}
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public long getWaitForSlaveTimeout() {
return this.waitForSlaveTimeout;
}
+ /**
+ * Configuration `waitForSlaveTimeout` is deprecated and will be removed in a future release. Use `waitForStandbyTimeout` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param waitForSlaveTimeout
+ */
+ @Deprecated
public void setWaitForSlaveTimeout(long waitForSlaveTimeout) {
this.waitForSlaveTimeout = waitForSlaveTimeout;
+ this.waitForStandbyTimeout = waitForSlaveTimeout;
+ LOG.warn("Configuration `waitForSlaveTimeout` is deprecated and will be removed in a future release. Use `waitForStandbyTimeout` instead.");
+
+ }
+
+ public boolean isWaitForStandby() {
+ return waitForStandby;
}
/**
- * Get the passiveSlave
+ * @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
+ */
+ public void setWaitForStandby(boolean waitForStandby) {
+ this.waitForStandby = waitForStandby;
+ this.waitForSlave = waitForStandby;
+ }
+
+ public long getWaitForStandbyTimeout() {
+ return this.waitForStandbyTimeout;
+ }
+
+ public void setWaitForStandbyTimeout(long waitForStandbyTimeout) {
+ this.waitForSlaveTimeout = waitForStandbyTimeout;
+ this.waitForStandbyTimeout = waitForStandbyTimeout;
+ }
+
+ /**
+ * Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
* @return the passiveSlave
*/
+ @Deprecated
public boolean isPassiveSlave() {
+ LOG.warn("Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.");
return this.passiveSlave;
}
/**
- * Set the passiveSlave
+ * Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
* @param passiveSlave the passiveSlave to set
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setPassiveSlave(boolean passiveSlave) {
this.passiveSlave = passiveSlave;
+ this.standbyInstance = passiveSlave;
+ LOG.warn("Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.");
Review comment:
Yes, I think we break-up the PR's a bit. Removing deprecated methods can be its own thing
--
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] ehossack-aws commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r667163323
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -2896,51 +2922,142 @@ public void setSslContext(SslContext sslContext) {
this.sslContext = sslContext;
}
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public boolean isShutdownOnSlaveFailure() {
return shutdownOnSlaveFailure;
}
/**
+ * Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param shutdownOnSlaveFailure
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setShutdownOnSlaveFailure(boolean shutdownOnSlaveFailure) {
this.shutdownOnSlaveFailure = shutdownOnSlaveFailure;
+ this.shutdownOnStandbyFailure = shutdownOnSlaveFailure;
+ LOG.warn("Configuration `shutdownOnSlaveFailure` is deprecated and will be removed in a future release. Use `shutdownOnStandbyFailure` instead.");
}
+ public boolean isShutdownOnStandbyFailure() {
+ return shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
+ */
+ public void setShutdownOnStandbyFailure(boolean shutdownOnStandbyFailure) {
+ this.shutdownOnSlaveFailure = shutdownOnStandbyFailure;
+ this.shutdownOnStandbyFailure = shutdownOnStandbyFailure;
+ }
+
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public boolean isWaitForSlave() {
return waitForSlave;
}
/**
+ * Configuration `waitForSlave` is deprecated and will be removed in a future release. Use `waitForStandby` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param waitForSlave
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setWaitForSlave(boolean waitForSlave) {
this.waitForSlave = waitForSlave;
+ this.waitForStandby = waitForSlave;
+ LOG.warn("Configuration `waitForSlave` is deprecated and will be removed in a future release. Use `waitForStandby` instead.");
}
+ /**
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ */
+ @Deprecated
public long getWaitForSlaveTimeout() {
return this.waitForSlaveTimeout;
}
+ /**
+ * Configuration `waitForSlaveTimeout` is deprecated and will be removed in a future release. Use `waitForStandbyTimeout` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
+ * @param waitForSlaveTimeout
+ */
+ @Deprecated
public void setWaitForSlaveTimeout(long waitForSlaveTimeout) {
this.waitForSlaveTimeout = waitForSlaveTimeout;
+ this.waitForStandbyTimeout = waitForSlaveTimeout;
+ LOG.warn("Configuration `waitForSlaveTimeout` is deprecated and will be removed in a future release. Use `waitForStandbyTimeout` instead.");
+
+ }
+
+ public boolean isWaitForStandby() {
+ return waitForStandby;
}
/**
- * Get the passiveSlave
+ * @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
+ */
+ public void setWaitForStandby(boolean waitForStandby) {
+ this.waitForStandby = waitForStandby;
+ this.waitForSlave = waitForStandby;
+ }
+
+ public long getWaitForStandbyTimeout() {
+ return this.waitForStandbyTimeout;
+ }
+
+ public void setWaitForStandbyTimeout(long waitForStandbyTimeout) {
+ this.waitForSlaveTimeout = waitForStandbyTimeout;
+ this.waitForStandbyTimeout = waitForStandbyTimeout;
+ }
+
+ /**
+ * Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
* @return the passiveSlave
*/
+ @Deprecated
public boolean isPassiveSlave() {
+ LOG.warn("Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.");
return this.passiveSlave;
}
/**
- * Set the passiveSlave
+ * Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.
+ * @deprecated AMQ-7514 in the move to inclusive nomenclature
* @param passiveSlave the passiveSlave to set
* @org.apache.xbean.Property propertyEditor="org.apache.activemq.util.BooleanEditor"
*/
+ @Deprecated
public void setPassiveSlave(boolean passiveSlave) {
this.passiveSlave = passiveSlave;
+ this.standbyInstance = passiveSlave;
+ LOG.warn("Configuration `passiveSlave` is deprecated and will be removed in a future release. Use `standbyInstance` instead.");
Review comment:
@mattrpav per your comment it also appears `isPassiveSlave` is not used. We could also remove this it seems like?
--
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] ehossack commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
ehossack commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r817043450
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -152,10 +152,15 @@
private boolean useShutdownHook = true;
private boolean useLoggingForShutdownErrors;
private boolean shutdownOnMasterFailure;
+ private boolean shutdownOnActiveFailure;
Review comment:
This was addressed in latest version of 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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too. Especially when leveldb and replicated kahadb was done. So some legacy nomiculture there with what master slave meant along with active and passive.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
Im also keen we align with Artemis broker for consistency sakes on the way forward here.
Happy for others to chip in here. As this is one of those things that once released its a done deal...
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666492182
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
I disagree with the ticket then
Active is state though. Either master or slave can be active. E.g. we talk about a slave being passive and then if master fails it becomes active. We will end up with very confusing terminology.
https://www.theserverside.com/opinion/Master-slave-terminology-alternatives-you-can-use-right-now
Also i would appreciate the discussion is not based on because aws does it. This is an apache project not an aws one.
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666493721
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Quickly reading the ticket actually mentions
New terms:
a. For shared storage: ‘active’ and ’standby’
b. For replication: ‘primary’ and ‘replica'
c. For 'white list' and 'blacklist': 'allow list' and 'deny list'
Like wise earlier in the same ticket it mentions
Here are just a few terms that should be changed: * The following terms are being targeted for change:
'master' and 'slave' should be replaced with the terms 'live' and 'backup''whitelist' and 'blacklist' should be replaced with the terms 'allowlist' and 'denylist'Rename all the git 'master' branches to the term 'main'
A little using terminology within context and avoiding conflicting terminology is needed here.
I agree i think also we historically overloaded the use of master slave too.
So in this case its the replication one, e.g. the broker is either the primary instance or the replica instance .. so primary replica should be used
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666471550
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Active doesnt sound like a good replacement master
Typically we would refer to master or slave being the current active node.
Active = state
Master / slave was what role the node took.
I think a different word to replace master should be found
--
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] michaelandrepearce commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #679:
URL: https://github.com/apache/activemq/pull/679#discussion_r666471550
##########
File path: activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java
##########
@@ -485,15 +490,15 @@ public JmsConnector removeJmsConnector(JmsConnector connector) {
}
public void masterFailed() {
- if (shutdownOnMasterFailure) {
- LOG.error("The Master has failed ... shutting down");
+ if (shutdownOnActiveFailure) {
+ LOG.error("The Active has failed ... shutting down");
Review comment:
Active doesnt sound like a good replacement for master
Typically we would refer to master or slave and which one is serving being the current active node.
Active and passive = state
Master / slave was what role the node took.
I think a different word to replace master should be found e.g.(primary/standby) (primary/replica)
--
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