You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by michaelandrepearce <gi...@git.apache.org> on 2017/08/24 07:12:59 UTC
[GitHub] qpid-jms pull request #12: QPIDJMS-315: Add support for Netty KQueue transpo...
GitHub user michaelandrepearce opened a pull request:
https://github.com/apache/qpid-jms/pull/12
QPIDJMS-315: Add support for Netty KQueue transport
Add KQueue support
Also add safer epoll and kqueue env check to protect from lib loading failure (as found in apache activemq artemis)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/michaelandrepearce/qpid-jms QPIDJMS-315
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-jms/pull/12.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 #12
----
commit 03b593a3cdf99642885f0a9f7de8da359892ae3f
Author: Michael Andre Pearce <mi...@me.com>
Date: 2017-08-24T07:11:56Z
QPIDJMS-315: Add support for Netty KQueue transport
Add KQueue support
Also add safer epoll and kqueue env check to protect from lib loading failure (as found in apache activemq artemis)
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-jms/pull/12
> Agreed its not ideal we have to do this, but this was found on epoll in artemis (which you already have epoll implemented and defaulted to true) , as such until netty improve the check then this check is worth while for the interim unless you default epoll to false currently the client has the same issue with epoll as found - https://issues.apache.org/jira/browse/ARTEMIS-1099
Given it seems it only happens if you use a 32bit JVM and only emits a warning, I don't think its serious enough to introduce all that not-ideal code for the existing epoll bits when noone has yet complained about it with the client. This is however an opportunity to get it addressed in Netty instead where it seems like it belongs.
> The classifier for the dependency for kqueue implementation is for osx build, there isn't one currently for freeBSD or other derivaitves (osx being bsd based), as such the check for the platform being osx, to be safe. E.g. our case is we do have java clients that are on macOS.
I realise that the existing artifact is targetting OSX, but that could change and its just another ugly check for us to fall afoul of later. Netty better knows what the restrictions on its usage are than we do, it seems like its availability check should be handling such things.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on the issue:
https://github.com/apache/qpid-jms/pull/12
Agree with @gemmellr on this, the checks are better provided to Netty for inclusion in the next release so that they can be maintained and matured along with the Epoll and KQueue implementations. In the case where the client emits an unwanted warning for Epoll on 32bit arch we already have a simple workaround which is to provide the URI option to disable the Epoll bits from being used.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
> Given it seems it only happens if you use a 32bit JVM and only emits a warning, I don't think its serious enough to introduce all that not-ideal code for the existing epoll bits when noone has yet complained about it with the client.
fair enough, ill remove then, i thought it be good to protect qpid from this, we also haven't actually experienced this issue as running 64bit JVM;s just something i noted in the artemis project, This also will remove the other part for KQueue you also dislike.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-jms/pull/12
I don't think the various environment etc checking being done really belongs in the client, it seems like that stuff belongs in Netty if the isAvailable() checks it offers aren't actually sufficient. Similarly it feels a little odd making it check for being on a Mac when KQueue is really for BSD rather than OSX specific, and they might open up support for others later, again it feels like Netty is better placed to be checking that type of thing.
I also think that if added the KQueue bit would be better set off by default for now and flipped later on. It's very new in Netty itself, whereas the epoll stuff had time to mature before it was added. Linux is also probably the primary platform we test and use the client on, whereas we don't really test on OS X at all (though I shall change that shortly), so sticking to the NIO bits by default there seems wiser for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
@gemmellr fyi raised https://github.com/netty/netty/issues/7150 for underlying issue within netty project.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
@gemmellr ping - sorry for delay was on route home, didn't need to update test, forgot it was explicitly being turned on and off anyhow. as such should be good.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
So on this pr front:
Default kqueue is now false
Reverted out the extra check for lib loading, kqueue is relying like epoll on the similar isAvailable() from netty.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms pull request #12: QPIDJMS-315: Add support for Netty KQueue transpo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/qpid-jms/pull/12
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
I just have to update test case to enable kqueue on env with macOS as default is off now, and so for test I need to enable. Will update in a bit and nudge once done
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
Hi @gemmellr
> I don't think the various environment etc checking being done really belongs in the client, it seems like that stuff belongs in Netty if the isAvailable() checks it offers aren't actually sufficient.
Agreed its not ideal we have to do this, but this was found on epoll in artemis (which you already have epoll implemented and defaulted to true) , as such until netty improve the check then this check is worth while for the interim unless you default epoll to false currently the client has the same issue with epoll as found - https://issues.apache.org/jira/browse/ARTEMIS-1099
> Similarly it feels a little odd making it check for being on a Mac when KQueue is really for BSD rather than OSX specific, and they might open up support for others later, again it feels like Netty is better placed to be checking that type of thing.
The classifier for the dependency for kqueue implementation is for osx build, there isn't one currently for freeBSD or other derivaitves (osx being bsd based), as such the check for the platform being osx, to be safe. E.g. our case is we do have java clients that are on macOS.
> also think that if added the KQueue bit would be better set off by default for now and flipped later on. It's very new in Netty itself, whereas the epoll stuff had time to mature before it was added. Linux is also probably the primary platform we test and use the client on, whereas we don't really test on OS X at all (though I shall change that shortly), so sticking to the NIO bits by default there seems wiser for now.
Sure this make sense we can make it default to false.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on the issue:
https://github.com/apache/qpid-jms/pull/12
@michaelandrepearce
The Epoll warning has been reported to Netty a few days ago, see [Issue:7120](https://github.com/netty/netty/issues/7120) which was reported by someone using it on ARM. You might want to add onto that one your experiences with Epoll on Artemis and also with KQueue, you could even provide a PR to try and prompt them to fix it quicker.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/qpid-jms/pull/12
@tabish121 ah good spot def will link up the issues. And yeah I'll try to make a pr there.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org