[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1789 ---
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1781 ---
[GitHub] activemq-artemis pull request #1786: ARTEMIS-1616 OpenWire improvements
Github user michaelandrepearce commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1786#discussion_r162492929 --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java --- @@ -361,13 +371,68 @@ public int hashCode() { } } + private static SimpleString[] splitWithCachedString(final SimpleString simpleString, final int delim) { + final String str = simpleString.str; + final byte[] data = simpleString.data; + final int length = str.length(); + List all = null; + int index = 0; + while (index < length) { + final int delimIndex = str.indexOf(delim, index); + if (delimIndex == -1) { +//just need to add the last one +break; + } else { +all = addSimpleStringPart(all, data, index, delimIndex); + } + index = delimIndex + 1; + } + if (all == null) { + return new SimpleString[]{simpleString}; + } else { + // Adding the last one + all = addSimpleStringPart(all, data, index, length); + // Converting it to arrays + final SimpleString[] parts = new SimpleString[all.size()]; + return all.toArray(parts); + } + } + + private static List addSimpleStringPart(List all, + final byte[] data, + final int startIndex, + final int endIndex) { + final int expectedLength = endIndex - startIndex; + final SimpleString ss; + if (expectedLength == 0) { + ss = EMPTY; + } else { + //extract a byte[] copy from this + final int ssIndex = startIndex << 1; + final int delIndex = endIndex << 1; + final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex); + ss = new SimpleString(bytes); + } + // We will create the ArrayList lazily + if (all == null) { + // There will be at least 3 strings on this case (which is the actual common usecase) --- End diff -- as noted what are these 3 "magical" use cases, it be good to enumerate them. I'm still left wondering :) ---
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1781#discussion_r162491865 --- Diff: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java --- @@ -557,6 +592,25 @@ public Object run(ActionContext context) throws Exception { filters.put("${global-max-section}", readTextFile(ETC_GLOBAL_MAX_SPECIFIED_TXT, filters)); } + if (jdbc) { + if (jdbcURL == null) { +jdbcURL = "jdbc:derby:" + getInstance().getAbsolutePath() + "/data/derby/db;create=true"; --- End diff -- @jbertram I want to keep this as is.. with some default to be used. ---
Re: [DISCUSS] Adding Derby as a dependency
Actually, I will keep this as is.. .the user should decide what driver to use... I will keep configurations by default as derby (if the user doesn't provide anything else on the CLI).. but that's just for easy of use. On Thu, Jan 18, 2018 at 5:18 PM, Clebert Suconicwrote: > thanks.. it's all good... > > > Actually I think not having the dependency on the main lib was a good > outcome anyways... > > > I could make a further change.. to programmatically download derby if > the user allows it. > > On Thu, Jan 18, 2018 at 5:02 PM, Michael André Pearce > wrote: >> Just for the record I gave an opinion in a discussion, I have not given a -1. >> >> I’m not veto or blocking anything. >> >> In actual fact as I said I’m for this I think it’s good. >> >> just I think as a community we should have some clear guidelines agreed of >> what goes in and what stays out, and how some things are allowed to be added >> if remotely have their own lifecycle. At the moment it’s quite obvious it’s >> unclear. >> >> >> >> Sent from my iPhone >> >>> On 18 Jan 2018, at 21:31, Clebert Suconic wrote: >>> >>> so, after my last update, >>> >>> When you create a server with the --jdbc option >>> >>> example: >>> >>> >>> ./artemis create /tmp/myserver --jdbc >>> >>> >>> >>> the CLI will still help the user to configure the server... but >>> instead of having the dependency ready, it will give you the following >>> message: >>> >>> >>> >>> >>> >>> Copy a jar containing the JDBC Driver >>> 'org.apache.derby.jdbc.EmbeddedDriver' into >>> /work/apache/six/artemis-distribution/target/apache-artemis-2.5.0-SNAPSHOT-bin/apache-artemis-2.5.0-SNAPSHOT/bin/xxx/lib >>> >>> >>> >>> >>> >>> >>> >>> The example will download it through maven dependencies.. and install >>> it at the created server/lib. >>> >>> >>> On Thu, Jan 18, 2018 at 2:02 PM, Clebert Suconic >>> wrote: I will remove the dependency to avoid confusion. The database example will download the derby driver and install it on the lib. I will also add a message during the broker creation telling the user to copy the driver in the right place. > On Thu, Jan 18, 2018 at 1:44 PM Timothy Bish wrote: > > Agree with Robbie on this. My view from the previous discussion on the > Kafka bridge is that is would be a third party project that could be > used by anyone that wished to have it but that it does not need to be > (or should be) included with the broker either in the repo or the > distribution. Third party tools / plugins can live on their own and > provide their own documentation / examples for their use and > installation into the broker. I think we covered this in the past > discussion fairly well. > > As for the Derby bits being added I'm not entirely against it but I do > think that it would be rare for anyone to actually end up using it in > any sort of production type scenarios. Most folks that want to use JDBC > have existing DB instances that provides some feature that they feel > requires them to use a JDBC store and will use a JDBC driver from their > vendor for that. We don't include derby in the 5.x distribution > although we do use it for testing the JDBC store. If you do include > derby in the distribution you should probably include it in an optional > folder or the like in the 'lib' folder to make it clear to those > maintaining a broker installation who are averse to keeping unused bits > around that it can be deleted without an ill affects on the broker. > >> On 01/18/2018 12:47 PM, Robbie Gemmell wrote: >> I can't speak for others on the previous thread, but the below was not >> my position in that prior discussion at all. I don't think the >> proposed kafka bridge component should be bundled in the broker repo >> or distribution, regardless where else the code lives, but that it >> should instead have its own independent lifecyle and distribution. >> >> I think I already covered, at least in part, in my earlier mail on >> this thread why I actually see these as different situations. >> >> Robbie >> >> On 18 January 2018 at 17:06, Justin Bertram wrote: ...if I made the connector implementation code available in maven central >>> will have it hosted in GitHub the source, then we’d be happy to have >>> that >>> packaged in to provide the functionality? >>> >>> Obviously I can only speak for myself here, but that's how I understood >>> the >>> previous discussion. >>> >>> >>> Justin >>> >>> On
Re: [DISCUSS] Adding Derby as a dependency
thanks.. it's all good... Actually I think not having the dependency on the main lib was a good outcome anyways... I could make a further change.. to programmatically download derby if the user allows it. On Thu, Jan 18, 2018 at 5:02 PM, Michael André Pearcewrote: > Just for the record I gave an opinion in a discussion, I have not given a -1. > > I’m not veto or blocking anything. > > In actual fact as I said I’m for this I think it’s good. > > just I think as a community we should have some clear guidelines agreed of > what goes in and what stays out, and how some things are allowed to be added > if remotely have their own lifecycle. At the moment it’s quite obvious it’s > unclear. > > > > Sent from my iPhone > >> On 18 Jan 2018, at 21:31, Clebert Suconic wrote: >> >> so, after my last update, >> >> When you create a server with the --jdbc option >> >> example: >> >> >> ./artemis create /tmp/myserver --jdbc >> >> >> >> the CLI will still help the user to configure the server... but >> instead of having the dependency ready, it will give you the following >> message: >> >> >> >> >> >> Copy a jar containing the JDBC Driver >> 'org.apache.derby.jdbc.EmbeddedDriver' into >> /work/apache/six/artemis-distribution/target/apache-artemis-2.5.0-SNAPSHOT-bin/apache-artemis-2.5.0-SNAPSHOT/bin/xxx/lib >> >> >> >> >> >> >> >> The example will download it through maven dependencies.. and install >> it at the created server/lib. >> >> >> On Thu, Jan 18, 2018 at 2:02 PM, Clebert Suconic >> wrote: >>> I will remove the dependency to avoid confusion. >>> >>> The database example will download the derby driver and install it on the >>> lib. >>> >>> >>> I will also add a message during the broker creation telling the user to >>> copy the driver in the right place. >>> >>> On Thu, Jan 18, 2018 at 1:44 PM Timothy Bish wrote: Agree with Robbie on this. My view from the previous discussion on the Kafka bridge is that is would be a third party project that could be used by anyone that wished to have it but that it does not need to be (or should be) included with the broker either in the repo or the distribution. Third party tools / plugins can live on their own and provide their own documentation / examples for their use and installation into the broker. I think we covered this in the past discussion fairly well. As for the Derby bits being added I'm not entirely against it but I do think that it would be rare for anyone to actually end up using it in any sort of production type scenarios. Most folks that want to use JDBC have existing DB instances that provides some feature that they feel requires them to use a JDBC store and will use a JDBC driver from their vendor for that. We don't include derby in the 5.x distribution although we do use it for testing the JDBC store. If you do include derby in the distribution you should probably include it in an optional folder or the like in the 'lib' folder to make it clear to those maintaining a broker installation who are averse to keeping unused bits around that it can be deleted without an ill affects on the broker. > On 01/18/2018 12:47 PM, Robbie Gemmell wrote: > I can't speak for others on the previous thread, but the below was not > my position in that prior discussion at all. I don't think the > proposed kafka bridge component should be bundled in the broker repo > or distribution, regardless where else the code lives, but that it > should instead have its own independent lifecyle and distribution. > > I think I already covered, at least in part, in my earlier mail on > this thread why I actually see these as different situations. > > Robbie > > On 18 January 2018 at 17:06, Justin Bertram wrote: >>> ...if I made the connector implementation code available in maven >>> central >> will have it hosted in GitHub the source, then we’d be happy to have >> that >> packaged in to provide the functionality? >> >> Obviously I can only speak for myself here, but that's how I understood >> the >> previous discussion. >> >> >> Justin >> >> On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < >> michael.andre.pea...@me.com> wrote: >> >>> Ok so if I understood this if I made the connector implementation code >>> available in maven central will have it hosted in GitHub the source, >>> then >>> we’d be happy to have that packaged in to provide the functionality? >>> >>> If that’s the case im +1 with this, and then I’ll work on doing that
[GitHub] activemq-artemis pull request #1790: ARTEMIS-1606 - Change AddressInfo Routi...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1790 ---
[GitHub] activemq-artemis issue #1790: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1790 Looks like master is broken since merge of https://github.com/apache/activemq-artemis/pull/1742 thus your PR build failed, this fix though looks fine, i have run it manually offline :) , ill merge it. ---
Re: [DISCUSS] Adding Derby as a dependency
Just for the record I gave an opinion in a discussion, I have not given a -1. I’m not veto or blocking anything. In actual fact as I said I’m for this I think it’s good. just I think as a community we should have some clear guidelines agreed of what goes in and what stays out, and how some things are allowed to be added if remotely have their own lifecycle. At the moment it’s quite obvious it’s unclear. Sent from my iPhone > On 18 Jan 2018, at 21:31, Clebert Suconicwrote: > > so, after my last update, > > When you create a server with the --jdbc option > > example: > > > ./artemis create /tmp/myserver --jdbc > > > > the CLI will still help the user to configure the server... but > instead of having the dependency ready, it will give you the following > message: > > > > > > Copy a jar containing the JDBC Driver > 'org.apache.derby.jdbc.EmbeddedDriver' into > /work/apache/six/artemis-distribution/target/apache-artemis-2.5.0-SNAPSHOT-bin/apache-artemis-2.5.0-SNAPSHOT/bin/xxx/lib > > > > > > > > The example will download it through maven dependencies.. and install > it at the created server/lib. > > > On Thu, Jan 18, 2018 at 2:02 PM, Clebert Suconic > wrote: >> I will remove the dependency to avoid confusion. >> >> The database example will download the derby driver and install it on the >> lib. >> >> >> I will also add a message during the broker creation telling the user to >> copy the driver in the right place. >> >> >>> On Thu, Jan 18, 2018 at 1:44 PM Timothy Bish wrote: >>> >>> Agree with Robbie on this. My view from the previous discussion on the >>> Kafka bridge is that is would be a third party project that could be >>> used by anyone that wished to have it but that it does not need to be >>> (or should be) included with the broker either in the repo or the >>> distribution. Third party tools / plugins can live on their own and >>> provide their own documentation / examples for their use and >>> installation into the broker. I think we covered this in the past >>> discussion fairly well. >>> >>> As for the Derby bits being added I'm not entirely against it but I do >>> think that it would be rare for anyone to actually end up using it in >>> any sort of production type scenarios. Most folks that want to use JDBC >>> have existing DB instances that provides some feature that they feel >>> requires them to use a JDBC store and will use a JDBC driver from their >>> vendor for that. We don't include derby in the 5.x distribution >>> although we do use it for testing the JDBC store. If you do include >>> derby in the distribution you should probably include it in an optional >>> folder or the like in the 'lib' folder to make it clear to those >>> maintaining a broker installation who are averse to keeping unused bits >>> around that it can be deleted without an ill affects on the broker. >>> On 01/18/2018 12:47 PM, Robbie Gemmell wrote: I can't speak for others on the previous thread, but the below was not my position in that prior discussion at all. I don't think the proposed kafka bridge component should be bundled in the broker repo or distribution, regardless where else the code lives, but that it should instead have its own independent lifecyle and distribution. I think I already covered, at least in part, in my earlier mail on this thread why I actually see these as different situations. Robbie On 18 January 2018 at 17:06, Justin Bertram wrote: >> ...if I made the connector implementation code available in maven >> central > will have it hosted in GitHub the source, then we’d be happy to have > that > packaged in to provide the functionality? > > Obviously I can only speak for myself here, but that's how I understood > the > previous discussion. > > > Justin > > On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < > michael.andre.pea...@me.com> wrote: > >> Ok so if I understood this if I made the connector implementation code >> available in maven central will have it hosted in GitHub the source, >> then >> we’d be happy to have that packaged in to provide the functionality? >> >> If that’s the case im +1 with this, and then I’ll work on doing that >> for >> the Kafka piece then. >> >> Cheers >> Mike >> >> Sent from my iPhone >> >>> On 18 Jan 2018, at 17:47, Justin Bertram wrote: >>> >>> I just reviewed your PR, Clebert, and made a comment. In general I >>> think >>> it looks great. Nice work. >>> >>> I've been thinking about your concern, Michael,
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1781#discussion_r162479786 --- Diff: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java --- @@ -557,6 +592,25 @@ public Object run(ActionContext context) throws Exception { filters.put("${global-max-section}", readTextFile(ETC_GLOBAL_MAX_SPECIFIED_TXT, filters)); } + if (jdbc) { + if (jdbcURL == null) { +jdbcURL = "jdbc:derby:" + getInstance().getAbsolutePath() + "/data/derby/db;create=true"; --- End diff -- Nope.. .lets not complicate things even further? :) The config will be defaulted to Derby if no parameters.. then the user can install derby. it's a nice experience out of box. easy to install. easy to run. ---
Re: [DISCUSS] Rework NMS.AMQP
On 01/17/2018 03:50 PM, cmorgan wrote: Hi Everyone, Happy New Year. I have a progress update about the NMS AMQP API Rework. The work done in the fork, https://github.com/cjwmorgan-sol-sys/nms-amqp, is close to being finished. Duane and I have also signed the ICLA. I will follow what Tim suggested by creating an NMS JIRA under the AMQNET project (if I'm mistaken about the project please let me know). I will also be creating subtasks for some task as Tim suggested, eg cleaning out code tree, update building system, get automated unit tests running, commit new code base, etc. Although some input or guidance for other subtasks to migrate to an apache repository would be appreciated. I will create the JIRA tasks tomorrow unless there is reason not to. Let me know if there are any questions or concern. Chris Morgan - Chris Morgan -- Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html Thanks for letting us know that you've been working on this. You can create issues on the AMQNET Jira if you'd like. I've looked through the code a bit and it looks like a nice start, sadly I can't build it as I don't have a platform to run Visual Studio 2017 so I can't comment on building it or results of testing etc. Some things that you might want to do is document what features of the NMS API are or aren't implemented, I see that it currently doesn't support transactions for instance. Other tasks would be to provide proper license files and add Apache license headers to all the source and other relevant files in the project. -- Tim Bish twitter: @tabish121 blog: http://timbish.blogspot.com/
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user jbertram commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1781#discussion_r162473338 --- Diff: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java --- @@ -557,6 +592,25 @@ public Object run(ActionContext context) throws Exception { filters.put("${global-max-section}", readTextFile(ETC_GLOBAL_MAX_SPECIFIED_TXT, filters)); } + if (jdbc) { + if (jdbcURL == null) { +jdbcURL = "jdbc:derby:" + getInstance().getAbsolutePath() + "/data/derby/db;create=true"; --- End diff -- Should this be left null as well rather than defaulting to Derby? ---
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user jbertram commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1781#discussion_r162473145 --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/config/ActiveMQDefaultConfiguration.java --- @@ -423,8 +423,8 @@ public static String getDefaultHapolicyBackupStrategy() { // Default database url. Derby database is used by default. private static String DEFAULT_DATABASE_URL = null; - // Default JDBC Driver class name - private static String DEFAULT_JDBC_DRIVER_CLASS_NAME = null; + // Default JDBC Driver class name, derby by default just for demo purposes + private static String DEFAULT_JDBC_DRIVER_CLASS_NAME = "org.apache.derby.jdbc.EmbeddedDriver"; --- End diff -- Should this be null since we won't be packaging Derby? ---
[GitHub] activemq-artemis pull request #1781: ARTEMIS-1613 Integrating JDBC into CLI ...
Github user jbertram commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1781#discussion_r162472842 --- Diff: artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/database-store.txt --- @@ -0,0 +1,18 @@ + + + --- End diff -- Change this comment since we won't package Derby? ---
Re: [DISCUSS] Adding Derby as a dependency
so, after my last update, When you create a server with the --jdbc option example: ./artemis create /tmp/myserver --jdbc the CLI will still help the user to configure the server... but instead of having the dependency ready, it will give you the following message: Copy a jar containing the JDBC Driver 'org.apache.derby.jdbc.EmbeddedDriver' into /work/apache/six/artemis-distribution/target/apache-artemis-2.5.0-SNAPSHOT-bin/apache-artemis-2.5.0-SNAPSHOT/bin/xxx/lib The example will download it through maven dependencies.. and install it at the created server/lib. On Thu, Jan 18, 2018 at 2:02 PM, Clebert Suconicwrote: > I will remove the dependency to avoid confusion. > > The database example will download the derby driver and install it on the > lib. > > > I will also add a message during the broker creation telling the user to > copy the driver in the right place. > > > On Thu, Jan 18, 2018 at 1:44 PM Timothy Bish wrote: >> >> Agree with Robbie on this. My view from the previous discussion on the >> Kafka bridge is that is would be a third party project that could be >> used by anyone that wished to have it but that it does not need to be >> (or should be) included with the broker either in the repo or the >> distribution. Third party tools / plugins can live on their own and >> provide their own documentation / examples for their use and >> installation into the broker. I think we covered this in the past >> discussion fairly well. >> >> As for the Derby bits being added I'm not entirely against it but I do >> think that it would be rare for anyone to actually end up using it in >> any sort of production type scenarios. Most folks that want to use JDBC >> have existing DB instances that provides some feature that they feel >> requires them to use a JDBC store and will use a JDBC driver from their >> vendor for that. We don't include derby in the 5.x distribution >> although we do use it for testing the JDBC store. If you do include >> derby in the distribution you should probably include it in an optional >> folder or the like in the 'lib' folder to make it clear to those >> maintaining a broker installation who are averse to keeping unused bits >> around that it can be deleted without an ill affects on the broker. >> >> On 01/18/2018 12:47 PM, Robbie Gemmell wrote: >> > I can't speak for others on the previous thread, but the below was not >> > my position in that prior discussion at all. I don't think the >> > proposed kafka bridge component should be bundled in the broker repo >> > or distribution, regardless where else the code lives, but that it >> > should instead have its own independent lifecyle and distribution. >> > >> > I think I already covered, at least in part, in my earlier mail on >> > this thread why I actually see these as different situations. >> > >> > Robbie >> > >> > On 18 January 2018 at 17:06, Justin Bertram wrote: >> >>> ...if I made the connector implementation code available in maven >> >>> central >> >> will have it hosted in GitHub the source, then we’d be happy to have >> >> that >> >> packaged in to provide the functionality? >> >> >> >> Obviously I can only speak for myself here, but that's how I understood >> >> the >> >> previous discussion. >> >> >> >> >> >> Justin >> >> >> >> On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < >> >> michael.andre.pea...@me.com> wrote: >> >> >> >>> Ok so if I understood this if I made the connector implementation code >> >>> available in maven central will have it hosted in GitHub the source, >> >>> then >> >>> we’d be happy to have that packaged in to provide the functionality? >> >>> >> >>> If that’s the case im +1 with this, and then I’ll work on doing that >> >>> for >> >>> the Kafka piece then. >> >>> >> >>> Cheers >> >>> Mike >> >>> >> >>> Sent from my iPhone >> >>> >> On 18 Jan 2018, at 17:47, Justin Bertram wrote: >> >> I just reviewed your PR, Clebert, and made a comment. In general I >> think >> it looks great. Nice work. >> >> I've been thinking about your concern, Michael, that the rules on >> integrations like this aren't clear. I went back and reviewed the >> >>> mailing >> list discussion and the discussion on your PR for the Kafka >> integration >> using the ConnectorService. As far as I can tell, the main issue >> with >> >>> your >> PR was that many didn't want to house the actual >> implementation-specific >> integration code within the Artemis project. It seems to me that >> this >> "rule" is being applied consistently in this case as well, namely >> that >> Artemis is providing an integration point (i.e. the JDBC layer) but >> >>>
[GitHub] activemq-artemis issue #1790: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1790 @michaelandrepearce The past PR has broken just this test: this is fixing it :+1: ---
[GitHub] activemq-artemis pull request #1790: ARTEMIS-1606 - Change AddressInfo Routi...
GitHub user franz1981 opened a pull request: https://github.com/apache/activemq-artemis/pull/1790 ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet This is fixing the broken testUnsupportedRoutingType test You can merge this pull request into a Git repository by running: $ git pull https://github.com/franz1981/activemq-artemis fixed_routing_type_test Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/1790.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 #1790 commit 48604155d21719ce204f0d31018b781140c03beb Author: Francesco NigroDate: 2018-01-18T19:54:53Z ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet This is fixing the broken testUnsupportedRoutingType test ---
Re: [DISCUSS] Adding Derby as a dependency
I will remove the dependency to avoid confusion. The database example will download the derby driver and install it on the lib. I will also add a message during the broker creation telling the user to copy the driver in the right place. On Thu, Jan 18, 2018 at 1:44 PM Timothy Bishwrote: > Agree with Robbie on this. My view from the previous discussion on the > Kafka bridge is that is would be a third party project that could be > used by anyone that wished to have it but that it does not need to be > (or should be) included with the broker either in the repo or the > distribution. Third party tools / plugins can live on their own and > provide their own documentation / examples for their use and > installation into the broker. I think we covered this in the past > discussion fairly well. > > As for the Derby bits being added I'm not entirely against it but I do > think that it would be rare for anyone to actually end up using it in > any sort of production type scenarios. Most folks that want to use JDBC > have existing DB instances that provides some feature that they feel > requires them to use a JDBC store and will use a JDBC driver from their > vendor for that. We don't include derby in the 5.x distribution > although we do use it for testing the JDBC store. If you do include > derby in the distribution you should probably include it in an optional > folder or the like in the 'lib' folder to make it clear to those > maintaining a broker installation who are averse to keeping unused bits > around that it can be deleted without an ill affects on the broker. > > On 01/18/2018 12:47 PM, Robbie Gemmell wrote: > > I can't speak for others on the previous thread, but the below was not > > my position in that prior discussion at all. I don't think the > > proposed kafka bridge component should be bundled in the broker repo > > or distribution, regardless where else the code lives, but that it > > should instead have its own independent lifecyle and distribution. > > > > I think I already covered, at least in part, in my earlier mail on > > this thread why I actually see these as different situations. > > > > Robbie > > > > On 18 January 2018 at 17:06, Justin Bertram wrote: > >>> ...if I made the connector implementation code available in maven > central > >> will have it hosted in GitHub the source, then we’d be happy to have > that > >> packaged in to provide the functionality? > >> > >> Obviously I can only speak for myself here, but that's how I understood > the > >> previous discussion. > >> > >> > >> Justin > >> > >> On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < > >> michael.andre.pea...@me.com> wrote: > >> > >>> Ok so if I understood this if I made the connector implementation code > >>> available in maven central will have it hosted in GitHub the source, > then > >>> we’d be happy to have that packaged in to provide the functionality? > >>> > >>> If that’s the case im +1 with this, and then I’ll work on doing that > for > >>> the Kafka piece then. > >>> > >>> Cheers > >>> Mike > >>> > >>> Sent from my iPhone > >>> > On 18 Jan 2018, at 17:47, Justin Bertram wrote: > > I just reviewed your PR, Clebert, and made a comment. In general I > think > it looks great. Nice work. > > I've been thinking about your concern, Michael, that the rules on > integrations like this aren't clear. I went back and reviewed the > >>> mailing > list discussion and the discussion on your PR for the Kafka > integration > using the ConnectorService. As far as I can tell, the main issue with > >>> your > PR was that many didn't want to house the actual > implementation-specific > integration code within the Artemis project. It seems to me that this > "rule" is being applied consistently in this case as well, namely that > Artemis is providing an integration point (i.e. the JDBC layer) but > >>> doesn't > house implementation-specific code (i.e. Derby). > > The consensus in our previous discussion was that > implementation-specific > integration code (e.g. your Kafka connector) should live outside the > >>> broker > code-base as a separate component with its own release cycle. This is > exactly how the integration with Derby is working. Derby lives > outside > >>> the > broker code-base with its own release cycle and is being pulled in via > Maven. If your Kafka connector was available via Maven and we wanted > to > create a broker example that used it I don't think that would be an > >>> issue. > To be clear, Derby is being packaged with the broker to serve as a > >>> default > JDBC implementation, but I don't think it would be an issue to package > >>> the > Kafka connector with the broker if there was a similar, real need. > > I don't see any contradictions or inequities here. I'd like to > confirm
Re: [DISCUSS] Adding Derby as a dependency
Agree with Robbie on this. My view from the previous discussion on the Kafka bridge is that is would be a third party project that could be used by anyone that wished to have it but that it does not need to be (or should be) included with the broker either in the repo or the distribution. Third party tools / plugins can live on their own and provide their own documentation / examples for their use and installation into the broker. I think we covered this in the past discussion fairly well. As for the Derby bits being added I'm not entirely against it but I do think that it would be rare for anyone to actually end up using it in any sort of production type scenarios. Most folks that want to use JDBC have existing DB instances that provides some feature that they feel requires them to use a JDBC store and will use a JDBC driver from their vendor for that. We don't include derby in the 5.x distribution although we do use it for testing the JDBC store. If you do include derby in the distribution you should probably include it in an optional folder or the like in the 'lib' folder to make it clear to those maintaining a broker installation who are averse to keeping unused bits around that it can be deleted without an ill affects on the broker. On 01/18/2018 12:47 PM, Robbie Gemmell wrote: I can't speak for others on the previous thread, but the below was not my position in that prior discussion at all. I don't think the proposed kafka bridge component should be bundled in the broker repo or distribution, regardless where else the code lives, but that it should instead have its own independent lifecyle and distribution. I think I already covered, at least in part, in my earlier mail on this thread why I actually see these as different situations. Robbie On 18 January 2018 at 17:06, Justin Bertramwrote: ...if I made the connector implementation code available in maven central will have it hosted in GitHub the source, then we’d be happy to have that packaged in to provide the functionality? Obviously I can only speak for myself here, but that's how I understood the previous discussion. Justin On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < michael.andre.pea...@me.com> wrote: Ok so if I understood this if I made the connector implementation code available in maven central will have it hosted in GitHub the source, then we’d be happy to have that packaged in to provide the functionality? If that’s the case im +1 with this, and then I’ll work on doing that for the Kafka piece then. Cheers Mike Sent from my iPhone On 18 Jan 2018, at 17:47, Justin Bertram wrote: I just reviewed your PR, Clebert, and made a comment. In general I think it looks great. Nice work. I've been thinking about your concern, Michael, that the rules on integrations like this aren't clear. I went back and reviewed the mailing list discussion and the discussion on your PR for the Kafka integration using the ConnectorService. As far as I can tell, the main issue with your PR was that many didn't want to house the actual implementation-specific integration code within the Artemis project. It seems to me that this "rule" is being applied consistently in this case as well, namely that Artemis is providing an integration point (i.e. the JDBC layer) but doesn't house implementation-specific code (i.e. Derby). The consensus in our previous discussion was that implementation-specific integration code (e.g. your Kafka connector) should live outside the broker code-base as a separate component with its own release cycle. This is exactly how the integration with Derby is working. Derby lives outside the broker code-base with its own release cycle and is being pulled in via Maven. If your Kafka connector was available via Maven and we wanted to create a broker example that used it I don't think that would be an issue. To be clear, Derby is being packaged with the broker to serve as a default JDBC implementation, but I don't think it would be an issue to package the Kafka connector with the broker if there was a similar, real need. I don't see any contradictions or inequities here. I'd like to confirm a +1 from you before I merge. Let me know what you think. Justin On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic < clebert.suco...@gmail.com> wrote: I am almost done with this.. I should be able to send a PR tomorrow.. I think it's looking nice. On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic wrote: @Martyn: That's exactly what I'm planning.. Having the embedded Derby.. just for out of box testing. someone would do ./artemis create --jdbc ./server-place and we would have artemis running with Derby right there. Now my question here was... where do we change to add stuff into lib. I changed dep.xml but it's not picking it up. On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor
Re: [DISCUSS] Adding Derby as a dependency
On Thu, Jan 18, 2018 at 1:12 PM, Arthur Naseefwrote: > On the providing core persistence - that is core to Artemis, and Artemis > provides a reasonable solution to that problem out-of-the-box. There are > dozens, if not hundreds, or alternatives we could add as near-storage. > There needs to be a clear line on that front. Not sure I understand why > anyone wants JDBC access to their messaging solution internal storage in my > not-so-humble opinion. They (JDBC and messaging) serve two very different > patterns and problem-sets. JDBC is already a feature.. I'm just providing easier configuration out of the box with a simple CLI tool. We are not discussin if we should have JDBC or not. We had to add because users *wanted it*. No other reason. as they say.. the "customer" is always right :)
[GitHub] activemq-artemis pull request #1742: ARTEMIS-1570 Flush appendExecutor befor...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1742 ---
Re: [DISCUSS] Adding Derby as a dependency
If I'm understanding the discussion well enough (apologies, TLDR) - one concern with adding this directly into core Artemis is ending up with too much complexity baked into the main product. Also, Mixing Concerns (i.e. reduced "separation of concerns"). Having a downstream project that integrates the two seems like a good way to reach the end goal. Yes, that means that downstream project may not always be kept up-to-date Would that really be a concern? On the providing core persistence - that is core to Artemis, and Artemis provides a reasonable solution to that problem out-of-the-box. There are dozens, if not hundreds, or alternatives we could add as near-storage. There needs to be a clear line on that front. Not sure I understand why anyone wants JDBC access to their messaging solution internal storage in my not-so-humble opinion. They (JDBC and messaging) serve two very different patterns and problem-sets. My 2 cents. HTH Art On Thu, Jan 18, 2018 at 10:47 AM, Robbie Gemmellwrote: > I can't speak for others on the previous thread, but the below was not > my position in that prior discussion at all. I don't think the > proposed kafka bridge component should be bundled in the broker repo > or distribution, regardless where else the code lives, but that it > should instead have its own independent lifecyle and distribution. > > I think I already covered, at least in part, in my earlier mail on > this thread why I actually see these as different situations. > > Robbie > > On 18 January 2018 at 17:06, Justin Bertram wrote: > >> ...if I made the connector implementation code available in maven > central > > will have it hosted in GitHub the source, then we’d be happy to have that > > packaged in to provide the functionality? > > > > Obviously I can only speak for myself here, but that's how I understood > the > > previous discussion. > > > > > > Justin > > > > On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < > > michael.andre.pea...@me.com> wrote: > > > >> Ok so if I understood this if I made the connector implementation code > >> available in maven central will have it hosted in GitHub the source, > then > >> we’d be happy to have that packaged in to provide the functionality? > >> > >> If that’s the case im +1 with this, and then I’ll work on doing that for > >> the Kafka piece then. > >> > >> Cheers > >> Mike > >> > >> Sent from my iPhone > >> > >> > On 18 Jan 2018, at 17:47, Justin Bertram wrote: > >> > > >> > I just reviewed your PR, Clebert, and made a comment. In general I > think > >> > it looks great. Nice work. > >> > > >> > I've been thinking about your concern, Michael, that the rules on > >> > integrations like this aren't clear. I went back and reviewed the > >> mailing > >> > list discussion and the discussion on your PR for the Kafka > integration > >> > using the ConnectorService. As far as I can tell, the main issue with > >> your > >> > PR was that many didn't want to house the actual > implementation-specific > >> > integration code within the Artemis project. It seems to me that this > >> > "rule" is being applied consistently in this case as well, namely that > >> > Artemis is providing an integration point (i.e. the JDBC layer) but > >> doesn't > >> > house implementation-specific code (i.e. Derby). > >> > > >> > The consensus in our previous discussion was that > implementation-specific > >> > integration code (e.g. your Kafka connector) should live outside the > >> broker > >> > code-base as a separate component with its own release cycle. This is > >> > exactly how the integration with Derby is working. Derby lives > outside > >> the > >> > broker code-base with its own release cycle and is being pulled in via > >> > Maven. If your Kafka connector was available via Maven and we wanted > to > >> > create a broker example that used it I don't think that would be an > >> issue. > >> > To be clear, Derby is being packaged with the broker to serve as a > >> default > >> > JDBC implementation, but I don't think it would be an issue to package > >> the > >> > Kafka connector with the broker if there was a similar, real need. > >> > > >> > I don't see any contradictions or inequities here. I'd like to > confirm a > >> > +1 from you before I merge. Let me know what you think. > >> > > >> > > >> > Justin > >> > > >> > On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic < > >> clebert.suco...@gmail.com> > >> > wrote: > >> > > >> >> I am almost done with this.. I should be able to send a PR tomorrow.. > >> >> I think it's looking nice. > >> >> > >> >> On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic > >> >> wrote: > >> >>> @Martyn: That's exactly what I'm planning.. Having the embedded > >> >>> Derby.. just for out of box testing. > >> >>> > >> >>> > >> >>> someone would do ./artemis create --jdbc ./server-place > >> >>> > >> >>> and we would have artemis running
Re: [DISCUSS] Adding Derby as a dependency
Clebert, I was looking for a +1 from Mike. My understanding is that a -1 from a committer (or is it just PMC?) is a veto [1], and I felt that was Mike's position. I think your PR is a good one and I'd like to merge it ASAP. Anybody feel free to correct me if I'm wrong on the veto thing. Justin [1] https://www.apache.org/foundation/voting.html#votes-on-code-modification On Thu, Jan 18, 2018 at 11:47 AM, Robbie Gemmellwrote: > I can't speak for others on the previous thread, but the below was not > my position in that prior discussion at all. I don't think the > proposed kafka bridge component should be bundled in the broker repo > or distribution, regardless where else the code lives, but that it > should instead have its own independent lifecyle and distribution. > > I think I already covered, at least in part, in my earlier mail on > this thread why I actually see these as different situations. > > Robbie > > On 18 January 2018 at 17:06, Justin Bertram wrote: > >> ...if I made the connector implementation code available in maven > central > > will have it hosted in GitHub the source, then we’d be happy to have that > > packaged in to provide the functionality? > > > > Obviously I can only speak for myself here, but that's how I understood > the > > previous discussion. > > > > > > Justin > > > > On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < > > michael.andre.pea...@me.com> wrote: > > > >> Ok so if I understood this if I made the connector implementation code > >> available in maven central will have it hosted in GitHub the source, > then > >> we’d be happy to have that packaged in to provide the functionality? > >> > >> If that’s the case im +1 with this, and then I’ll work on doing that for > >> the Kafka piece then. > >> > >> Cheers > >> Mike > >> > >> Sent from my iPhone > >> > >> > On 18 Jan 2018, at 17:47, Justin Bertram wrote: > >> > > >> > I just reviewed your PR, Clebert, and made a comment. In general I > think > >> > it looks great. Nice work. > >> > > >> > I've been thinking about your concern, Michael, that the rules on > >> > integrations like this aren't clear. I went back and reviewed the > >> mailing > >> > list discussion and the discussion on your PR for the Kafka > integration > >> > using the ConnectorService. As far as I can tell, the main issue with > >> your > >> > PR was that many didn't want to house the actual > implementation-specific > >> > integration code within the Artemis project. It seems to me that this > >> > "rule" is being applied consistently in this case as well, namely that > >> > Artemis is providing an integration point (i.e. the JDBC layer) but > >> doesn't > >> > house implementation-specific code (i.e. Derby). > >> > > >> > The consensus in our previous discussion was that > implementation-specific > >> > integration code (e.g. your Kafka connector) should live outside the > >> broker > >> > code-base as a separate component with its own release cycle. This is > >> > exactly how the integration with Derby is working. Derby lives > outside > >> the > >> > broker code-base with its own release cycle and is being pulled in via > >> > Maven. If your Kafka connector was available via Maven and we wanted > to > >> > create a broker example that used it I don't think that would be an > >> issue. > >> > To be clear, Derby is being packaged with the broker to serve as a > >> default > >> > JDBC implementation, but I don't think it would be an issue to package > >> the > >> > Kafka connector with the broker if there was a similar, real need. > >> > > >> > I don't see any contradictions or inequities here. I'd like to > confirm a > >> > +1 from you before I merge. Let me know what you think. > >> > > >> > > >> > Justin > >> > > >> > On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic < > >> clebert.suco...@gmail.com> > >> > wrote: > >> > > >> >> I am almost done with this.. I should be able to send a PR tomorrow.. > >> >> I think it's looking nice. > >> >> > >> >> On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic > >> >> wrote: > >> >>> @Martyn: That's exactly what I'm planning.. Having the embedded > >> >>> Derby.. just for out of box testing. > >> >>> > >> >>> > >> >>> someone would do ./artemis create --jdbc ./server-place > >> >>> > >> >>> and we would have artemis running with Derby right there. > >> >>> > >> >>> > >> >>> > >> >>> Now my question here was... where do we change to add stuff into > lib. > >> >>> I changed dep.xml but it's not picking it up. > >> >>> > >> >>> > >> >>> > >> >>> > >> >>> On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor > >> >> wrote: > >> Michael, > >> > >> I think all Clebert is suggesting here is to have something that > works > >> >> out > >> the box to demonstrate the JDBC store. Derby is a good candidate > as > >> it > >> >> can > >> work in memory, and we it in a
[GitHub] activemq-artemis pull request #1742: ARTEMIS-1570 Flush appendExecutor befor...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1742#discussion_r162421523 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/SharedNothingReplicationTest.java --- @@ -0,0 +1,388 @@ +package org.apache.activemq.artemis.tests.integration.replication; + + +import org.apache.activemq.artemis.api.core.ActiveMQBuffer; +import org.apache.activemq.artemis.api.core.ActiveMQException; +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.api.core.RoutingType; +import org.apache.activemq.artemis.api.core.client.*; +import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; +import org.apache.activemq.artemis.core.config.ClusterConnectionConfiguration; +import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.config.ha.ReplicaPolicyConfiguration; +import org.apache.activemq.artemis.core.config.ha.ReplicatedPolicyConfiguration; +import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl; +import org.apache.activemq.artemis.core.io.SequentialFileFactory; +import org.apache.activemq.artemis.core.io.mapped.MappedSequentialFileFactory; +import org.apache.activemq.artemis.core.journal.LoaderCallback; +import org.apache.activemq.artemis.core.journal.PreparedTransactionInfo; +import org.apache.activemq.artemis.core.journal.RecordInfo; +import org.apache.activemq.artemis.core.journal.impl.JournalImpl; +import org.apache.activemq.artemis.core.message.impl.CoreMessage; +import org.apache.activemq.artemis.core.message.impl.CoreMessagePersister; +import org.apache.activemq.artemis.core.persistence.Persister; +import org.apache.activemq.artemis.core.persistence.impl.journal.JournalRecordIds; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.ActiveMQServers; +import org.apache.activemq.artemis.core.server.JournalType; +import org.apache.activemq.artemis.junit.Wait; +import org.jboss.logging.Logger; +import org.junit.*; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +public class SharedNothingReplicationTest { +private static final Logger logger = Logger.getLogger(SharedNothingReplicationTest.class); + +@Rule +public TemporaryFolder brokersFolder = new TemporaryFolder(); + +private SlowMessagePersister slowMessagePersister; + +@Before +public void setUp() throws Exception { +slowMessagePersister = new SlowMessagePersister(); +CoreMessagePersister.theInstance = slowMessagePersister; +} + +@After +public void tearDown() throws Exception { +if (slowMessagePersister != null) { +CoreMessagePersister.theInstance = slowMessagePersister.persister; +} +} + +@Test +public void testReplicateFromSlowLive() throws Exception { +// start live +Configuration liveConfiguration = createLiveConfiguration(); +ActiveMQServer liveServer = ActiveMQServers.newActiveMQServer(liveConfiguration); +liveServer.start(); + +Wait.waitFor(() -> liveServer.isStarted()); + +CoreMessagePersister.theInstance = SlowMessagePersister._getInstance(); + +final CountDownLatch replicated = new CountDownLatch(1); + +ServerLocator locator = ServerLocatorImpl.newLocator("tcp://localhost:61616"); +locator.setCallTimeout(60_000L); +locator.setConnectionTTL(60_000L); +locator.addClusterTopologyListener(new ClusterTopologyListener() { +@Override +public void nodeUP(TopologyMember member, boolean last) { +logger.infof("nodeUP fired last=%s, live=%s, backup=%s", last, member.getLive(), member.getBackup()); +if (member.getBackup() != null) { +replicated.countDown(); +} +} + +@Override +public void nodeDown(long eventUID, String nodeID) { + +} +}); + +final ClientSessionFactory csf = locator.createSessionFactory(); +ClientSession sess = csf.createSession(); +sess.createQueue("slow", RoutingType.ANYCAST, "slow", true); +sess.close(); +Executor
[GitHub] activemq-artemis issue #1742: ARTEMIS-1570 Flush appendExecutor before take ...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/1742 do you want to contribute more? I can help you in any way you need. ---
[GitHub] activemq-artemis issue #1742: ARTEMIS-1570 Flush appendExecutor before take ...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/1742 nice job on this one!!! impressed!!! ---
Re: [DISCUSS] Adding Derby as a dependency
I can't speak for others on the previous thread, but the below was not my position in that prior discussion at all. I don't think the proposed kafka bridge component should be bundled in the broker repo or distribution, regardless where else the code lives, but that it should instead have its own independent lifecyle and distribution. I think I already covered, at least in part, in my earlier mail on this thread why I actually see these as different situations. Robbie On 18 January 2018 at 17:06, Justin Bertramwrote: >> ...if I made the connector implementation code available in maven central > will have it hosted in GitHub the source, then we’d be happy to have that > packaged in to provide the functionality? > > Obviously I can only speak for myself here, but that's how I understood the > previous discussion. > > > Justin > > On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < > michael.andre.pea...@me.com> wrote: > >> Ok so if I understood this if I made the connector implementation code >> available in maven central will have it hosted in GitHub the source, then >> we’d be happy to have that packaged in to provide the functionality? >> >> If that’s the case im +1 with this, and then I’ll work on doing that for >> the Kafka piece then. >> >> Cheers >> Mike >> >> Sent from my iPhone >> >> > On 18 Jan 2018, at 17:47, Justin Bertram wrote: >> > >> > I just reviewed your PR, Clebert, and made a comment. In general I think >> > it looks great. Nice work. >> > >> > I've been thinking about your concern, Michael, that the rules on >> > integrations like this aren't clear. I went back and reviewed the >> mailing >> > list discussion and the discussion on your PR for the Kafka integration >> > using the ConnectorService. As far as I can tell, the main issue with >> your >> > PR was that many didn't want to house the actual implementation-specific >> > integration code within the Artemis project. It seems to me that this >> > "rule" is being applied consistently in this case as well, namely that >> > Artemis is providing an integration point (i.e. the JDBC layer) but >> doesn't >> > house implementation-specific code (i.e. Derby). >> > >> > The consensus in our previous discussion was that implementation-specific >> > integration code (e.g. your Kafka connector) should live outside the >> broker >> > code-base as a separate component with its own release cycle. This is >> > exactly how the integration with Derby is working. Derby lives outside >> the >> > broker code-base with its own release cycle and is being pulled in via >> > Maven. If your Kafka connector was available via Maven and we wanted to >> > create a broker example that used it I don't think that would be an >> issue. >> > To be clear, Derby is being packaged with the broker to serve as a >> default >> > JDBC implementation, but I don't think it would be an issue to package >> the >> > Kafka connector with the broker if there was a similar, real need. >> > >> > I don't see any contradictions or inequities here. I'd like to confirm a >> > +1 from you before I merge. Let me know what you think. >> > >> > >> > Justin >> > >> > On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic < >> clebert.suco...@gmail.com> >> > wrote: >> > >> >> I am almost done with this.. I should be able to send a PR tomorrow.. >> >> I think it's looking nice. >> >> >> >> On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic >> >> wrote: >> >>> @Martyn: That's exactly what I'm planning.. Having the embedded >> >>> Derby.. just for out of box testing. >> >>> >> >>> >> >>> someone would do ./artemis create --jdbc ./server-place >> >>> >> >>> and we would have artemis running with Derby right there. >> >>> >> >>> >> >>> >> >>> Now my question here was... where do we change to add stuff into lib. >> >>> I changed dep.xml but it's not picking it up. >> >>> >> >>> >> >>> >> >>> >> >>> On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor >> >> wrote: >> Michael, >> >> I think all Clebert is suggesting here is to have something that works >> >> out >> the box to demonstrate the JDBC store. Derby is a good candidate as >> it >> >> can >> work in memory, and we it in a lot in our tests. I've actually not >> >> talked >> to Clebert about this, so he can confirm/deny if this was his intent, >> >> but I >> don't see this being related to maintaining a connector service >> implementation? The only Derby specific thing here would be to ship >> the >> lib as part of the distro and to configure a JDBC URL. I guess we >> could >> ask for a JDBC URL as part of the prompt, and tell the user to drop >> the >> >> lib >> on the class path, but having a quick and easy way to set up and test >> Artemis + JDBC sounds like a UX win to me. >> >> Cheers >> >> >> >> >> >> >> >> On Mon, Jan 15, 2018
[GitHub] activemq-artemis pull request #1742: ARTEMIS-1570 Flush appendExecutor befor...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1742#discussion_r162418130 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/SharedNothingReplicationTest.java --- @@ -0,0 +1,388 @@ +package org.apache.activemq.artemis.tests.integration.replication; + + +import org.apache.activemq.artemis.api.core.ActiveMQBuffer; +import org.apache.activemq.artemis.api.core.ActiveMQException; +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.api.core.RoutingType; +import org.apache.activemq.artemis.api.core.client.*; +import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; +import org.apache.activemq.artemis.core.config.ClusterConnectionConfiguration; +import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.config.ha.ReplicaPolicyConfiguration; +import org.apache.activemq.artemis.core.config.ha.ReplicatedPolicyConfiguration; +import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl; +import org.apache.activemq.artemis.core.io.SequentialFileFactory; +import org.apache.activemq.artemis.core.io.mapped.MappedSequentialFileFactory; +import org.apache.activemq.artemis.core.journal.LoaderCallback; +import org.apache.activemq.artemis.core.journal.PreparedTransactionInfo; +import org.apache.activemq.artemis.core.journal.RecordInfo; +import org.apache.activemq.artemis.core.journal.impl.JournalImpl; +import org.apache.activemq.artemis.core.message.impl.CoreMessage; +import org.apache.activemq.artemis.core.message.impl.CoreMessagePersister; +import org.apache.activemq.artemis.core.persistence.Persister; +import org.apache.activemq.artemis.core.persistence.impl.journal.JournalRecordIds; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.ActiveMQServers; +import org.apache.activemq.artemis.core.server.JournalType; +import org.apache.activemq.artemis.junit.Wait; +import org.jboss.logging.Logger; +import org.junit.*; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +public class SharedNothingReplicationTest { +private static final Logger logger = Logger.getLogger(SharedNothingReplicationTest.class); + +@Rule +public TemporaryFolder brokersFolder = new TemporaryFolder(); + +private SlowMessagePersister slowMessagePersister; + +@Before +public void setUp() throws Exception { +slowMessagePersister = new SlowMessagePersister(); +CoreMessagePersister.theInstance = slowMessagePersister; +} + +@After +public void tearDown() throws Exception { +if (slowMessagePersister != null) { +CoreMessagePersister.theInstance = slowMessagePersister.persister; +} +} + +@Test +public void testReplicateFromSlowLive() throws Exception { +// start live +Configuration liveConfiguration = createLiveConfiguration(); +ActiveMQServer liveServer = ActiveMQServers.newActiveMQServer(liveConfiguration); +liveServer.start(); + +Wait.waitFor(() -> liveServer.isStarted()); + +CoreMessagePersister.theInstance = SlowMessagePersister._getInstance(); + +final CountDownLatch replicated = new CountDownLatch(1); + +ServerLocator locator = ServerLocatorImpl.newLocator("tcp://localhost:61616"); +locator.setCallTimeout(60_000L); +locator.setConnectionTTL(60_000L); +locator.addClusterTopologyListener(new ClusterTopologyListener() { +@Override +public void nodeUP(TopologyMember member, boolean last) { +logger.infof("nodeUP fired last=%s, live=%s, backup=%s", last, member.getLive(), member.getBackup()); +if (member.getBackup() != null) { +replicated.countDown(); +} +} + +@Override +public void nodeDown(long eventUID, String nodeID) { + +} +}); + +final ClientSessionFactory csf = locator.createSessionFactory(); +ClientSession sess = csf.createSession(); +sess.createQueue("slow", RoutingType.ANYCAST, "slow", true); +sess.close(); +Executor
Re: [DISCUSS] Adding Derby as a dependency
On Thu, Jan 18, 2018 at 11:47 AM, Justin Bertramwrote: > I don't see any contradictions or inequities here. I'd like to confirm a > +1 from you before I merge. Let me know what you think. I'm not sure if you asked a +1 from me or Michael.. but that's exactly what I think... we could still consume Michael's component somewhere. The issue was about the release cycle. I even offered to have it depending on a example.. showing how it could be integrated.
Re: [DISCUSS] Adding Derby as a dependency
> ...if I made the connector implementation code available in maven central will have it hosted in GitHub the source, then we’d be happy to have that packaged in to provide the functionality? Obviously I can only speak for myself here, but that's how I understood the previous discussion. Justin On Thu, Jan 18, 2018 at 10:53 AM, Michael André Pearce < michael.andre.pea...@me.com> wrote: > Ok so if I understood this if I made the connector implementation code > available in maven central will have it hosted in GitHub the source, then > we’d be happy to have that packaged in to provide the functionality? > > If that’s the case im +1 with this, and then I’ll work on doing that for > the Kafka piece then. > > Cheers > Mike > > Sent from my iPhone > > > On 18 Jan 2018, at 17:47, Justin Bertramwrote: > > > > I just reviewed your PR, Clebert, and made a comment. In general I think > > it looks great. Nice work. > > > > I've been thinking about your concern, Michael, that the rules on > > integrations like this aren't clear. I went back and reviewed the > mailing > > list discussion and the discussion on your PR for the Kafka integration > > using the ConnectorService. As far as I can tell, the main issue with > your > > PR was that many didn't want to house the actual implementation-specific > > integration code within the Artemis project. It seems to me that this > > "rule" is being applied consistently in this case as well, namely that > > Artemis is providing an integration point (i.e. the JDBC layer) but > doesn't > > house implementation-specific code (i.e. Derby). > > > > The consensus in our previous discussion was that implementation-specific > > integration code (e.g. your Kafka connector) should live outside the > broker > > code-base as a separate component with its own release cycle. This is > > exactly how the integration with Derby is working. Derby lives outside > the > > broker code-base with its own release cycle and is being pulled in via > > Maven. If your Kafka connector was available via Maven and we wanted to > > create a broker example that used it I don't think that would be an > issue. > > To be clear, Derby is being packaged with the broker to serve as a > default > > JDBC implementation, but I don't think it would be an issue to package > the > > Kafka connector with the broker if there was a similar, real need. > > > > I don't see any contradictions or inequities here. I'd like to confirm a > > +1 from you before I merge. Let me know what you think. > > > > > > Justin > > > > On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic < > clebert.suco...@gmail.com> > > wrote: > > > >> I am almost done with this.. I should be able to send a PR tomorrow.. > >> I think it's looking nice. > >> > >> On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic > >> wrote: > >>> @Martyn: That's exactly what I'm planning.. Having the embedded > >>> Derby.. just for out of box testing. > >>> > >>> > >>> someone would do ./artemis create --jdbc ./server-place > >>> > >>> and we would have artemis running with Derby right there. > >>> > >>> > >>> > >>> Now my question here was... where do we change to add stuff into lib. > >>> I changed dep.xml but it's not picking it up. > >>> > >>> > >>> > >>> > >>> On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor > >> wrote: > Michael, > > I think all Clebert is suggesting here is to have something that works > >> out > the box to demonstrate the JDBC store. Derby is a good candidate as > it > >> can > work in memory, and we it in a lot in our tests. I've actually not > >> talked > to Clebert about this, so he can confirm/deny if this was his intent, > >> but I > don't see this being related to maintaining a connector service > implementation? The only Derby specific thing here would be to ship > the > lib as part of the distro and to configure a JDBC URL. I guess we > could > ask for a JDBC URL as part of the prompt, and tell the user to drop > the > >> lib > on the class path, but having a quick and easy way to set up and test > Artemis + JDBC sounds like a UX win to me. > > Cheers > > > > > > > > On Mon, Jan 15, 2018 at 7:24 AM, Michael André Pearce < > michael.andre.pea...@me.com> wrote: > > > Well it kind of is. > > > > are we then saying if a third party lib in this case Derby DB > >> implements > > an interface that artemis provides in this case JDBC in the other > case > > ConnectorService we are happy to depend on it and ship it with > artemis? > > > > I really don’t want to upset or annoy you but at the same time I > >> really do > > want an even playing field. > > > > As I already said I’m happy for artemis to have these. I think if > >> someone > > is willing to support it let it be there. If it ends up being > >>
Re: [DISCUSS] Adding Derby as a dependency
Ok so if I understood this if I made the connector implementation code available in maven central will have it hosted in GitHub the source, then we’d be happy to have that packaged in to provide the functionality? If that’s the case im +1 with this, and then I’ll work on doing that for the Kafka piece then. Cheers Mike Sent from my iPhone > On 18 Jan 2018, at 17:47, Justin Bertramwrote: > > I just reviewed your PR, Clebert, and made a comment. In general I think > it looks great. Nice work. > > I've been thinking about your concern, Michael, that the rules on > integrations like this aren't clear. I went back and reviewed the mailing > list discussion and the discussion on your PR for the Kafka integration > using the ConnectorService. As far as I can tell, the main issue with your > PR was that many didn't want to house the actual implementation-specific > integration code within the Artemis project. It seems to me that this > "rule" is being applied consistently in this case as well, namely that > Artemis is providing an integration point (i.e. the JDBC layer) but doesn't > house implementation-specific code (i.e. Derby). > > The consensus in our previous discussion was that implementation-specific > integration code (e.g. your Kafka connector) should live outside the broker > code-base as a separate component with its own release cycle. This is > exactly how the integration with Derby is working. Derby lives outside the > broker code-base with its own release cycle and is being pulled in via > Maven. If your Kafka connector was available via Maven and we wanted to > create a broker example that used it I don't think that would be an issue. > To be clear, Derby is being packaged with the broker to serve as a default > JDBC implementation, but I don't think it would be an issue to package the > Kafka connector with the broker if there was a similar, real need. > > I don't see any contradictions or inequities here. I'd like to confirm a > +1 from you before I merge. Let me know what you think. > > > Justin > > On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconic > wrote: > >> I am almost done with this.. I should be able to send a PR tomorrow.. >> I think it's looking nice. >> >> On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic >> wrote: >>> @Martyn: That's exactly what I'm planning.. Having the embedded >>> Derby.. just for out of box testing. >>> >>> >>> someone would do ./artemis create --jdbc ./server-place >>> >>> and we would have artemis running with Derby right there. >>> >>> >>> >>> Now my question here was... where do we change to add stuff into lib. >>> I changed dep.xml but it's not picking it up. >>> >>> >>> >>> >>> On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor >> wrote: Michael, I think all Clebert is suggesting here is to have something that works >> out the box to demonstrate the JDBC store. Derby is a good candidate as it >> can work in memory, and we it in a lot in our tests. I've actually not >> talked to Clebert about this, so he can confirm/deny if this was his intent, >> but I don't see this being related to maintaining a connector service implementation? The only Derby specific thing here would be to ship the lib as part of the distro and to configure a JDBC URL. I guess we could ask for a JDBC URL as part of the prompt, and tell the user to drop the >> lib on the class path, but having a quick and easy way to set up and test Artemis + JDBC sounds like a UX win to me. Cheers On Mon, Jan 15, 2018 at 7:24 AM, Michael André Pearce < michael.andre.pea...@me.com> wrote: > Well it kind of is. > > are we then saying if a third party lib in this case Derby DB >> implements > an interface that artemis provides in this case JDBC in the other case > ConnectorService we are happy to depend on it and ship it with artemis? > > I really don’t want to upset or annoy you but at the same time I >> really do > want an even playing field. > > As I already said I’m happy for artemis to have these. I think if >> someone > is willing to support it let it be there. If it ends up being >> unsupportable > remove it. Though that wasn’t the outcome from the last discussion. > > I really do think we need to have clear rules on this. That are generic > about any component, for anyone. > > eg if a component lies without someone maintaining then after 6months >> it > goes to inactive, if still after a year no one steps in it gets >> removed and > moves to an attic. > > > > Sent from my iPhone > >> On 15 Jan 2018, at 02:14, Clebert Suconic >> > wrote: >> >> That’s different. We are not implementing JDBC
Re: [DISCUSS] Adding Derby as a dependency
I just reviewed your PR, Clebert, and made a comment. In general I think it looks great. Nice work. I've been thinking about your concern, Michael, that the rules on integrations like this aren't clear. I went back and reviewed the mailing list discussion and the discussion on your PR for the Kafka integration using the ConnectorService. As far as I can tell, the main issue with your PR was that many didn't want to house the actual implementation-specific integration code within the Artemis project. It seems to me that this "rule" is being applied consistently in this case as well, namely that Artemis is providing an integration point (i.e. the JDBC layer) but doesn't house implementation-specific code (i.e. Derby). The consensus in our previous discussion was that implementation-specific integration code (e.g. your Kafka connector) should live outside the broker code-base as a separate component with its own release cycle. This is exactly how the integration with Derby is working. Derby lives outside the broker code-base with its own release cycle and is being pulled in via Maven. If your Kafka connector was available via Maven and we wanted to create a broker example that used it I don't think that would be an issue. To be clear, Derby is being packaged with the broker to serve as a default JDBC implementation, but I don't think it would be an issue to package the Kafka connector with the broker if there was a similar, real need. I don't see any contradictions or inequities here. I'd like to confirm a +1 from you before I merge. Let me know what you think. Justin On Mon, Jan 15, 2018 at 1:53 PM, Clebert Suconicwrote: > I am almost done with this.. I should be able to send a PR tomorrow.. > I think it's looking nice. > > On Mon, Jan 15, 2018 at 9:44 AM, Clebert Suconic > wrote: > > @Martyn: That's exactly what I'm planning.. Having the embedded > > Derby.. just for out of box testing. > > > > > > someone would do ./artemis create --jdbc ./server-place > > > > and we would have artemis running with Derby right there. > > > > > > > > Now my question here was... where do we change to add stuff into lib. > > I changed dep.xml but it's not picking it up. > > > > > > > > > > On Mon, Jan 15, 2018 at 7:54 AM, Martyn Taylor > wrote: > >> Michael, > >> > >> I think all Clebert is suggesting here is to have something that works > out > >> the box to demonstrate the JDBC store. Derby is a good candidate as it > can > >> work in memory, and we it in a lot in our tests. I've actually not > talked > >> to Clebert about this, so he can confirm/deny if this was his intent, > but I > >> don't see this being related to maintaining a connector service > >> implementation? The only Derby specific thing here would be to ship the > >> lib as part of the distro and to configure a JDBC URL. I guess we could > >> ask for a JDBC URL as part of the prompt, and tell the user to drop the > lib > >> on the class path, but having a quick and easy way to set up and test > >> Artemis + JDBC sounds like a UX win to me. > >> > >> Cheers > >> > >> > >> > >> > >> > >> > >> > >> On Mon, Jan 15, 2018 at 7:24 AM, Michael André Pearce < > >> michael.andre.pea...@me.com> wrote: > >> > >>> Well it kind of is. > >>> > >>> are we then saying if a third party lib in this case Derby DB > implements > >>> an interface that artemis provides in this case JDBC in the other case > >>> ConnectorService we are happy to depend on it and ship it with artemis? > >>> > >>> I really don’t want to upset or annoy you but at the same time I > really do > >>> want an even playing field. > >>> > >>> As I already said I’m happy for artemis to have these. I think if > someone > >>> is willing to support it let it be there. If it ends up being > unsupportable > >>> remove it. Though that wasn’t the outcome from the last discussion. > >>> > >>> I really do think we need to have clear rules on this. That are generic > >>> about any component, for anyone. > >>> > >>> eg if a component lies without someone maintaining then after 6months > it > >>> goes to inactive, if still after a year no one steps in it gets > removed and > >>> moves to an attic. > >>> > >>> > >>> > >>> Sent from my iPhone > >>> > >>> > On 15 Jan 2018, at 02:14, Clebert Suconic > > >>> wrote: > >>> > > >>> > That’s different. We are not implementing JDBC here. > >>> > > >>> > > >>> > We can still provide a pluggavle interface and have the feature you > wrote > >>> > plugging into artemis. Even adding examples with dependencies > towards > >>> it. > >>> > I think it was agreed back then. > >>> > > >>> > > >>> > That’s a different discussion from here though. > >>> > > >>> > On Sun, Jan 14, 2018 at 5:26 PM Michael André Pearce < > >>> > michael.andre.pea...@me.com> wrote: > >>> > > >>> >> Yes. And JDBC the pluggable interface point, JDBC is the api. This > is > >>> just > >>> >> as
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user cshannon commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1789#discussion_r162393664 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/StompPluginTest.java --- @@ -127,10 +131,33 @@ public void testSendAndReceive() throws Exception { AFTER_CREATE_SESSION, BEFORE_CLOSE_SESSION, AFTER_CLOSE_SESSION, BEFORE_CREATE_CONSUMER, AFTER_CREATE_CONSUMER, BEFORE_CLOSE_CONSUMER, AFTER_CLOSE_CONSUMER, BEFORE_CREATE_QUEUE, AFTER_CREATE_QUEUE, MESSAGE_ACKED, BEFORE_SEND, AFTER_SEND, BEFORE_MESSAGE_ROUTE, AFTER_MESSAGE_ROUTE, BEFORE_DELIVER, - AFTER_DELIVER); + AFTER_DELIVER, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS); + + } catch (Throwable e) { + fail(e.getMessage()); + } + + } + + @Test + public void testStompAutoCreateAddress() throws Exception { + + try { + URI uri = new URI("ws+v12.stomp://localhost:61613"); + StompClientConnection newConn = StompClientConnectionFactory.createClientConnection(uri); + newConn.connect(defUser, defPass); + + subscribeQueue(newConn, "a-sub", "autoCreated"); + + // unsub + unsubscribe(newConn, "a-sub"); + newConn.disconnect(); + + verifier.validatePluginMethodsAtLeast(1, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS, + BEFORE_REMOVE_ADDRESS, AFTER_REMOVE_ADDRESS); } catch (Throwable e) { - e.printStackTrace(); + fail(e.getMessage()); --- End diff -- ok i updated the PR ---
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user cshannon commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1789#discussion_r162392727 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/StompPluginTest.java --- @@ -127,10 +131,33 @@ public void testSendAndReceive() throws Exception { AFTER_CREATE_SESSION, BEFORE_CLOSE_SESSION, AFTER_CLOSE_SESSION, BEFORE_CREATE_CONSUMER, AFTER_CREATE_CONSUMER, BEFORE_CLOSE_CONSUMER, AFTER_CLOSE_CONSUMER, BEFORE_CREATE_QUEUE, AFTER_CREATE_QUEUE, MESSAGE_ACKED, BEFORE_SEND, AFTER_SEND, BEFORE_MESSAGE_ROUTE, AFTER_MESSAGE_ROUTE, BEFORE_DELIVER, - AFTER_DELIVER); + AFTER_DELIVER, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS); + + } catch (Throwable e) { + fail(e.getMessage()); + } + + } + + @Test + public void testStompAutoCreateAddress() throws Exception { + + try { + URI uri = new URI("ws+v12.stomp://localhost:61613"); + StompClientConnection newConn = StompClientConnectionFactory.createClientConnection(uri); + newConn.connect(defUser, defPass); + + subscribeQueue(newConn, "a-sub", "autoCreated"); + + // unsub + unsubscribe(newConn, "a-sub"); + newConn.disconnect(); + + verifier.validatePluginMethodsAtLeast(1, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS, + BEFORE_REMOVE_ADDRESS, AFTER_REMOVE_ADDRESS); } catch (Throwable e) { - e.printStackTrace(); + fail(e.getMessage()); --- End diff -- yeah i can remove it, i'm not sure why it's there at all ---
[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1783#discussion_r162391229 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/transformer/ServerMessageImpl.java --- @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.transformer; + +import org.apache.activemq.artemis.api.core.ICoreMessage; +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.message.impl.CoreMessage; +import org.apache.activemq.artemis.core.message.impl.MessageInternalImpl; +import org.apache.activemq.artemis.core.paging.PagingStore; +import org.apache.activemq.artemis.core.server.MessageReference; +import org.apache.activemq.artemis.core.server.Queue; +import org.apache.activemq.artemis.core.server.ServerMessage; + +@Deprecated --- End diff -- can you add a strong statement on javadoc? something .. DO NOT USE, for backward compatibility only! ---
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1789#discussion_r162390676 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/StompPluginTest.java --- @@ -127,10 +131,33 @@ public void testSendAndReceive() throws Exception { AFTER_CREATE_SESSION, BEFORE_CLOSE_SESSION, AFTER_CLOSE_SESSION, BEFORE_CREATE_CONSUMER, AFTER_CREATE_CONSUMER, BEFORE_CLOSE_CONSUMER, AFTER_CLOSE_CONSUMER, BEFORE_CREATE_QUEUE, AFTER_CREATE_QUEUE, MESSAGE_ACKED, BEFORE_SEND, AFTER_SEND, BEFORE_MESSAGE_ROUTE, AFTER_MESSAGE_ROUTE, BEFORE_DELIVER, - AFTER_DELIVER); + AFTER_DELIVER, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS); + + } catch (Throwable e) { + fail(e.getMessage()); + } + + } + + @Test + public void testStompAutoCreateAddress() throws Exception { + + try { + URI uri = new URI("ws+v12.stomp://localhost:61613"); + StompClientConnection newConn = StompClientConnectionFactory.createClientConnection(uri); + newConn.connect(defUser, defPass); + + subscribeQueue(newConn, "a-sub", "autoCreated"); + + // unsub + unsubscribe(newConn, "a-sub"); + newConn.disconnect(); + + verifier.validatePluginMethodsAtLeast(1, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS, + BEFORE_REMOVE_ADDRESS, AFTER_REMOVE_ADDRESS); } catch (Throwable e) { - e.printStackTrace(); + fail(e.getMessage()); --- End diff -- h.. but it was being ignored before? maybe just remove the catch and let it fail? remove the throws Exception on the test? ---
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1789#discussion_r162390344 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/StompPluginTest.java --- @@ -127,10 +131,33 @@ public void testSendAndReceive() throws Exception { AFTER_CREATE_SESSION, BEFORE_CLOSE_SESSION, AFTER_CLOSE_SESSION, BEFORE_CREATE_CONSUMER, AFTER_CREATE_CONSUMER, BEFORE_CLOSE_CONSUMER, AFTER_CLOSE_CONSUMER, BEFORE_CREATE_QUEUE, AFTER_CREATE_QUEUE, MESSAGE_ACKED, BEFORE_SEND, AFTER_SEND, BEFORE_MESSAGE_ROUTE, AFTER_MESSAGE_ROUTE, BEFORE_DELIVER, - AFTER_DELIVER); + AFTER_DELIVER, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS); + + } catch (Throwable e) { + fail(e.getMessage()); + } + + } + + @Test + public void testStompAutoCreateAddress() throws Exception { + + try { + URI uri = new URI("ws+v12.stomp://localhost:61613"); + StompClientConnection newConn = StompClientConnectionFactory.createClientConnection(uri); + newConn.connect(defUser, defPass); + + subscribeQueue(newConn, "a-sub", "autoCreated"); + + // unsub + unsubscribe(newConn, "a-sub"); + newConn.disconnect(); + + verifier.validatePluginMethodsAtLeast(1, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS, + BEFORE_REMOVE_ADDRESS, AFTER_REMOVE_ADDRESS); } catch (Throwable e) { - e.printStackTrace(); + fail(e.getMessage()); --- End diff -- this is actually the only request I'm going to make.. almost merged this. ---
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1789#discussion_r162390192 --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/StompPluginTest.java --- @@ -127,10 +131,33 @@ public void testSendAndReceive() throws Exception { AFTER_CREATE_SESSION, BEFORE_CLOSE_SESSION, AFTER_CLOSE_SESSION, BEFORE_CREATE_CONSUMER, AFTER_CREATE_CONSUMER, BEFORE_CLOSE_CONSUMER, AFTER_CLOSE_CONSUMER, BEFORE_CREATE_QUEUE, AFTER_CREATE_QUEUE, MESSAGE_ACKED, BEFORE_SEND, AFTER_SEND, BEFORE_MESSAGE_ROUTE, AFTER_MESSAGE_ROUTE, BEFORE_DELIVER, - AFTER_DELIVER); + AFTER_DELIVER, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS); + + } catch (Throwable e) { + fail(e.getMessage()); + } + + } + + @Test + public void testStompAutoCreateAddress() throws Exception { + + try { + URI uri = new URI("ws+v12.stomp://localhost:61613"); + StompClientConnection newConn = StompClientConnectionFactory.createClientConnection(uri); + newConn.connect(defUser, defPass); + + subscribeQueue(newConn, "a-sub", "autoCreated"); + + // unsub + unsubscribe(newConn, "a-sub"); + newConn.disconnect(); + + verifier.validatePluginMethodsAtLeast(1, BEFORE_ADD_ADDRESS, AFTER_ADD_ADDRESS, + BEFORE_REMOVE_ADDRESS, AFTER_REMOVE_ADDRESS); } catch (Throwable e) { - e.printStackTrace(); + fail(e.getMessage()); --- End diff -- the printStackTrace is useful when it fails :) I have been there. ---
[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1783#discussion_r162389643 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/transformer/ServerMessageImpl.java --- @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.transformer; + +import org.apache.activemq.artemis.api.core.ICoreMessage; +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.message.impl.CoreMessage; +import org.apache.activemq.artemis.core.message.impl.MessageInternalImpl; +import org.apache.activemq.artemis.core.paging.PagingStore; +import org.apache.activemq.artemis.core.server.MessageReference; +import org.apache.activemq.artemis.core.server.Queue; +import org.apache.activemq.artemis.core.server.ServerMessage; + +@Deprecated +public class ServerMessageImpl extends MessageInternalImpl implements ServerMessage { + + private CoreMessage message; + + private boolean valid = false; + + public boolean isValid() { + return false; + } + + @Override + public ICoreMessage getICoreMessage() { + return message; + } + + public ServerMessageImpl(Message message) { + super(message.toCore()); + this.message = (CoreMessage) message.toCore(); + } + + @Override + public ServerMessage setMessageID(long id) { + message.setMessageID(id); + return this; + } + + @Override + public MessageReference createReference(Queue queue) { + throw new UnsupportedOperationException(); + } + + /** +* This will force encoding of the address, and will re-check the buffer +* This is to avoid setMessageTransient which set the address without changing the buffer +* +* @param address +*/ + @Override + public void forceAddress(SimpleString address) { + message.setAddress(address); + } + + @Override + public int incrementRefCount() throws Exception { + throw new UnsupportedOperationException(); + } + + @Override + public int decrementRefCount() throws Exception { + throw new UnsupportedOperationException(); + } + + @Override + public int incrementDurableRefCount() { + throw new UnsupportedOperationException(); + } + + @Override + public int decrementDurableRefCount() { + throw new UnsupportedOperationException(); + } + + @Override + public ServerMessage copy(long newID) { + throw new UnsupportedOperationException(); + } + + @Override + public ServerMessage copy() { + throw new UnsupportedOperationException(); + } + + @Override + public int getMemoryEstimate() { + return message.getMemoryEstimate(); + } + + @Override + public int getRefCount() { + return message.getRefCount(); + } + + @Override + public ServerMessage makeCopyForExpiryOrDLA(long newID, + MessageReference originalReference, + boolean expiry, + boolean copyOriginalHeaders) throws Exception { + throw new UnsupportedOperationException(); + } + + @Override + public void setOriginalHeaders(ServerMessage otherServerMessage, MessageReference originalReference, boolean expiry) { + + ICoreMessage other = otherServerMessage.getICoreMessage(); + + SimpleString originalQueue = other.getSimpleStringProperty(Message.HDR_ORIGINAL_QUEUE); + + if (originalQueue != null) { + message.putStringProperty(Message.HDR_ORIGINAL_QUEUE, originalQueue);
[GitHub] activemq-artemis issue #1781: ARTEMIS-1613 Integrating JDBC into CLI (create...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1781 I think it's worth clarifying the specific purpose of Derby in the documentation. My understanding here is that Derby is being packaged for demonstration purposes only and shouldn't be used as a production JDBC implementation. ---
[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...
GitHub user cshannon opened a pull request: https://github.com/apache/activemq-artemis/pull/1789 ARTEMIS-1619 - Add plugin support for address lifecyle Adding callbacks to the plugin API for address creation, update and removals You can merge this pull request into a Git repository by running: $ git pull https://github.com/cshannon/activemq-artemis ARTEMIS-1619 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/1789.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 #1789 commit 07b8e7975d2e393f8a0c9c8b75e40bac36f13519 Author: Christopher L. Shannon (cshannon)Date: 2018-01-18T15:10:43Z ARTEMIS-1619 - Add plugin support for address lifecyle Adding callbacks to the plugin API for address creation, update and removals ---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1787 ---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Github user michaelandrepearce commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1787#discussion_r162367328 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java --- @@ -1284,7 +1284,7 @@ private boolean checkDuplicateID(final Message message, boolean isDuplicate = false; if (duplicateIDBytes != null) { -cache = getDuplicateIDCache(message.getAddressSimpleString()); +cache = getDuplicateIDCache(context.getAddress(message)); --- End diff -- No worries I can do separate pr later ---
[GitHub] activemq-artemis issue #1787: ARTEMIS-1577 Address-settings policies not wor...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1787 +1 Iâm happy as noted I can pr some bits later ---
[GitHub] activemq-artemis issue #1788: ARTEMIS-1617 - Properly set autoCreated flag o...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1788 Nice work, @cshannon. ---
[GitHub] activemq-artemis pull request #1788: ARTEMIS-1617 - Properly set autoCreated...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1788 ---
[GitHub] activemq-artemis pull request #1771: ARTEMIS-1600 Support masked passwords i...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1771 ---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1787#discussion_r162358705 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java --- @@ -1284,7 +1284,7 @@ private boolean checkDuplicateID(final Message message, boolean isDuplicate = false; if (duplicateIDBytes != null) { -cache = getDuplicateIDCache(message.getAddressSimpleString()); +cache = getDuplicateIDCache(context.getAddress(message)); --- End diff -- @michaelandrepearce caching here goes beyond the scope here. I would need to change a lot of signatures to pass an address as a parameter.. or cache within each method used during routing. We can do that.. but it goes beyond the scope of this fix. ---
[GitHub] activemq-artemis issue #1783: ARTEMIS-1611 Added support for 1.x transformer...
Github user mtaylor commented on the issue: https://github.com/apache/activemq-artemis/pull/1783 @jmesnil Made the changes you suggested. @clebertsuconic I agree with @jmesnil that adding back the original types is the clearest and safest thing to do, with the disadvantage of add more deprecated code back. ---
[GitHub] activemq-artemis pull request #1783: ARTEMIS-1611 Added support for 1.x tran...
Github user mtaylor commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1783#discussion_r162356443 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/transformer/ServerMessageImpl.java --- @@ -0,0 +1,192 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.transformer; + +import org.apache.activemq.artemis.api.core.ICoreMessage; +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.message.impl.CoreMessage; +import org.apache.activemq.artemis.core.message.impl.MessageInternalImpl; +import org.apache.activemq.artemis.core.paging.PagingStore; +import org.apache.activemq.artemis.core.server.MessageReference; +import org.apache.activemq.artemis.core.server.Queue; +import org.apache.activemq.artemis.core.server.ServerMessage; +import org.apache.activemq.artemis.core.server.impl.MessageReferenceImpl; +import sun.reflect.generics.reflectiveObjects.NotImplementedException; --- End diff -- Yes this was a mistake. Fixed it. ---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1787#discussion_r162353710 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java --- @@ -736,11 +736,11 @@ public RoutingStatus route(final Message message, throw new IllegalStateException("Message cannot be routed more than once"); } - setPagingStore(message); + setPagingStore(context.getAddress(message), message); AtomicBoolean startedTX = new AtomicBoolean(false); - final SimpleString address = message.getAddressSimpleString(); + final SimpleString address = context.getAddress(message); --- End diff -- I'm just going for the simple refactoring here.. every call to message.getAddressSimplString() to be calling context.getAddress() ---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Github user clebertsuconic commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1787#discussion_r162353815 --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java --- @@ -736,11 +736,11 @@ public RoutingStatus route(final Message message, throw new IllegalStateException("Message cannot be routed more than once"); } - setPagingStore(message); + setPagingStore(context.getAddress(message), message); AtomicBoolean startedTX = new AtomicBoolean(false); - final SimpleString address = message.getAddressSimpleString(); + final SimpleString address = context.getAddress(message); --- End diff -- Your comment could apply to the previous version as well.. I didn't make any big changes here. nor did want to risk anything else. ---
[GitHub] activemq-artemis pull request #1775: ARTEMIS-1587 Add setting to control the...
Github user mtaylor commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1775#discussion_r162352945 --- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd --- @@ -2790,6 +2790,14 @@ + --- End diff -- This is so that any override can be switched to OFF. ---
[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...
Github user mtaylor commented on the issue: https://github.com/apache/activemq-artemis/pull/1775 Sorry guys but I am -1 on this. I thought the idea of this was to behave in a similar way to default-max-consumers, or default-purge-on-no-consumers. I am not happy about overriding the protocol handlers behaviour in this way. I can see value in a default-queue-durability setting, for cases when it's not known. Can the original requirement not be addressed using auto-delete-queues and non-durable messages? If non-durable queues/temps are really required, how about a persistence=false address-setting instead? ---
[GitHub] activemq-artemis pull request #1788: ARTEMIS-1617 - Properly set autoCreated...
GitHub user cshannon opened a pull request: https://github.com/apache/activemq-artemis/pull/1788 ARTEMIS-1617 - Properly set autoCreated flag on address Flag needs to be set when auto creating an address so that the address can be removed later if auto delete is configured when creating a subscription with MQTT You can merge this pull request into a Git repository by running: $ git pull https://github.com/cshannon/activemq-artemis ARTEMIS-1617 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/1788.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 #1788 commit dbc21a0c0d245ee7eeab2f992d3150165add4d1b Author: Christopher L. Shannon (cshannon)Date: 2018-01-18T13:36:07Z ARTEMIS-1617 - Properly set autoCreated flag on address Flag needs to be set when auto creating an address so that the address can be removed later if auto delete is configured when creating a subscription with MQTT ---
[GitHub] activemq-artemis pull request #1775: ARTEMIS-1587 Add setting to control the...
Github user stanlyDoge commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1775#discussion_r162343251 --- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd --- @@ -2790,6 +2790,14 @@ + --- End diff -- Please, would you be so patient and explain me, how to do this? I have implemented enum (in latest commit), but I don't think so it is what you intended. ---
[GitHub] activemq-artemis pull request #1777: ARTEMIS-1606 - Change AddressInfo Routi...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1777 ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Great thanks, ill merge then. ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 These are the flamegraph of a couple of profiled tests, one on master: ![image](https://user-images.githubusercontent.com/13125299/35093335-7394c032-fc42-11e7-862e-936863f37eff.png) Where the violet bars are the `AddressInfo::new` and the `AddressInfo::getRoutingType` (+ iterator) while with this PR: ![image](https://user-images.githubusercontent.com/13125299/35093498-d48ef4ca-fc42-11e7-81c9-04565ea66c64.png) There isn't anymore any cost associated with `AddressInfo`: for me is a thumbs up! Althouh it seems negligible (at a first look) the impact of this change has changed the garbage produced and the CPU time required to call `doSend`: indeed the latencies are better too. Well done :100: ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @michaelandrepearce I'm running a test before and after, but first can you try add ".jvmArgs("-XX:+UseG1GC")" and `.forks(2)` to your benchmark? ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @franz1981 i was waiting on you, to give a thumbs up before i merged, is that a thumbs up? ---
[GitHub] activemq-artemis pull request #1786: ARTEMIS-1616 OpenWire improvements
Github user franz1981 commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1786#discussion_r162272711 --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java --- @@ -361,13 +371,68 @@ public int hashCode() { } } + private static SimpleString[] splitWithCachedString(final SimpleString simpleString, final int delim) { --- End diff -- To avoid the translation into into each time because String::indexOf is using int ---
[GitHub] activemq-artemis pull request #1786: ARTEMIS-1616 OpenWire improvements
Github user franz1981 commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1786#discussion_r162272578 --- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java --- @@ -361,13 +371,68 @@ public int hashCode() { } } + private static SimpleString[] splitWithCachedString(final SimpleString simpleString, final int delim) { + final String str = simpleString.str; + final byte[] data = simpleString.data; + final int length = str.length(); + List all = null; + int index = 0; + while (index < length) { + final int delimIndex = str.indexOf(delim, index); + if (delimIndex == -1) { +//just need to add the last one +break; + } else { +all = addSimpleStringPart(all, data, index, delimIndex); + } + index = delimIndex + 1; + } + if (all == null) { + return new SimpleString[]{simpleString}; + } else { + // Adding the last one + all = addSimpleStringPart(all, data, index, length); + // Converting it to arrays + final SimpleString[] parts = new SimpleString[all.size()]; + return all.toArray(parts); + } + } + + private static List addSimpleStringPart(List all, + final byte[] data, + final int startIndex, + final int endIndex) { + final int expectedLength = endIndex - startIndex; + final SimpleString ss; + if (expectedLength == 0) { + ss = EMPTY; + } else { + //extract a byte[] copy from this + final int ssIndex = startIndex << 1; + final int delIndex = endIndex << 1; + final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex); + ss = new SimpleString(bytes); + } + // We will create the ArrayList lazily + if (all == null) { + // There will be at least 3 strings on this case (which is the actual common usecase) --- End diff -- Good point: the ArrayList is temporary and although it won't be allocated on the stack having 2 instead of 3 won't make a big difference considering that will die young. I've noticed that most use cases need at least 3 to avoid enlarging of capacity so I've changed it: I haven't changed the original version yet as you have noticed and I could do :) 2 or 3 are very "magical" numbers anyway :P ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Very good results! A lil OT but seems that we really start need a JMH folder with all the benchs on Artemis eh? :P ---