[GitHub] activemq-artemis issue #2038: ARTEMIS-1828 CLI option for queue's routing-ty...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2038 @rakstar Looks good, was about to merge this (sorry been a bit busy with broker fixes), unfortunately theres a merge conflict now, are you able to rebase / resolve? ---
[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2041 @gaohoward sorry taken a while to look at this as have been burning hours on other issue. From what i can tell from the JS this looks ok, i assume form reading the js, the idea here is more clearly show who is live and who is a slave, calling a method to deduce this, rather than the hardcoded master=true, is this roughly a correct understanding? If so +1 from me. ---
[GitHub] activemq-artemis issue #2374: ARTEMIS-2127 Add auth details to consumer crea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2374 LGTM, as no other review comments (its been 23 days), merging. ---
[GitHub] activemq-artemis pull request #2374: ARTEMIS-2127 Add auth details to consum...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2374 ---
[GitHub] activemq-artemis pull request #2410: ARTEMIS-2125 Tabs preference changes to...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2410 ---
[GitHub] activemq-artemis issue #2410: ARTEMIS-2125 Tabs preference changes to displa...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2410 thanks @feuillemorte for the contribution, and thanks @tadayosi for reviewing. merging now. ---
[GitHub] activemq-artemis pull request #2419: ARTEMIS-1710 Allow for management msgs ...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2419 ---
[GitHub] activemq-artemis issue #2419: ARTEMIS-1710 Allow for management msgs to exce...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2419 @franz1981 nice, we like docs :) ---
[GitHub] activemq-artemis pull request #2413: NO-JIRA fixing tests
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2413 ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 alternative option 1 - https://github.com/apache/activemq-artemis/pull/2423 ---
[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2423 @tabish121 one of the alternative approaches, where when this feature is enabled, we simply convert to CORE, this then avoids making any changes to AMQPMessage (keeping untouched) ---
[GitHub] activemq-artemis pull request #2423: ARTEMIS-2168 Fix "populate-validated-us...
GitHub user michaelandrepearce opened a pull request: https://github.com/apache/activemq-artemis/pull/2423 ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages When populate-validated-user is true, force conversion to CORE as AMQP messages are immutable. Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages. Add additional test cases to cover AMQP for feature. You can merge this pull request into a Git repository by running: $ git pull https://github.com/michaelandrepearce/activemq-artemis ARTEMIS-2168-v2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2423.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 #2423 commit cce891325f72fc921ff964440861432b2f320fb3 Author: Michael André Pearce Date: 2018-11-06T21:30:52Z ARTEMIS-2142 Fix ServerJMSMessage Fix ServerJMSMessage so it correctly transposes JMSX headers. Push common JMS to Message Interface mappings into MessageUtil commit 213f32d70f7f1c2e14686341a581727875e40dc3 Author: Michael André Pearce Date: 2018-11-08T22:18:29Z ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages When populate-validated-user is true, force conversion to CORE as AMQP messages are immutable. Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages. Add additional test cases to cover AMQP for feature. ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 @tabish121 im open to suggestion on how to implement this in a way thats more amenable to yourself. Other ideas i have: 1) Introduce a broker flag that specially and clearly named something like "allow-amqp-modfication-spec-break" then allows AMQP spec break, so its so clear that we're enabling a spec break. 2) If the populate-validated-user is set, then a or copy of the message is made. 3) Another idea, is there a tamper flag on AMQP spec? If so could we can set that? basically allowing you to say the message was mutated via the broker (and even possible say what was mutated (in this case user)). ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 @tabish121 ok, so the use case here, is per https://issues.apache.org/jira/browse/ARTEMIS-584 essentially you cannot/do not trust the user sent on the message, you only trust the user used during auth to the broker, this is quite important for an audit requirement. In this case as i noted as a user you are explicitly saying you wish to violate spec and modify the message. whilst i agree by default the broker should NOT break any specs, there does need ability to violate/or override for a feature, this would be on par with some of the FQQN stuff that allows users to violate some bits in JMS, (e.g. get a JMS Queue actually bound to a JMS Topic subscription), but you do this explicitly knowing this. ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user tabish121 commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 @michaelandrepearce I may have worked due to a bug that allowed the broker to violate the AMQP specification but was fixed later (https://issues.apache.org/jira/browse/ARTEMIS-1092) and should continue to not allow the broker to violate the AMQP 1.0 specification. ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 @tabish121 this used to work. Also note this is only when the toggle populate-validated-user is enabled, which is off by default. ---
[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...
Github user tabish121 commented on the issue: https://github.com/apache/activemq-artemis/pull/2422 -1 The properties section of an AMQP message is immutable and cannot be changed by the broker as per the AMQP 1.0 specification (Section 3.2) http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#section-message-format ---
[GitHub] activemq-artemis pull request #2422: ARTEMIS-2168 Fix "populate-validated-us...
GitHub user michaelandrepearce opened a pull request: https://github.com/apache/activemq-artemis/pull/2422 ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages. Add additional test cases to cover AMQP for feature. Add fix to AMQP message This PR builds on top of other fixes currently in https://github.com/apache/activemq-artemis/pull/2418 as such either this PR can be merged which includes both fixes, or if the other PR is merged first, this PR will need a rebase on that. You can merge this pull request into a Git repository by running: $ git pull https://github.com/michaelandrepearce/activemq-artemis ARTEMIS-2168 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2422.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 #2422 commit cce891325f72fc921ff964440861432b2f320fb3 Author: Michael André Pearce Date: 2018-11-06T21:30:52Z ARTEMIS-2142 Fix ServerJMSMessage Fix ServerJMSMessage so it correctly transposes JMSX headers. Push common JMS to Message Interface mappings into MessageUtil commit 2682443247a30274c673f52f645de87667c759dd Author: Michael André Pearce Date: 2018-11-08T22:18:29Z ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages. Add additional test cases to cover AMQP for feature. Add fix to AMQP message ---
[GitHub] activemq-artemis pull request #2387: ARTEMIS-2142 Support JMSXGroupSeq -1to ...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2387#discussion_r231956436
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
---
@@ -289,6 +289,12 @@ public SimpleString getGroupID() {
return this.getSimpleStringProperty(Message.HDR_GROUP_ID);
}
+ @Override
+ public int getGroupSequence() {
+ final Integer integer =
this.getIntProperty(Message.HDR_GROUP_SEQUENCE);
--- End diff --
The regression introduced new, is fixed by
https://github.com/apache/activemq-artemis/pull/2418
So the new regressions will go if/when that pr is merged
---
[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2418 Just a note, not related to fixing the current sequence issue. But if we left the setters in they would fix a regression in feature around enforced broker side user, that was caused with the native AMQP message support. ---
[GitHub] activemq-artemis pull request #2387: ARTEMIS-2142 Support JMSXGroupSeq -1to ...
Github user jdanekrh commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2387#discussion_r231901536
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
---
@@ -289,6 +289,12 @@ public SimpleString getGroupID() {
return this.getSimpleStringProperty(Message.HDR_GROUP_ID);
}
+ @Override
+ public int getGroupSequence() {
+ final Integer integer =
this.getIntProperty(Message.HDR_GROUP_SEQUENCE);
--- End diff --
This just blew up for me. It is not Mike's bug per se, it just possibly
sends execution to this little function which cannot handle `null`s
https://github.com/apache/activemq-artemis/blob/10ecb358cb587c8417deeb799723d8274b0a44a5/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java#L211-L214
In my JVM,
```
public static Integer valueOf(String var0) throws NumberFormatException
{
return parseInt(var0, 10);
}
```
and
```
public static int parseInt(String var0, int var1) throws
NumberFormatException {
if (var0 == null) {
throw new NumberFormatException("null");
```
Here is a failing test (from cli-java project) on travis ci,
https://travis-ci.org/rh-messaging/cli-java/jobs/451880325 at around
> 13:28:41,911 ERROR Error while sending a message!
> java.lang.NumberFormatException: null
It was not supposed to fetch a 2.7.0 snapshot of artemis-cli-java, but
somehow it did... mystery for the next time, for me or @michalxo.
---
[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...
Github user TomasHofman commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2287#discussion_r231878791
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
---
@@ -299,44 +303,52 @@ 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;
+ try {
+FileLock lock = tryLock(lockPosition);
+isRecurringFailure = false;
+
+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");
+ }
+} else {
+ return lock;
}
-
-if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
- throw new Exception("timed out waiting for 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;
+
+long waitTime = LOCK_ACCESS_FAILURE_WAIT_TIME;
+if (lockAcquisitionTimeout != -1) {
+ final long remainingTime = lockAcquisitionTimeout -
(System.currentTimeMillis() - start);
+ if (remainingTime <= 0) {
+ throw new Exception("timed out waiting for lock");
+ }
+ waitTime = Collections.min(Arrays.asList(waitTime,
remainingTime));
--- End diff --
Good point, updated.
---
[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_r231870842
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
---
@@ -299,44 +303,52 @@ 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;
+ try {
+FileLock lock = tryLock(lockPosition);
+isRecurringFailure = false;
+
+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");
+ }
+} else {
+ return lock;
}
-
-if (lockAcquisitionTimeout != -1 &&
(System.currentTimeMillis() - start) > lockAcquisitionTimeout) {
- throw new Exception("timed out waiting for 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;
+
+long waitTime = LOCK_ACCESS_FAILURE_WAIT_TIME;
+if (lockAcquisitionTimeout != -1) {
+ final long remainingTime = lockAcquisitionTimeout -
(System.currentTimeMillis() - start);
+ if (remainingTime <= 0) {
+ throw new Exception("timed out waiting for lock");
+ }
+ waitTime = Collections.min(Arrays.asList(waitTime,
remainingTime));
--- End diff --
`Math.min` will do the same as `Collections.min(Arrays.asList(waitTime,
remainingTime));`
---
[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...
Github user TomasHofman commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2287#discussion_r231830652
--- 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 --
Modified to exit if timeout already reached, and don't sleep longer then
remaining time to timeout.
---
[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...
Github user TomasHofman commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2287#discussion_r231822854
--- 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 --
Now when I look at this again, this whole second loop in the _original
code_ doesn't make sense - the only way execution could get here is when the
```interrupted``` flag was set to true, in which case we should exit
immediately. The comment mentions "deadlock exception", but any exception in
the first loop would terminate the method.
I'm gonna remove this second loop altogether.
---
[GitHub] activemq-artemis pull request #2421: [ARTEMIS-2166]Unable to delete queue wi...
GitHub user shailendra14k opened a pull request:
https://github.com/apache/activemq-artemis/pull/2421
[ARTEMIS-2166]Unable to delete queue with single quote from console
JIRA:- https://issues.apache.org/jira/browse/ARTEMIS-2166
I assume a single quote(') is not a reserved character so queue name with
the single quote is valid. Currently, the replace logic is only applied to the
console; I think it's not required. If yes then we will have to apply the same
replace logic for address and CLI delete command.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shailendra14k/activemq-artemis ARTEMIS-2166
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2421.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 #2421
commit 6b8f136154b0482be5dc82e39a0815fcc520a192
Author: Shailendra Kumar Singh
Date: 2018-11-08T03:58:48Z
[ARTEMIS-2166]Unable to delete queue with single quote from console
---
