[GitHub] activemq-artemis pull request #2020: ARTEMIS-1812 fix for missing (core) mes...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/2020


---

[GitHub] activemq-artemis pull request #2020: ARTEMIS-1812 fix for missing (core) mes...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2020#discussion_r182439370
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/failover/FailoverTest.java
 ---
@@ -1641,6 +1642,55 @@ public void 
testFailThenReceiveMoreMessagesAfterFailover() throws Exception {
   receiveDurableMessages(consumer);
}
 
+   @Test(timeout = 12)
+   public void testLiveKilledBackupReceivesCoreMessage() throws Exception {
+  createSessionFactory();
+  SimpleString lalaQ = new SimpleString("lalaQ");
+
+  ClientSession session = createSession(sf, true, true);
+
+  AddressSettings addressSettings = new AddressSettings();
+  addressSettings.setAutoDeleteQueues(true).setAutoCreateQueues(true);
+  
backupServer.getServer().getConfiguration().addAddressesSetting(lalaQ.toString(),
 addressSettings);
+
+  session.createQueue(lalaQ, RoutingType.MULTICAST, lalaQ, null, true);
+
+  ClientProducer producer = session.createProducer(lalaQ);
+
+  producer.send(createMessage(session, 0, true));
+  producer.send(createMessage(session, 1, true));
+
+  ClientConsumer consumer = session.createConsumer(lalaQ, true);
+
+  session.start();
+
+  ClientMessage message = consumer.receive(1000);
+  Assert.assertNotNull(message);
+
+  crash(session);
+
+  message = consumer.receive(1000);
+  Assert.assertNotNull(message);
+  message.acknowledge();
+
+  message = consumer.receive(1000);
+  Assert.assertNotNull(message);
+  message.acknowledge();
+
+  message = consumer.receive(1000);
+  Assert.assertNull(message);
+
+
+  //queue should be destroyed after consuming all messages
+  
backupServer.getServer().getActiveMQServerControl().destroyQueue(lalaQ.toString(),
 true);
--- End diff --

The issue is on auto-created queues?


if you need persistent queues and failover.. create them... I wouldn't fix 
this.


---

[GitHub] activemq-artemis pull request #2020: ARTEMIS-1812 fix for missing (core) mes...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2020#discussion_r182438794
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/ServerSessionPacketHandler.java
 ---
@@ -658,6 +658,9 @@ private void onSessionSend(Packet packet) {
  try {
 final SessionSendMessage message = (SessionSendMessage) packet;
 requiresResponse = message.isRequiresResponse();
+if 
(this.session.getMatchingQueue(message.getMessage().getAddressSimpleString(), 
RoutingType.ANYCAST) == null || 
this.session.getMatchingQueue(message.getMessage().getAddressSimpleString(), 
RoutingType.MULTICAST) == null) {
--- End diff --

This will kill performance.. we cannot accept this
Matcing Queue on every onSessionSend.. not happening


---

[GitHub] activemq-artemis pull request #2020: ARTEMIS-1812 fix for missing (core) mes...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2020#discussion_r182086148
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
 ---
@@ -829,7 +829,13 @@ public RoutingStatus route(final Message message,
 
   // TODO auto-create queues here?
   // first check for the auto-queue creation thing
-  if (bindings == null) {
+  if (bindings == null && 
!address.toString().equals("activemq.notifications")) {
--- End diff --

This seems a symptom of another issue instead of a proper fix.

it seems you are fixing a symptom of another issue. Accepting this fix will 
probably hide a real issue somewhere else.


Can you replicate this on a test.. so we can validate the real issue?


---

[GitHub] activemq-artemis pull request #2020: ARTEMIS-1812 fix for missing (core) mes...

[stanlyDoge]

GitHub user stanlyDoge opened a pull request:

https://github.com/apache/activemq-artemis/pull/2020

ARTEMIS-1812 fix for missing (core) messages after master kill (HA)

When I try to send some core messages to slave broker (master is dead), 
messages are lost. Debug console says "Couldn't find any bindings for 
address=TEST.foo". By digging into code, I think this is the spot, which is 
responsible for this bug: 
https://github.com/apache/activemq-artemis/blob/master/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java#L832
 

I have tried to create queue with default values here. It fixes the bug, 
but i am afraid it can create much more new bugs. Git blame says 
@clebertsuconic and @jbertram could know more about this. Can you guys please 
approve/disprove this fix? If it is ok, please do not merge, I would like to 
bring some test for this and delete comments. Thank you.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stanlyDoge/activemq-artemis ARTEMIS-1812

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2020.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2020


commit 688397ea74897d5ad49d4694a74c14849148b8de
Author: Stanislav Knot 
Date:   2018-04-16T13:46:22Z

ARTEMIS-1812 fix for missing (core) messages after master kill (HA)




---