Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic can you take a look at the additional commit ?
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1803
---
GitHub user gaohoward reopened a pull request:
https://github.com/apache/activemq-artemis/pull/1800
ARTEMIS-1626 Disable thread leak check for failing tests
The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
reopen for further discussion.
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic
regarding the thread leak logging, I don't think I got it right. Do you
mean if test failed, we give the thread logging, what about test that pases?
(just keep in mi
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
Right now it fails the test. If you changed it to log only if the test
failed. You got what you wanted. And what we wanted. Itâs a good
compromise.
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
I think logging would eventually turn on the thread checking, that's the
original behavior, no need to change anything, right?
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
Please donât close those. Just change it to log.
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1803
I can merge this.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
All we asked was logging instead.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
Why close it instead of logging it ?
---
Github user gaohoward closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1800
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
ok, I think it's better to take cautions as people worries about it.
Thanks you guys for the opinions !
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
@clebertsuconic Np I've understood what you mean :)
I've pushed another commit with another solution less "reactive" but more
simlar to the original code: let me know if it seems bet
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1799
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1786
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
@franz1981 typo on my last message.. I actually meant the autoRead stuff.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
I ran the whole testsuite and it didn't complete.
it's a very nice start though.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
@franz1981 I told you it was ok to do it this way.. but I'm a bit concerned
by the use of setResponse(true) and false through this.
is there a way to return the packet like
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1805
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1802
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1804
---
GitHub user clebertsuconic reopened a pull request:
https://github.com/apache/activemq-artemis/pull/1803
ARTEMIS-1628 Limit pool size on artemis journal
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-arte
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/1805
ARTEMIS-609 fix interceptor XML docs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-609
Alternat
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1801#discussion_r163065589
--- Diff:
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
---
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/1804
ARTEMIS-608 document adding runtime deps
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-608
Alte
Github user mtaylor commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1783#discussion_r163036074
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/transformer/ServerMessageImpl.java
---
@@ -0,0 +1,190 @@
+/*
Github user mtaylor commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1783#discussion_r163035962
--- Diff:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
---
@@ -23,4 +23,7 @@
@Deprecated
public
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
The errors on the completion are critical. They are IOerrors. I think the
critical exception will be called anyways.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
@gtully Good point! I've added an async send of error that finally will end
with the critical error as before: I hope (@clebertsuconic could help confirm
that) that it is safe enough.
Github user clebertsuconic closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1803
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1803
Please do not merge this.. i will add a log.warn if -1
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward I undestand your points you've showed on my questions, but as
@clebertsuconic has pointed
> the ideal world is far from reality.. there are tests that will be
failin
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1786#discussion_r163012542
--- Diff:
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
---
@@ -361,13 +371,68 @@ public int ha
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/1803
ARTEMIS-1628 Limit pool size on artemis journal
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemi
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1783#discussion_r163011302
--- Diff:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
---
@@ -23,4 +23,7 @@
@Deprecated
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1783#discussion_r163010905
--- Diff:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TypedProperties.java
---
@@ -23,4 +23,7 @@
@Deprecated
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1791
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1786
@franz1981 nudge, only thing holding me from merging this is the number of
commits.
I guess really should be a @clebertsuconic nudge if he's asked for this on
purpose.
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1792
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Really good work, I like this, really good to see OpenWire getting this :)
Have you run the full OpenWire test suite? I know not all run in the PR
build, so worth check
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1798
---
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1793#discussion_r163007469
--- Diff:
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenwireConverter.ja
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1796
Could the logging events, have LOG codes, e.g. like ActiveMQServerLogger
---
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1795
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1795
Thanks.
---
GitHub user tabish121 opened a pull request:
https://github.com/apache/activemq-artemis/pull/1802
ARTEMIS-1504 Update Qpid JMS to 0.29.0 and proton-j to 0.25.0
Updates to latest Qpid JMS and the latest Proton-J release
You can merge this pull request into a Git repository by running
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1796
Couple of things:
- Nice work!
- It's not clear to me why some things are logged at DEBUG vs. INFO.
Perhaps the documentation could address this.
- Since you're using the
Github user gtully commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
think any error from the io completion callback should result in
connection.sendException(...);
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward the idea world is far from reality.. there are tests that will
be failing forever on the testsuite and people will ignore them.. a result will
be a mess that people won't
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic re: "test1 failed.. leaked.. than test2 failed because
there's a thread leak... if you don't report the leak from 1.. you won't know
that test2 could fail later.."
I
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward as I said.. I have been the one to fix these freaking issues
when the testsuite is acting up... so... I would be okay with your changes if
you at least logged the leakage
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
> @clebertsuconic I'm not convinced that it is that important to log a
leakage on a failed test. If a test
> fails it should be fixed. With this PR you won't miss a real leakage.
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@franz1981 If the test fails, whether cleanly or not, we get reported and
should fix it. The leakage is not important here. After you fix it and it begin
to pass, the thread check rule w
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Please take a look here:
```
2018-01-22 17:28:59,899 WARN [org.apache.activemq.artemis.core.server]
Error during message dispatch: java.lang.RuntimeException: null
at
or
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic I'm not convinced that it is that important to log a
leakage on a failed test. If a test fails it should be fixed. With this PR you
won't miss a real leakage.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward
I understand, but often a failed test is expected to fail cleanly (without
having leaking threads): having that information ignored could affect anyway
the other tests an
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward I'm ok on not reporting a failure.. but it should always log a
leakage.. even for failed tests..
We could instead of failing on leakage.. log.warn instead af
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic @mtaylor @franz1981 We shouldn't disable the thread check
for all tests. What this PR do it selectively disable the check for failing
tests. If a test passes the check is
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@mtaylor the opposite.. we could have it enabled on the dev and test
profiles.. and have it off by default. just like checkstyle is off on dev
profile.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward test1 failed.. leaked.. than test2 failed because there's a
thread leak... if you don't report the leak from 1.. you won't know that test2
could fail later..
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic @gaohoward
Some time ago has happened that I've written a test that was failing and I
adming that having the thread leak check has helped me to find that a test
wasn
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
We already have a dev profile. Perhaps it could be disabled there.
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic I don't think the PR make the debugging of thread leaking
harder.
For example suppose we have 3 tests, test1, test2 and test3 running in this
order.
Let's say tes
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
How about we just have a system variable to switch off the thread check
rule for debug purposes. Default being on. -DapplyThreadCheckRule=true
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
so, I don't mean to be disrespectul.. your changes are nice technically..
but I don't want to lose visibility of leaking threads.. even if it's for a
failed test.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
a failing test leaking is a double failure... it failed and then leaked..
you can fix that situation with finally blocks...
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@gaohoward I have been there before... finding a test that's failing
because of a previous leak it's not fun... I don't want to back into that...
If a thread is leaking a t
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1801
Possibly I will add a test but is tricky and I need to run CI on it too
:+1:
---
GitHub user franz1981 opened a pull request:
https://github.com/apache/activemq-artemis/pull/1801
ARTEMIS-1607 OpenWire is sending responses too early with durable messages
AMQSession is sending response back without waiting past I/O tasks on
StorageManager to be finished
You can m
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
I think the more interesting case is that a 'passing' test that leaking
threads. If that's the case it's very hard to debug. But for a failing test,
leaking threads is not that meaningfu
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
We are supposed to write tests to cleanup resources.. even when it fails.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
failed tests are also supposed to cleanup their resources...
example
try {
}
finally {
close()
}
---
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
@clebertsuconic If a failing test causes thread leaking and causes other
tests to fail, the thread checking can't fix it right?
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Let me see if I understand.. you implemented OpenwireMessage here?
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1800
-1 I don't agree with this... a thread leaking on a failing test will make
another test to fail.
This is about raising awareness of leaked threads.
---
GitHub user gaohoward opened a pull request:
https://github.com/apache/activemq-artemis/pull/1800
ARTEMIS-1626 Disable thread leak check for failing tests
The ThreadLeakCheckRule is used to check thread leaks
after each test is finished. However when a test fails, it is
not n
GitHub user stanlyDoge opened a pull request:
https://github.com/apache/activemq-artemis/pull/1799
ARTEMIS-1625 fix moving messages
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stanlyDoge/activemq-artemis E986
Alternatively y
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1785
---
Github user gtully commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1785#discussion_r162907379
--- Diff:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/VirtualTopicToFQQNOpenWireTest.java
---
@@
Github user jostbg commented on the issue:
https://github.com/apache/activemq-artemis/pull/1775
@michaelandrepearce All thoughts and ideas are appreciated :-) I could
achieve what we need (despite it violates JMS spec) by changing the durable
flag of a QueueConfig in beforeCreateQueue
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1794
---
Github user andytaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1794
looks good will get it merged
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1775
On the note of broker plugin, you do realise you can intercept
beforeCreateAddress and beforeCreateQueue and a lot of other functions, this
way you could do what you want withou
Github user jmesnil commented on the issue:
https://github.com/apache/activemq-artemis/pull/1795
Commit amended with a test
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1775
As I stated there is already a global flag, if there is an address level
flag then this should have the exact same behaviour. Also Iâm not advocating
changing the global flag
Github user jostbg commented on the issue:
https://github.com/apache/activemq-artemis/pull/1775
@michaelandrepearce I cannot modify the durable flag of AMQP messages as
they are meant to be immutable and it would prevent using features like digital
signatures AFAIK. What we are lookin
87 matches
Mail list logo
