activemq icon indicating copy to clipboard operation
activemq copied to clipboard

Feature/replica broker

Open NikitaShupletsov opened this issue 3 years ago • 15 comments

AMQ-8354

Asynchronous replication plugin

NikitaShupletsov avatar Jun 20 '22 20:06 NikitaShupletsov

What will be the behavior when the replication queue is full and the replica broker is unreachable? Have you considered this case?

trex-amazon avatar Oct 25 '22 06:10 trex-amazon

What will be the behavior when the replication queue is full and the replica broker is unreachable? Have you considered this case?

Sorry for the late reply. We've come up with the idea of so called "message compaction". we scan the replication queue and delete events that cancel each other out(like send and ack)

NikitaShupletsov avatar Jan 04 '23 23:01 NikitaShupletsov

This is a big feature I would have expected some discussion on mailing lists around it and it's design. Maybe I missed it?

michaelandrepearce avatar Jan 05 '23 01:01 michaelandrepearce

This is a big feature I would have expected some discussion on mailing lists around it and it's design. Maybe I missed it?

I tried to initiate a discussion a couple times, but it didn't get too far. the thread: https://lists.apache.org/thread/kzt173o56qdok1m0hghgo0564812nyzd

NikitaShupletsov avatar Jan 05 '23 02:01 NikitaShupletsov

It's important to gain support from community, committers and pmc on dev list first for the feature. Especially in the instance here it seems this is one of your first contributions to the community. Maybe now you have this draft try to reignite it?

michaelandrepearce avatar Jan 05 '23 02:01 michaelandrepearce

@michaelandrepearce this work was started quite some time ago by @ehossack-aws when he was with the Amazon MQ team. He published this design and attempted to get feedback on the dev mailing list but I don't think there was much discussion.

I haven't seen Nikita's latest changes so I was planning on doing a review and leaving some comments. Is there anything more you think I can do to help get the ball rolling? I know this is an important feature for the Amazon MQ team so it would be great it we can figure out how to get the community behind it!

lucastetreault avatar Jan 05 '23 03:01 lucastetreault

A bit missing I'm struggling with how the acks back from a mirror is working reliably here where clients can be connect to either. I'm assuming you're trying to mimick Artemis here.

The other bit assuming jms2 is still a target for the classic activemq 5.x stream, is how this fits into the design of support shared subs in that roadmap. E.g. ensuring this doesn't trip that initiative / effort at all.

michaelandrepearce avatar Jan 05 '23 03:01 michaelandrepearce

Are there testing numbers are available to review? I recall a previous convo (maybe mailing list?) around that, but don't see anything in this thread or the JIRA

mattrpav avatar Jan 05 '23 04:01 mattrpav

From a community standpoint, I don't see any issue with this PR:

  • we started to work on design with Etienne while ago, we shared a proposal on the mailing list
  • a document has been shared on the mailing list too with details
  • as this is a new feature/addition, with limited impacts on the existing behavior, I don't see any blocker to move forward.

Definitely, I'm supporter of this (that's why I already worked with the guys on design, and I will do a review pass): it makes sense to have this in addition of Spring 6, Jakarta, JMS 2, bookkeeper, ... new features we are planning for ActiveMQ.

I will do a new pass on this one then.

jbonofre avatar Jan 06 '23 06:01 jbonofre

On first pass, I have concerns that an approach needs this much internal changes to things like Queue.java and Topic.java. There may very well be needs to change internal APIs, but I think it would be best to separate those changes out as broker API/SPI change set in order to support different scenarios.

mattrpav avatar Jan 06 '23 14:01 mattrpav

A bit missing I'm struggling with how the acks back from a mirror is working reliably here where clients can be connect to either. I'm assuming you're trying to mimick Artemis here.

the replica is not meant to be used at the same time with primary. we are working on the changes to block connections on the relica side. the feature is designed as a hot-warm pair not a hot-hot one.

The other bit assuming jms2 is still a target for the classic activemq 5.x stream, is how this fits into the design of support shared subs in that roadmap. E.g. ensuring this doesn't trip that initiative / effort at all.

the plugin will need to be updated to support new features. right now I don't know how much work is needed. but when/if this change is merged, we are planning to support it, so if it starts blocking some feature releases, we will try to work through

NikitaShupletsov avatar Jan 09 '23 21:01 NikitaShupletsov

I haven't gone through much of this yet but the number 1 thing is anything here needs to be isolated and as Matt pointed out I have some concerns with internal changes. This needs to not impact anything in the broker if it's not running so any changes that would impact performance or could introduce other issues/bugs would be a problem. Essentially if it's disabled (which it will be by default) then the broker shouldn't behave any differently than today.

I also wonder whether or not this should be part of the core broker. At this point I'm not really convinced this should be included and may be better as a stand alone plugin that is maintained separately.

cshannon avatar Jan 10 '23 11:01 cshannon

I haven't gone through much of this yet but the number 1 thing is anything here needs to be isolated and as Matt pointed out I have some concerns with internal changes. This needs to not impact anything in the broker if it's not running so any changes that would impact performance or could introduce other issues/bugs would be a problem. Essentially if it's disabled (which it will be by default) then the broker shouldn't behave any differently than today.

i totally agree. We changed the internal classes just to add a couple methods(and modified one that hasn't been used since pure master-slave was deleted). Hence nothing should be impacted by the changes when the plugin is turned off.

NikitaShupletsov avatar Jan 10 '23 17:01 NikitaShupletsov

due to the concerns regarding the internal changes, I extracted them to a separate PR: https://github.com/apache/activemq/pull/953 so that it's easier to review. the description contains the explanation what and why was changes. please feel free to comment)

NikitaShupletsov avatar Jan 10 '23 20:01 NikitaShupletsov

I also wonder whether or not this should be part of the core broker. At this point I'm not really convinced this should be included and may be better as a stand alone plugin that is maintained separately.

This feature has broad value and we are seeing demand from lots of ActiveMQ customers. Similar to Artemis where it is part of the core broker, we see similar needs for ActiveMQ Classic. Having it as a part of the core broker will bring more visibility and support for the plugin for the broader community.

NikitaShupletsov avatar Jan 10 '23 22:01 NikitaShupletsov