[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/2287
  
No, if `lockAcquisitionTimeout` is set to -1, it will wait 2 seconds, and 
then retry the lock. I don't see what you are referring to.

There are two situations:
* `tryLock(pos)` returns null (= lock is already taken), *this hasn't 
changed*.
* `tryLock(pos)` throws IOException:
  * If `lockAcquisitionTimeout == -1` it will wait 2 seconds and then 
retry, forever.
  * If `lockAcquisitionTimeout != -1`, it will wait 2 seconds or remaining 
time until timeout and retry, or if remaining time is <= 0 exception is thrown.


---

[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2287#discussion_r240294762
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
 ---
@@ -49,6 +49,8 @@
 
private static final byte NOT_STARTED = 'N';
 
+   private static final long LOCK_ACCESS_FAILURE_WAIT_TIME = 2000;
--- End diff --

I added another test to verify that `lockAcquisitionTimeout` is honored.

The only hard waiting time is 500ms on line 313, but that's the original 
behavior. I can make that honor `lockAcquisitionTimeout` as well if desired.


---

[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2287#discussion_r240293147
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
 ---
@@ -49,6 +49,8 @@
 
private static final byte NOT_STARTED = 'N';
 
+   private static final long LOCK_ACCESS_FAILURE_WAIT_TIME = 2000;
--- End diff --

@clebertsuconic no, the `lockAcquisitionTimeout` is not ignored.

If you check the line 337, actual waiting time is either 
`LOCK_ACCESS_FAILURE_WAIT_TIME` or time remaining until 
`lockAcquisitionTimeout` runs out, whichever is lower, so acquisition timeout 
set by user is honored.


---

[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/2287
  
@jbertram @franz1981 we could really use this...


---

[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/2287
  
Everything looks OK now, so I will wait for merging and porting to 2.6.x, 
and then will update downstream PR for 2.6.3.jbossorg-x (will cherry-pick -x 
from 2.6.x port).


---

[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/2287
  
No automation? Thanks, will do. :)


---

[GitHub] activemq-artemis issue #2287: ARTEMIS-2069 Backup doesn't activate after sha...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/2287
  
Retest this please.


---

[GitHub] activemq-artemis pull request #2287: ARTEMIS-2069 Backup doesn't activate af...

[TomasHofman]

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

[TomasHofman]

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

[TomasHofman]

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 #2287: ARTEMIS-2069 Backup doesn't activate af...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

ARTEMIS-2069 Backup doesn't activate after shared store is reconnected

https://issues.apache.org/jira/browse/ARTEMIS-2069
https://issues.jboss.org/browse/WFLY-10968
https://issues.jboss.org/browse/JBEAP-15343

Fix tries to prevent a server activation thread from terminating when 
FileLockNodeManager.tryLock() throws an IOException, e.g. because temporarily 
inaccessible NFS directory.
The node manager will repeat tryLock() call every two seconds.
WARN message with stack trace will be printed on first failure, DEBUG 
messages will be printed on recurring failures.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis 
JBEAP-14032-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2287.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 #2287


commit 162e71816e54137534c0bc8b2c3d6c85f941917d
Author: Tomas Hofman 
Date:   2018-09-03T13:47:03Z

ARTEMIS-2069 Backup doesn't activate after shared store is reconnected




---

[GitHub] activemq-artemis issue #1987: ARTEMIS-1781 Connector parameters not backward...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/1987
  
@clebertsuconic I added my attempt at compatibility test.


---

[GitHub] activemq-artemis issue #1987: ARTEMIS-1781 Connector parameters not backward...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/1987
  
@clebertsuconic could you point me to where you keep similar tests? So far 
I have a deal with EAP QE that they would create it for EAP.


---

[GitHub] activemq-artemis pull request #1987: [ENTMQBR-1034] Connector parameters not...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

[ENTMQBR-1034] Connector parameters not backward compatible

https://issues.jboss.org/browse/ENTMQBR-1034

Fixing backward compatibility with HornetQ:
* Translate transport parameters to dash notation instead of camel-case 
when talking to legacy clients.
* Do not add passwordCodec parameter unless maskPassword is true.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis 
JBEAP-14190-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1987.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 #1987


commit a09b374b8613f3216b9d683f4bfb2e4a9e62e463
Author: Tomas Hofman <thofman@...>
Date:   2018-03-28T13:07:21Z

[ENTMQBR-1034] Connector parameters not backward compatible




---

[GitHub] activemq-artemis pull request #1311: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1311#discussion_r119632869
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -643,7 +643,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
--- End diff --

OK, 1.x and 1.5.5-x PRs should be good now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1311: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

ARTEMIS-1187 Incompatible version when recreating a session with olde…

…r server

https://issues.apache.org/jira/browse/ARTEMIS-1187
https://issues.jboss.org/browse/JBEAP-9522

Upstream PR: https://github.com/apache/activemq-artemis/pull/1298

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis JBEAP-9522-1.x

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1311.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 #1311


commit e6e366872252f80ce36d063925f7961e0a2015ee
Author: Tomas Hofman <thof...@redhat.com>
Date:   2017-05-29T10:27:09Z

ARTEMIS-1187 Incompatible version when recreating a session with older 
server




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1309: ARTEMIS-1186 Consumer.receive hangs if ...

[TomasHofman]

Github user TomasHofman closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119549290
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

Thanks, I'm glad for feedback, I might have also be wrong...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1309: ARTEMIS-1186 Consumer.receive hangs if ...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

ARTEMIS-1186 Consumer.receive hangs if http acceptor with non-zero ba…

…tch-delay is configured

https://issues.apache.org/jira/browse/ARTEMIS-1186

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis JBEAP-6570-1.x

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1309.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 #1309


commit 9085b6d617d9f3445a24b5a19ecf4e27519995a8
Author: Tomas Hofman <thof...@redhat.com>
Date:   2017-05-29T10:00:32Z

ARTEMIS-1186 Consumer.receive hangs if http acceptor with non-zero 
batch-delay is configured




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119296506
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

The ```newCreateSession()``` is only called in ```recreateSession()``` 
method, so my reasoning was that the message will be sent to the same server 
that the client was previously connected to (is that correct?), so the 
serverVersion that was previously established can be reused.

The alternative could be to start a cycle of CreateSessionMessages with the 
highest know version and decrease the version if 
```ActiveMQIncompatibleClientServerException``` is returned? I think this is 
how establishing new connection works.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

ARTEMIS-1187 Incompatible version when recreating a session with olde…

…r server

https://issues.apache.org/jira/browse/ARTEMIS-1187
https://issues.jboss.org/browse/JBEAP-9522

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis JBEAP-9522

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1298.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 #1298


commit c3871a183f48b6cccaa3a65e7c14b2b64e16b39d
Author: Tomas Hofman <thof...@redhat.com>
Date:   2017-05-29T10:27:09Z

ARTEMIS-1187 Incompatible version when recreating a session with older 
server




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1297: ARTEMIS-1186 Consumer.receive hangs if ...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

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

ARTEMIS-1186 Consumer.receive hangs if http acceptor with non-zero ba…

…tch-delay is configured

https://issues.apache.org/jira/browse/ARTEMIS-1186
https://issues.jboss.org/browse/JBEAP-6570

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis JBEAP-6570

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1297.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 #1297


commit ae9faac09ee65cd3dc657aedd23ac36f423f5a19
Author: Tomas Hofman <thof...@redhat.com>
Date:   2017-05-29T10:00:32Z

ARTEMIS-1186 Consumer.receive hangs if http acceptor with non-zero 
batch-delay is configured




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #849: ARTEMIS-805 old JMS clients failing on new brok...

[TomasHofman]

Github user TomasHofman commented on the issue:

https://github.com/apache/activemq-artemis/pull/849
  
@clebertsuconic will this get into 1.4 branch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #849: ARTEMIS-805 old JMS clients failing on n...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/849#discussion_r84056887
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -290,9 +292,19 @@ public int getServerVersion() {
 
@Override
public ClientSession.AddressQuery addressQuery(final SimpleString 
address) throws ActiveMQException {
-  SessionBindingQueryResponseMessage_V3 response = 
(SessionBindingQueryResponseMessage_V3) sessionChannel.sendBlocking(new 
SessionBindingQueryMessage(address), PacketImpl.SESS_BINDINGQUERY_RESP_V3);
-
-  return new AddressQueryImpl(response.isExists(), 
response.getQueueNames(), response.isAutoCreateJmsQueues(), 
response.isAutoCreateJmsTopics());
+  if (sessionChannel.supports(PacketImpl.SESS_BINDINGQUERY_RESP_V3)) {
--- End diff --

I checked and failing tests are passing on my local machine when using 
original commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #849: ARTEMIS-805 old JMS clients failing on n...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/849#discussion_r84055604
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -290,9 +292,19 @@ public int getServerVersion() {
 
@Override
public ClientSession.AddressQuery addressQuery(final SimpleString 
address) throws ActiveMQException {
-  SessionBindingQueryResponseMessage_V3 response = 
(SessionBindingQueryResponseMessage_V3) sessionChannel.sendBlocking(new 
SessionBindingQueryMessage(address), PacketImpl.SESS_BINDINGQUERY_RESP_V3);
-
-  return new AddressQueryImpl(response.isExists(), 
response.getQueueNames(), response.isAutoCreateJmsQueues(), 
response.isAutoCreateJmsTopics());
+  if (sessionChannel.supports(PacketImpl.SESS_BINDINGQUERY_RESP_V3)) {
--- End diff --

```sessionChannel.supports()``` doesn't give correct results, I think I was 
trying that. The problem was that ```connection.getClientVersion()``` returned 
zero in ```ChannelImpl.supports()```, hence I was comparing version numbers 
directly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #791: ARTEMIS-747 multiple CDATA events on imp...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/791#discussion_r80201908
  
--- Diff: 
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataImporter.java
 ---
@@ -444,33 +444,59 @@ private void processMessageBody(Message message) 
throws XMLStreamException, IOEx
  }
   }
   reader.next();
+  ActiveMQServerLogger.LOGGER.debug("XMLStreamReader impl: " + reader);
   if (isLarge) {
  tempFileName = UUID.randomUUID().toString() + ".tmp";
  ActiveMQServerLogger.LOGGER.debug("Creating temp file " + 
tempFileName + " for large message.");
  try (OutputStream out = new FileOutputStream(tempFileName)) {
-while (reader.hasNext()) {
-   if (reader.getEventType() == 
XMLStreamConstants.END_ELEMENT) {
-  break;
-   }
-   else {
-  String characters = new 
String(reader.getTextCharacters(), reader.getTextStart(), 
reader.getTextLength());
-  String trimmedCharacters = characters.trim();
-  if (trimmedCharacters.length() > 0) { // this will skip 
"indentation" characters
- byte[] data = decode(trimmedCharacters);
- out.write(data);
-  }
-   }
-   reader.next();
-}
+getMessageBodyBytes(new MessageBodyBytesProcessor() {
+   @Override
+   public void processBodyBytes(byte[] bytes) throws 
IOException {
+  out.write(bytes);
+   }
+});
  }
  FileInputStream fileInputStream = new 
FileInputStream(tempFileName);
  BufferedInputStream bufferedInput = new 
BufferedInputStream(fileInputStream);
  ((ClientMessage) message).setBodyInputStream(bufferedInput);
   }
   else {
- reader.next(); // step past the "indentation" characters to get 
to the CDATA with the message body
- String characters = new String(reader.getTextCharacters(), 
reader.getTextStart(), reader.getTextLength());
- message.getBodyBuffer().writeBytes(decode(characters.trim()));
+ getMessageBodyBytes(new MessageBodyBytesProcessor() {
+@Override
+public void processBodyBytes(byte[] bytes) throws IOException {
+   message.getBodyBuffer().writeBytes(bytes);
+}
+ });
+  }
+   }
+
+   /**
+* Message bodies are written to XML as Base64 encoded CDATA elements. 
Some parser implementations won't read the
+* entire CDATA element at once (e.g. Woodstox) so it's possible for 
multiple CDATA events to be combined into a
+* single Base64 encoded string.  You can't decode bits and pieces of 
each CDATA.  Each CDATA has to be decoded in
+* its entirety.
+*
+* @param processor used to deal with the decoded CDATA elements
+* @throws IOException
+* @throws XMLStreamException
+*/
+   private void getMessageBodyBytes(MessageBodyBytesProcessor processor) 
throws IOException, XMLStreamException {
+  int currentEventType;
+  StringBuilder cdata = new StringBuilder();
+  while (reader.hasNext()) {
+ currentEventType = reader.getEventType();
+ if (currentEventType == XMLStreamConstants.END_ELEMENT) {
+break;
+ }
+ // when we hit a CHARACTERS event we know that the entire CDATA 
is complete so decode and pass back to the processor
+ else if (currentEventType == XMLStreamConstants.CHARACTERS && 
cdata.length() > 0) {
--- End diff --

JDK's STAX implementation reports CDATA sections also as 
XMLStreamConstants.CHARACTERS (instead of XMLStreamConstants.CDATA like 
Woodstox do) by default. So this condition needs to check that the text 
contains white spaces only. Probably reader.isWhiteSpace() should do.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---