Skip to content

Commit f43bf52

Browse files
jbertramjbertram
committed
ARTEMIS-6043 updating divert w/mngmnt API can persist invalid config
This commit fixes two related issues: - Ensures the broker can still start if recovering a persisted divert configuration fails - Ensures the broker does not persist an invalid divert configuration
1 parent ed1b48e commit f43bf52

9 files changed

Lines changed: 254 additions & 22 deletions

File tree

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/config/DivertConfiguration.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ public class DivertConfiguration implements Serializable, EncodingSupport {
6666
public DivertConfiguration() {
6767
}
6868

69+
public DivertConfiguration(DivertConfiguration config) {
70+
this.name = config.getName();
71+
this.routingName = config.getRoutingName();
72+
this.address = config.getAddress();
73+
this.forwardingAddress = config.getForwardingAddress();
74+
this.exclusive = config.isExclusive();
75+
this.filterString = config.getFilterString();
76+
this.transformerConfiguration = config.getTransformerConfiguration();
77+
this.routingType = config.getRoutingType();
78+
}
79+
6980
/**
7081
* Set the value of a parameter based on its "key" {@code String}. Valid key names and corresponding {@code static}
7182
* {@code final} are:
@@ -108,7 +119,7 @@ public DivertConfiguration set(String key, String value) {
108119
setTransformerConfiguration(transformerConfiguration);
109120
}
110121
} else if (key.equals(ROUTING_TYPE)) {
111-
setRoutingType(ComponentConfigurationRoutingType.valueOf(value));
122+
setRoutingType(value == null ? null : ComponentConfigurationRoutingType.valueOf(value));
112123
}
113124
}
114125
return this;
@@ -227,7 +238,9 @@ public String toJSON() {
227238
builder.add(TRANSFORMER_CONFIGURATION, tc.createJsonObjectBuilder());
228239
}
229240

230-
if (getRoutingType() != null) {
241+
if (getRoutingType() == null) {
242+
builder.add(ROUTING_TYPE, JsonValue.NULL);
243+
} else {
231244
builder.add(ROUTING_TYPE, getRoutingType().name());
232245
}
233246

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/StorageManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.activemq.artemis.api.core.Message;
3131
import org.apache.activemq.artemis.api.core.Pair;
3232
import org.apache.activemq.artemis.api.core.SimpleString;
33+
import org.apache.activemq.artemis.core.config.DivertConfiguration;
3334
import org.apache.activemq.artemis.core.io.IOCallback;
3435
import org.apache.activemq.artemis.core.io.OperationConsistencyLevel;
3536
import org.apache.activemq.artemis.core.io.SequentialFile;
@@ -391,6 +392,8 @@ JournalLoadInformation loadBindingJournal(List<QueueBindingInfo> queueBindingInf
391392

392393
List<PersistedDivertConfiguration> recoverDivertConfigurations();
393394

395+
DivertConfiguration getDivertConfiguration(String name);
396+
394397
void storeBridgeConfiguration(PersistedBridgeConfiguration persistedBridgeConfiguration) throws Exception;
395398

396399
void deleteBridgeConfiguration(String bridgeName) throws Exception;

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.apache.activemq.artemis.api.core.SimpleString;
4747
import org.apache.activemq.artemis.core.buffers.impl.ChannelBufferWrapper;
4848
import org.apache.activemq.artemis.core.config.Configuration;
49+
import org.apache.activemq.artemis.core.config.DivertConfiguration;
4950
import org.apache.activemq.artemis.core.filter.Filter;
5051
import org.apache.activemq.artemis.core.io.IOCallback;
5152
import org.apache.activemq.artemis.core.io.IOCriticalErrorListener;
@@ -845,6 +846,16 @@ public List<PersistedDivertConfiguration> recoverDivertConfigurations() {
845846
return new ArrayList<>(mapPersistedDivertConfigurations.values());
846847
}
847848

849+
@Override
850+
public DivertConfiguration getDivertConfiguration(String name) {
851+
PersistedDivertConfiguration persistedDivertConfiguration = mapPersistedDivertConfigurations.get(name);
852+
if (persistedDivertConfiguration != null) {
853+
return new DivertConfiguration(persistedDivertConfiguration.getDivertConfiguration());
854+
} else {
855+
return null;
856+
}
857+
}
858+
848859
@Override
849860
public void storeBridgeConfiguration(PersistedBridgeConfiguration persistedBridgeConfiguration) throws Exception {
850861
storeConfiguration(persistedBridgeConfiguration, mapPersistedBridgeConfigurations);

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/nullpm/NullStorageManager.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.activemq.artemis.api.core.Message;
3333
import org.apache.activemq.artemis.api.core.Pair;
3434
import org.apache.activemq.artemis.api.core.SimpleString;
35+
import org.apache.activemq.artemis.core.config.DivertConfiguration;
3536
import org.apache.activemq.artemis.core.io.IOCallback;
3637
import org.apache.activemq.artemis.core.io.IOCriticalErrorListener;
3738
import org.apache.activemq.artemis.core.io.OperationConsistencyLevel;
@@ -498,6 +499,11 @@ public List<PersistedDivertConfiguration> recoverDivertConfigurations() {
498499
return null;
499500
}
500501

502+
@Override
503+
public DivertConfiguration getDivertConfiguration(String name) {
504+
return null;
505+
}
506+
501507
@Override
502508
public void storeBridgeConfiguration(PersistedBridgeConfiguration persistedBridgeConfiguration) throws Exception {
503509
}

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,4 +1544,7 @@ void slowConsumerDetected(String sessionID,
15441544

15451545
@LogMessage(id = 224163, value = "Failed to clone SHA256 MessageDigest, falling back to getInstance", level = LogMessage.Level.INFO)
15461546
void sha256CloneNotSupported(CloneNotSupportedException cns);
1547+
1548+
@LogMessage(id = 224164, value = "Failed to recover stored configuration for divert named '{}': {}. To repair this record create a new divert with the same name via the management API.", level = LogMessage.Level.WARN)
1549+
void failedToRecoverStoredDivertConfiguration(String divertName, String divert);
15471550
}

‎artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java‎

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3069,38 +3069,56 @@ public Divert updateDivert(DivertConfiguration config) throws Exception {
30693069
return null;
30703070
}
30713071

3072-
final Divert divert = divertBinding.getDivert();
3072+
DivertConfiguration onStorageDivert = storageManager.getDivertConfiguration(config.getName());
3073+
final Divert inMemoryDivert = divertBinding.getDivert();
30733074

30743075
Filter filter = FilterImpl.createFilter(config.getFilterString());
30753076
if (filter == null) {
3076-
divert.setFilter(null);
3077+
inMemoryDivert.setFilter(null);
3078+
if (onStorageDivert != null) {
3079+
onStorageDivert.setFilterString(null);
3080+
}
30773081
} else {
3078-
if (!filter.equals(divert.getFilter())) {
3079-
divert.setFilter(filter);
3082+
if (!filter.equals(inMemoryDivert.getFilter())) {
3083+
inMemoryDivert.setFilter(filter);
3084+
if (onStorageDivert != null) {
3085+
onStorageDivert.setFilterString(config.getFilterString());
3086+
}
30803087
}
30813088
}
30823089

30833090
if (config.getTransformerConfiguration() != null) {
3084-
getServiceRegistry().removeDivertTransformer(divert.getUniqueName().toString());
3091+
getServiceRegistry().removeDivertTransformer(inMemoryDivert.getUniqueName().toString());
30853092
Transformer transformer = getServiceRegistry().getDivertTransformer(
30863093
config.getName(), config.getTransformerConfiguration());
3087-
divert.setTransformer(transformer);
3094+
inMemoryDivert.setTransformer(transformer);
3095+
if (onStorageDivert != null) {
3096+
onStorageDivert.setTransformerConfiguration(config.getTransformerConfiguration());
3097+
}
30883098
}
30893099

30903100
if (config.getForwardingAddress() != null) {
30913101
SimpleString forwardAddress = SimpleString.of(config.getForwardingAddress());
3092-
if (!forwardAddress.equals(divert.getForwardAddress())) {
3093-
divert.setForwardAddress(forwardAddress);
3102+
if (!forwardAddress.equals(inMemoryDivert.getForwardAddress())) {
3103+
inMemoryDivert.setForwardAddress(forwardAddress);
3104+
if (onStorageDivert != null) {
3105+
onStorageDivert.setForwardingAddress(config.getForwardingAddress());
3106+
}
30943107
}
30953108
}
30963109

3097-
if (config.getRoutingType() != null && divert.getRoutingType() != config.getRoutingType()) {
3098-
divert.setRoutingType(config.getRoutingType());
3110+
if (config.getRoutingType() != null && inMemoryDivert.getRoutingType() != config.getRoutingType()) {
3111+
inMemoryDivert.setRoutingType(config.getRoutingType());
3112+
if (onStorageDivert != null) {
3113+
onStorageDivert.setRoutingType(config.getRoutingType());
3114+
}
30993115
}
31003116

3101-
storageManager.storeDivertConfiguration(new PersistedDivertConfiguration(config));
3117+
if (onStorageDivert != null) {
3118+
storageManager.storeDivertConfiguration(new PersistedDivertConfiguration(onStorageDivert));
3119+
}
31023120

3103-
return divert;
3121+
return inMemoryDivert;
31043122
}
31053123

31063124
@Override
@@ -4465,15 +4483,20 @@ private void recoverStoredDiverts() throws Exception {
44654483
if (storageManager.recoverDivertConfigurations() != null) {
44664484

44674485
for (PersistedDivertConfiguration persistedDivertConfiguration : storageManager.recoverDivertConfigurations()) {
4468-
//has it been removed from config
4469-
boolean deleted = configuration.getDivertConfigurations().stream().noneMatch(divertConfiguration -> divertConfiguration.getName().equals(persistedDivertConfiguration.getName()));
4470-
// if it has remove it if configured to do so
4471-
if (deleted) {
4472-
if (addressSettingsRepository.getMatch(persistedDivertConfiguration.getDivertConfiguration().getAddress()).getConfigDeleteDiverts() == DeletionPolicy.FORCE) {
4473-
storageManager.deleteDivertConfiguration(persistedDivertConfiguration.getName());
4474-
} else {
4475-
deployDivert(persistedDivertConfiguration.getDivertConfiguration());
4486+
try {
4487+
//has it been removed from config
4488+
boolean deleted = configuration.getDivertConfigurations().stream().noneMatch(divertConfiguration -> divertConfiguration.getName().equals(persistedDivertConfiguration.getName()));
4489+
// if it has remove it if configured to do so
4490+
if (deleted) {
4491+
if (addressSettingsRepository.getMatch(persistedDivertConfiguration.getDivertConfiguration().getAddress()).getConfigDeleteDiverts() == DeletionPolicy.FORCE) {
4492+
storageManager.deleteDivertConfiguration(persistedDivertConfiguration.getName());
4493+
} else {
4494+
deployDivert(persistedDivertConfiguration.getDivertConfiguration());
4495+
}
44764496
}
4497+
} catch (Exception e) {
4498+
logger.debug(e.getMessage(), e);
4499+
ActiveMQServerLogger.LOGGER.failedToRecoverStoredDivertConfiguration(persistedDivertConfiguration.getName(), String.valueOf(persistedDivertConfiguration.getDivertConfiguration()));
44774500
}
44784501
}
44794502
}

‎artemis-server/src/test/java/org/apache/activemq/artemis/core/transaction/impl/TransactionImplTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.activemq.artemis.api.core.Message;
3434
import org.apache.activemq.artemis.api.core.Pair;
3535
import org.apache.activemq.artemis.api.core.SimpleString;
36+
import org.apache.activemq.artemis.core.config.DivertConfiguration;
3637
import org.apache.activemq.artemis.core.io.IOCallback;
3738
import org.apache.activemq.artemis.core.io.OperationConsistencyLevel;
3839
import org.apache.activemq.artemis.core.io.SequentialFile;
@@ -746,6 +747,11 @@ public List<PersistedDivertConfiguration> recoverDivertConfigurations() {
746747
return null;
747748
}
748749

750+
@Override
751+
public DivertConfiguration getDivertConfiguration(String name) {
752+
return null;
753+
}
754+
749755
@Override
750756
public void storeBridgeConfiguration(PersistedBridgeConfiguration persistedBridgeConfiguration) throws Exception {
751757
}

‎tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/SendAckFailTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.apache.activemq.artemis.api.core.client.ClientSessionFactory;
4545
import org.apache.activemq.artemis.api.core.client.ServerLocator;
4646
import org.apache.activemq.artemis.core.config.Configuration;
47+
import org.apache.activemq.artemis.core.config.DivertConfiguration;
4748
import org.apache.activemq.artemis.core.config.impl.SecurityConfiguration;
4849
import org.apache.activemq.artemis.core.io.IOCallback;
4950
import org.apache.activemq.artemis.core.io.OperationConsistencyLevel;
@@ -748,6 +749,11 @@ public List<PersistedDivertConfiguration> recoverDivertConfigurations() {
748749
return null;
749750
}
750751

752+
@Override
753+
public DivertConfiguration getDivertConfiguration(String name) {
754+
return null;
755+
}
756+
751757
@Override
752758
public void storeBridgeConfiguration(PersistedBridgeConfiguration persistedBridgeConfiguration) throws Exception {
753759
}

0 commit comments

Comments
 (0)