Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh, at this point there are lots of conflicts on this PR. They
either need to be fixed and this PR can move forward or it needs to be closed.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@clebertsuconic @RaiSaurabh @michaelandrepearce
**Re performances**
I've the first results using just a small benchmark with 1 producer/1
consumer using 100 bytes messages.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce There is another testsuite just for openwire.
somethings that we inherited from activemq5.. it's under
./tests/activemq5-unit-tests
that one was
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
i Got a lot of test failures previous, but it seemed a lot better when I
ran a suite a few weeks back, is that since his latest changes and work that
you ran it? Iâm just
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
this is a nice start.. but it will require some work... a lot of tests are
broken... the converters need to be updated... (especially on the openwire
testsuite).
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@clebertsuconic you able to give @raisaurabh a pr with what you mean.
Iâve gone though it myself and in general looks fine from what I can tell, eg
it has a check to see if
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
There should be some simplification on the converters here. They shouldn't
be needed at all when talking openwire <-> openwire. I saw some issues when I
was looking into this.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh have sent a PR to your branch, that solves some bits for you.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh thanks, thats making my life easier, just looking at this atm,
if you hadn't guessed :)
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Ok @michaelandrepearce .
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh could you rebase your branch on current master then, for me?
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce Yes, I did.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh im struggling to rebase this on top of master. Did you rebase
after @franz1981's work?
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@raisaurabh great stuff, if I get a chance Iâll try have a look. If not
hopefully Clebert will be able to.
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce @clebertsuconic Apologies for delaying this and
dragging this long. I tried to fix the code to pass all the test cases of
OpenWire but still, I get 5 failed test
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh, the Travis CI issues should be resolved. Can you please
rebase this PR and push -f? Thanks!
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh I've just checked vs the `integration/openwire` tests in
`integration-tests` and it fail 22 tests/338.
The original number of failing tests here is 1, so anything over
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce I have implemented the changes asked and also checked
that this persists to disk and is recoverable on restart of the broker without
converting to the core message.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Seems some issue with Travis and Jenkins for this project is disabled.
---
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
for this kind of big PR to be merged, we need the full testsuite to be
passing.
Not talking about the PR testsuite.
There's an issue that we don't have an env at
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Really need PR build to be green. Maybe re-push with a comment update to
get try trigger another build.
Seems some methods have been still left in-implemented. Also
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh @michaelandrepearce
I've run a couple of benchmarks and the results are strange, perf wire the
improvements are on par or (slightly) less than
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh I'm doing some preliminary benchmarks to see if the number of
buffer copies are decreased :+1:
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@franz1981 I have implemented the changes @michaelandrepearce requested.
@michaelandrepearce could you please review it.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh Please check the @michaelandrepearce comments about this PR
first :+1:
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce Looks like another issue with GIT repo. Could you
please check.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Here is a much more complete sample, i would be expecting to see (this is
what i was meaning in the original PR comment on: #1793)
```
/**
* Licensed to the
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Copied from example i gave originally:
https://github.com/apache/activemq-artemis/pull/1793
e.g. along this line.
public class OpenwireMessage extends
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh @franz1981 I just got some perf results back on this, and its
not pretty. (i retract my prev comments (i deleted)
After looking again it seems like all this
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@franz1981 im happy to do the merge, but i would like a thumbs up from you
before i do.
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh looks really good to me, as noted by @franz1981 there is quite
a few bits to optimise, but i personally would prefer we merge this (as long as
no functional
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Thanks, @michaelandrepearce @franz1981 for the review. I have implemented
the changes.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce I will warn the PM squad :+1:
---
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@franz1981 I think thereâs a general Jenkins PR build issue. If you have
direct ping ability or nudge directly Martyn or Clebert or someone on PM ?
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@RaiSaurabh Please rebase it and force push it to re-trigger a Jenkins
build and hopefully it will pass
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce Seems there is some problem with the Git due to which
this build failed. Could you please check and let me know.
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
@michaelandrepearce updated the code as per your comments.
---
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Thanks, @michaelandrepearce @clebertsuconic and @franz1981 for the review
comments. I have tried to fix it. Could you please review the changes now.
---
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 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
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
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 RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Ok, @franz1981 I will wait for the merging.
---
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1793
Please check this too https://github.com/apache/activemq-artemis/pull/1786
Probably will be easier to wait mine to be merged: the changes I've pushed
are including some refactoring
44 matches
Mail list logo
