[GitHub] activemq-artemis issue #2410: ARTEMIS-2125 Tabs preference changes to displa...
Github user tadayosi commented on the issue: https://github.com/apache/activemq-artemis/pull/2410 @feuillemorte Sorry for the late response. Looks good to me! ---
[GitHub] activemq-artemis pull request #2385: ARTEMIS-1929 race in STOMP identical du...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2385 ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 Pushed update and build with above mentioned changes, ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 So i looked at this some more to see what i can do, i simply can't work around the fact the OpenWireMessage will always return an int, as such it will always set sequence, so where the producer is OpenWire then when we transform to any other protocol then it will exist. " coreMessage.putIntProperty(AMQ_MSG_GROUP_SEQUENCE, messageSend.getGroupSequence()); " So we will need to simply handle mapping this better during converters. I can though remove the setters i had added into AMQPMessage, i think this is the most controversial part. Ill update with those bits removed. ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user gemmellr commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 > So openwire always returns a value on asking groupseq. It never gives an ability to determine if it was set by client or not. It does seem to return a value if you ask for one, which it shouldn't, but it also does not populate the property name all the time, so its not clear why the value would be asked for. This is showing up in the AMQP message because it is in the property names being iterated, which it shouldn't be. > We have to be able to set it somehow during conversion. This is being set on message conversion. The issues here are on protocol conversion. Were not changing amqp message From what I see, the method is not used during conversion to an AMQP message, with that being set directly on the underlying Properties section within the converter. Setting it any other time is then stepping toward a protocol violation. I am not asking for just a sticking plaster. I did suggest that we fix small "one line" bugs separately from comparatively massive general code refactorings (did not say they shouldnt be done). I suggest that we avoid actively violating the protocol as this likely will likely result (and as mentioned, the setter was noted as being deliberately left out of the AMQP bits when that method was added). I do not think we should emit group-sequence values that were never set on the message. I'm done giving feedback on Artemis issues for a while. ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 I can add tests that would show the other breaks i found. That a sticky plaster null check solution wouldnt fix. ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 I would much rather we fix this proper of your adament for a sticky plaster then fine. But i see doing that being problematic longer run as we just keep hiding underlying issues ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 Re: Oh, right. Yes, according to the changelog adding it, it deliberately wasn't implemented in the AMQPMessage, which makes sense because per my earlier comment using it would be a protocol violation given it is an immutable part of the message. So that bit should be unwound I think. We have to be able to set it somehow. This is being set on messags conversion. The issues here are on protocol conversion. Were not changing amqp message ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 So openwire always returns a value on asking groupseq. It never gives an ability to determine if it was set by client or not. ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user gemmellr commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 > Re seq not set, its an int field so its tech always set if you request it, just should default 0 if not set. JMSXGroupSeq is set explicitly by the application. If it isn't set by the application, then the property doesn't exist, the same as any other int property. It shouldn't be in the property names and we shouldn't be setting group-sequence to 0 on the wire when nothing has actually set JMSXGroupSeq. > Re the name this is the name in the message interface that already existed for that. Oh, right. Yes, according to the changelog adding it, it deliberately wasn't implemented in the AMQPMessage, which makes sense because per my earlier comment using it would be a protocol violation given it is an immutable part of the message. So that bit should be unwound I think. > If i was just to fix the test, it would have been a one liner null check , and i didnt want to just fix the test (would have been less work for me too if i did that). But the fact is as i mentioned above finding a few things that better to fix it all up a bit, thus why its a bit more involved change I think a one-liner fix would probably have been better, keeping the general improvement separate/later. It makes it easier to see the actual issue and fix is, and makes backporting bug fixes easier without extra changes either being in the way or tagging along (not that that actually applies here specifically, but generally I mean). As I've said though, it doesnt seem that either actually addresses the apparent main issue though, of group sequence values being present in messages a sequence was never set on. ---
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user jsmucr commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 Okay. :) ---
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 @jsmucr maybe then an integration test to prove it, and also to make sure if it does work. No one breaks it on you ---
[GitHub] activemq-artemis issue #2413: NO-JIRA fixing tests
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2413 Looks sensible to me. I normally leave you just to merge. Let me know if youd rather someone else (me) to merge, ill do it later tonight if so ---
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user jsmucr commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 I think you mean something like this, right? ```JAVA_ARGS="$JAVA_ARGS -Dcom.sun.management.jmxremote" JAVA_ARGS="$JAVA_ARGS -Dcom.sun.management.jmxremote.authenticate=false" JAVA_ARGS="$JAVA_ARGS -Dcom.sun.management.jmxremote.ssl=false" JAVA_ARGS="$JAVA_ARGS -Dcom.sun.management.jmxremote.port=1099" JAVA_ARGS="$JAVA_ARGS -Dcom.sun.management.jmxremote.rmi.port=1098" JAVA_ARGS="$JAVA_ARGS -Djava.rmi.server.hostname=edimq-broker-master-az1.dc01.clouedi.local"``` This works but it's not how it's meant to be set up. The proper way is to set the port in the `management.xml` file and this patch allows you to set the RMI port there as well (using the `rmi-registry-port` attribute). It worked for me. ---
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 We fix the ports in my org this with using system properties on java command line modifiying artemis.profile ---
[GitHub] activemq-artemis issue #2385: ARTEMIS-1929 race in STOMP identical durable s...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2385 Lgtm ---
[GitHub] activemq-artemis issue #2385: ARTEMIS-1929 race in STOMP identical durable s...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/2385 @jostbg, yes I think they're essentially the same. ---
[GitHub] activemq-artemis issue #2385: ARTEMIS-1929 race in STOMP identical durable s...
Github user jostbg commented on the issue: https://github.com/apache/activemq-artemis/pull/2385 @jbertram is that a similar issue, but with AMQP? https://issues.apache.org/jira/browse/ARTEMIS-2140 ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 So the issue was sequence wasnt being transposed correctly also found other jmsx ones werent in doing so. But just been under the cover. Thus what we fix here. Re the name this is the name in the message interface that already existed for that. ---
Re: Website
I'd be happy to help with adding content, I remember a hackathon was
mentioned at one point.
Any way since it's now in git it would be great if people could contribute
so we can have a better website sooner rather than later.
Andy
On Tue, 6 Nov 2018 at 17:19, Martyn Taylor wrote:
> The site is uploaded and links added.
>
> Note there's currently a sync issue with EU mirrors[1], it's currently
> being worked on. To view the changes in the interim, use the US mirror[2].
>
> Cheers
> Martyn
>
> 1. https://status.apache.org/
> 2. http://activemq.us.apache.org/
>
> On Tue, Nov 6, 2018 at 4:58 PM Martyn Taylor wrote:
>
> > I've went ahead and populated the repository, there's a GitHub mirror
> > here[1]. I've also generated the static content and should have the work
> > in progress hosted under a separately directory and a message on the
> > homepage shortly.
> >
> > Cheers
> >
> > 1. https://github.com/apache/activemq-website
> >
> >
> > On Fri, Oct 26, 2018 at 8:54 PM Bruce Snyder
> > wrote:
> >
> >> +1
> >>
> >> Bruce
> >>
> >> On Tue, Oct 23, 2018 at 2:26 PM Clebert Suconic <
> >> clebert.suco...@gmail.com>
> >> wrote:
> >>
> >> > What about this. WE could :
> >> >
> >> > - commit this on git
> >> > - upload a snapshot for review under our web-site.
> >> > On Mon, Oct 22, 2018 at 8:46 AM Clebert Suconic
> >> > wrote:
> >> > >
> >> > > +1000. We had that options discussed here a few times. Having the
> old
> >> > website somewhere with a link is the way to go for the migration.
> >> > >
> >> > > On Mon, Oct 22, 2018 at 7:47 AM michael.andre.pearce <
> >> > michael.andre.pea...@me.com.invalid> wrote:
> >> > >>
> >> > >> Sounds good to me.
> >> > >>
> >> > >>
> >> > >> Sent from my Samsung Galaxy smartphone.
> >> > >> Original message From: Martyn Taylor <
> >> > mtay...@redhat.com> Date: 22/10/2018 12:38 (GMT+00:00) To:
> >> > dev@activemq.apache.org Subject: Re: Website
> >> > >> How about an equivalent message on the home page of the new
> website,
> >> > >> directing users to the old site?
> >> > >>
> >> > >> On Fri, Oct 19, 2018 at 10:39 PM Bruce Snyder <
> >> bruce.sny...@gmail.com>
> >> > >> wrote:
> >> > >>
> >> > >> > Check out the Camel website and the message that they posted
> about
> >> > their
> >> > >> > website changes that are underway:
> >> > >> >
> >> > >> > https://camel.apache.org/
> >> > >> >
> >> > >> > Bruce
> >> > >> >
> >> > >> > On Wed, Oct 17, 2018 at 3:30 AM Martyn Taylor <
> mtay...@redhat.com>
> >> > wrote:
> >> > >> >
> >> > >> > > On Fri, Oct 12, 2018 at 8:38 PM Bruce Snyder <
> >> > bruce.sny...@gmail.com>
> >> > >> > > wrote:
> >> > >> > >
> >> > >> > > > Fair points, Martyn. I like the idea of a combination of JIRA
> >> > issues
> >> > >> > > plus a
> >> > >> > > > list of those issues that is easily visible somewhere such as
> >> the
> >> > wiki.
> >> > >> > > >
> >> > >> > > > Seeing as how we are moving the website around completely, I
> >> > believe we
> >> > >> > > > need to include the docs to support the older versions in
> some
> >> > way. So,
> >> > >> > > the
> >> > >> > > > first thought that came to mind is a search capability in
> order
> >> > for
> >> > >> > users
> >> > >> > > > to find them easier. Another idea, we could also keep the old
> >> site
> >> > >> > intact
> >> > >> > > > and available as a sub-domain or URL, e.g.,
> >> > 5x.activemq.apache.org,
> >> > >> > > > activemq.apache.org/5.x, etc. However, the advantage of a
> >> search
> >> > >> > feature
> >> > >> > > > is
> >> > >> > > > that it would be useful across the site, not just for older
> >> > stuff. We
> >> > >> > > might
> >> > >> > > > even be able structure the search to allow users to select a
> >> > version of
> >> > >> > > the
> >> > >> > > > docs that they would like to search. Anyway, just some
> >> thoughts.
> >> > >> > > >
> >> > >> > > > I agree that we should try to get the new site functional
> ASAP,
> >> > but
> >> > >> > that
> >> > >> > > > should not eliminate the old site entirely. We should not
> >> abandon
> >> > users
> >> > >> > > of
> >> > >> > > > older versions.
> >> > >> > > >
> >> > >> > > +1 Bruce.
> >> > >> > >
> >> > >> > > I had thought that the export of the current site into the
> >> gitbook
> >> > had
> >> > >> > all
> >> > >> > > the info, but perhaps there are missing pieces. I like the
> idea
> >> of
> >> > >> > keeping
> >> > >> > > the old site running and link to it from the new site. This
> >> ensures
> >> > >> > > existing users have all the info they need, but let's us move
> >> > forward
> >> > >> > with
> >> > >> > > new content. Perhaps we can switch out the exiting home page
> for
> >> > the new
> >> > >> > > one and add a link to old website AMQ project page ("Existing
> >> users
> >> > >> > looking
> >> > >> > > for the original ActiveMQ website click here."). If we are
> able
> >> to
> >> > get
> >> > >> > > some stats on number of accesses to a particular page, we can
> use
> >> > that
> >> > >> > info
> >>
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user andytaylor commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 Im not sure it does, i wrote this code and remember it not working well when i did the same thing. Also you can set this as a separate system property as per the docs ---
[GitHub] activemq-artemis issue #2420: Allow configuration of RMI registry port
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/2420 Have you confirmed that this change actually sets the RMI registry port as expected? ---
[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2287#discussion_r231511486
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
---
@@ -299,36 +301,57 @@ protected FileLock tryLock(final long lockPos) throws
IOException {
protected FileLock lock(final long lockPosition) throws Exception {
long start = System.currentTimeMillis();
+ boolean isRecurringFailure = false;
while (!interrupted) {
- FileLock lock = tryLock(lockPosition);
-
- if (lock == null) {
-try {
- Thread.sleep(500);
-} catch (InterruptedException e) {
- return null;
-}
-
-if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
- throw new Exception("timed out waiting for lock");
+ try {
+FileLock lock = tryLock(lockPosition);
+isRecurringFailure = false;
+
+if (lock == null) {
+ logger.debug("lock is null");
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ return null;
+ }
+
+ if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
+ throw new Exception("timed out waiting for lock");
+ }
+} else {
+ return lock;
}
- } else {
-return lock;
+ } catch (IOException e) {
+// IOException during trylock() may be a temporary issue, e.g.
NFS volume not being accessible
+logger.log(isRecurringFailure ? Logger.Level.DEBUG :
Logger.Level.WARN,
+"Failure when accessing a lock file", e);
+isRecurringFailure = true;
--- End diff --
This would continue to attempt to lock regardless the
`lockAcquisitionTimeout` (if any): i suppose it would be more robust if you
will check the timeout anyway. There are tests that are using that timeout and
I suppose it should be honored
---
[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2287#discussion_r231513243
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
---
@@ -299,36 +301,57 @@ protected FileLock tryLock(final long lockPos) throws
IOException {
protected FileLock lock(final long lockPosition) throws Exception {
long start = System.currentTimeMillis();
+ boolean isRecurringFailure = false;
while (!interrupted) {
- FileLock lock = tryLock(lockPosition);
-
- if (lock == null) {
-try {
- Thread.sleep(500);
-} catch (InterruptedException e) {
- return null;
-}
-
-if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
- throw new Exception("timed out waiting for lock");
+ try {
+FileLock lock = tryLock(lockPosition);
+isRecurringFailure = false;
+
+if (lock == null) {
+ logger.debug("lock is null");
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ return null;
+ }
+
+ if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
+ throw new Exception("timed out waiting for lock");
+ }
+} else {
+ return lock;
}
- } else {
-return lock;
+ } catch (IOException e) {
+// IOException during trylock() may be a temporary issue, e.g.
NFS volume not being accessible
+logger.log(isRecurringFailure ? Logger.Level.DEBUG :
Logger.Level.WARN,
+"Failure when accessing a lock file", e);
+isRecurringFailure = true;
+Thread.sleep(LOCK_ACCESS_FAILURE_WAIT_TIME);
}
}
// todo this is here because sometimes channel.lock throws a
resource deadlock exception but trylock works,
// need to investigate further and review
- FileLock lock;
+ FileLock lock = null;
--- End diff --
Same thing as the comment above.
---
[GitHub] activemq-artemis pull request #2420: Allow configuration of RMI registry por...
GitHub user jsmucr opened a pull request: https://github.com/apache/activemq-artemis/pull/2420 Allow configuration of RMI registry port Previously the port was always random. This caused problems with remote JMX connections that needed to overcome firewalls. As of this patch, it's possible to make the RMI port static and whitelist it in the firewall settings. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jsmucr/activemq-artemis master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2420.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 #2420 commit 44b7a13f2f50ddc156eb51f9c11dc083da4a31dc Author: Å mucr Jan Date: 2018-11-07T13:49:23Z Allow configuration of RMI registry port Previously the port was always random. This caused problems with remote JMX connections that needed to overcome firewalls. As of this patch it's possible to make the RMI port static and whitelist it in the firewall settings. ---
[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...
Github user mnovak1 commented on the issue: https://github.com/apache/activemq-artemis/pull/2287 @TomasHofman Could you update the state of this PR, please? ---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user gemmellr commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 I tried the tests and they worked, but it occurred to me that though changes might be needed in general to fix whats currently stopping the tests passing, just making them pass is likely masking another issue that is the actual problem. I had a look what one of the tests is actually doing with JMSXGroupSeq given it worked before and didnt now. The answer seems to be nothing at all, which is somewhat expected given its not testing that. As such the failure to handle JMSXGroupSeq properly isnt the main issue, the fact its present at all would be. If nothing set it then it shouldn't be present. With this change in place I can see the received AMQP message in one of the previously-failing tests does have a group sequence value (of 0) populated when it seemingly should not. More of an aside, I'm not hugely fond of 'getValidatedUserId' as a name since its not clear anything has actually validated it. The various setters added to AMQPMessage perhaps also open up scope for protocol violations (since they are all immutable properties). ---
[GitHub] activemq pull request #317: AMQ-7082 We should make sure that pages that is ...
Github user asfgit closed the pull request at: https://github.com/apache/activemq/pull/317 ---
[GitHub] activemq-artemis pull request #2419: ARTEMIS-2100 address routing-type overr...
GitHub user franz1981 reopened a pull request: https://github.com/apache/activemq-artemis/pull/2419 ARTEMIS-2100 address routing-type overridden on attaching AMQP sender Added docs to explain the behaviour of management addresses on paging You can merge this pull request into a Git repository by running: $ git pull https://github.com/franz1981/activemq-artemis ARTEMIS-1710_docs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2419.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 #2419 commit 9016517540940773a5ceaa5709fac357d9edcb25 Author: Francesco Nigro Date: 2018-11-06T13:52:57Z ARTEMIS-2100 address routing-type overridden on attaching AMQP sender Added docs to explain the behaviour of management addresses on paging ---
[GitHub] activemq-artemis issue #2419: ARTEMIS-2100 address routing-type overridden o...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2419 Wrong jira number :P ---
[GitHub] activemq-artemis pull request #2419: ARTEMIS-2100 address routing-type overr...
Github user franz1981 closed the pull request at: https://github.com/apache/activemq-artemis/pull/2419 ---
[GitHub] activemq-artemis pull request #2419: ARTEMIS-2100 address routing-type overr...
GitHub user franz1981 opened a pull request: https://github.com/apache/activemq-artemis/pull/2419 ARTEMIS-2100 address routing-type overridden on attaching AMQP sender Added docs to explain the behaviour of management addresses on paging You can merge this pull request into a Git repository by running: $ git pull https://github.com/franz1981/activemq-artemis ARTEMIS-1710_docs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2419.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 #2419 commit 9016517540940773a5ceaa5709fac357d9edcb25 Author: Francesco Nigro Date: 2018-11-06T13:52:57Z ARTEMIS-2100 address routing-type overridden on attaching AMQP sender Added docs to explain the behaviour of management addresses on paging ---
