[GitHub] [activemq] mattrpav commented on a change in pull request #679: [AMQ-7514] Deprecate configuration that uses non-inclusive language

[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