[GitHub] activemq-artemis pull request #1789: ARTEMIS-1619 - Add plugin support for a...

2018-01-18 Thread asfgit
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 ...

2018-01-18 Thread asfgit
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

2018-01-18 Thread michaelandrepearce
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 ...

2018-01-18 Thread clebertsuconic
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

2018-01-18 Thread Clebert Suconic
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 Suconic
 wrote:
> 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

2018-01-18 Thread Clebert Suconic
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 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...

2018-01-18 Thread asfgit
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 ...

2018-01-18 Thread michaelandrepearce
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

2018-01-18 Thread Michael André Pearce
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
>> 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 ...

2018-01-18 Thread clebertsuconic
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

2018-01-18 Thread Timothy Bish

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 ...

2018-01-18 Thread jbertram
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 ...

2018-01-18 Thread jbertram
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 ...

2018-01-18 Thread jbertram
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

2018-01-18 Thread Clebert Suconic
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, 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 ...

2018-01-18 Thread franz1981
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...

2018-01-18 Thread franz1981
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 Nigro 
Date:   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

2018-01-18 Thread Clebert Suconic
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
> >>> 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

2018-01-18 Thread Timothy Bish
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 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

2018-01-18 Thread Clebert Suconic
On Thu, Jan 18, 2018 at 1:12 PM, Arthur Naseef  wrote:
> 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...

2018-01-18 Thread asfgit
Github user asfgit closed the pull request at:

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


---


Re: [DISCUSS] Adding Derby as a dependency

2018-01-18 Thread Arthur Naseef
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 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 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

2018-01-18 Thread Justin Bertram
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 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 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...

2018-01-18 Thread clebertsuconic
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 ...

2018-01-18 Thread clebertsuconic
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 ...

2018-01-18 Thread clebertsuconic
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

2018-01-18 Thread Robbie Gemmell
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 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...

2018-01-18 Thread clebertsuconic
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

2018-01-18 Thread Clebert Suconic
On Thu, Jan 18, 2018 at 11:47 AM, Justin Bertram  wrote:

> 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

2018-01-18 Thread Justin Bertram
> ...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 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

2018-01-18 Thread Michael André Pearce
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 
> 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

2018-01-18 Thread Justin Bertram
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 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...

2018-01-18 Thread cshannon
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...

2018-01-18 Thread cshannon
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...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread jbertram
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...

2018-01-18 Thread cshannon
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 ...

2018-01-18 Thread asfgit
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 ...

2018-01-18 Thread michaelandrepearce
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...

2018-01-18 Thread michaelandrepearce
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...

2018-01-18 Thread jbertram
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...

2018-01-18 Thread asfgit
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...

2018-01-18 Thread asfgit
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 ...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread mtaylor
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...

2018-01-18 Thread mtaylor
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 ...

2018-01-18 Thread clebertsuconic
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 ...

2018-01-18 Thread clebertsuconic
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...

2018-01-18 Thread mtaylor
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 ...

2018-01-18 Thread mtaylor
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...

2018-01-18 Thread cshannon
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...

2018-01-18 Thread stanlyDoge
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...

2018-01-18 Thread asfgit
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 ...

2018-01-18 Thread michaelandrepearce
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 ...

2018-01-18 Thread franz1981
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 ...

2018-01-18 Thread franz1981
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 ...

2018-01-18 Thread michaelandrepearce
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

2018-01-18 Thread franz1981
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

2018-01-18 Thread franz1981
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 ...

2018-01-18 Thread franz1981
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


---