[GitHub] activemq-artemis pull request #1093: ARTEMIS-994 Support Netty Native Epoll ...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1093: ARTEMIS-994 Support Netty Native Epoll ...

asfgit
GitHub user michaelandrepearce opened a pull request:

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

    ARTEMIS-994 Support Netty Native Epoll on Linux

    The following changes are made to support Epoll.
   
    * Refactored SharedNioEventLoopGroup into renamed SharedEventLoopGroup to be generic (as so we can re-use for both Nio and Epoll)
    * Add support and toggles for Epoll in NettyAcceptor and NettyConnector (with fall back to NIO if cannot load Epoll)
    * Removal from code of PartialPooledByteBufAllocator, caused bad address when doing native, and no longer needed - see jira discussion
   
    New Connector Properties:
   
    useEpoll - toggles to use epoll or not, default true (but we failback to nio gracefully)
    epollRemotingThreads = same behaviour as nioRemotingThreads but for Epoll.
    useEpollGlobalWorkerPool = same behaviour as useNioGlobalWorkerPool but for Epoll.
   
    New Acceptor Properties:
   
    useEpoll - toggles to use epoll or not, default true (but we failback to nio gracefully)
    useEpollGlobalWorkerPool = same behaviour as useNioGlobalWorkerPool but for Epoll.


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

    $ git pull https://github.com/michaelandrepearce/activemq-artemis netty-native-epoll

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

    https://github.com/apache/activemq-artemis/pull/1093.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 #1093
   
----
commit 0ac8f806d220c71b27133261e07a29a0813fff4d
Author: Michael Pearce <[hidden email]>
Date:   2017-02-24T13:29:22Z

    ARTEMIS-994 Support Netty Native Epoll on Linux
   
    Refactored SharedNioEventLoopGroup to be generic (as so we can re-use for both Nio and Epoll)
    Add support and toggles for Epoll in NettyAcceptor and NettyConnector (with fall back to NIO if cannot load Epoll)

commit 7bba1b062a7dfc15a000f95cb8e7ba21af20a430
Author: Michael Pearce <[hidden email]>
Date:   2017-02-24T17:51:30Z

    ARTEMIS-994 Support Netty Native Epoll on Linux
   
    —committing - missed file change
   
    Refactored SharedNioEventLoopGroup to be generic (as so we can re-use for both Nio and Epoll)
    Add support and toggles for Epoll in NettyAcceptor and NettyConnector (with fall back to NIO if cannot load Epoll)

commit ea82374c7f8fe31dedff537c07254a8868d1a62b
Author: Michael André Pearce <[hidden email]>
Date:   2017-03-12T20:37:14Z

    Merge pull request #2 from apache/master
   
    merge apache into fork

commit 1a079e8f94f0597543a0b16cdf08e9c2c2fe8541
Author: Michael André Pearce <[hidden email]>
Date:   2017-03-12T20:38:27Z

    Merge pull request #3 from michaelandrepearce/master
   
    merge master into branch

commit bc82c3615feab1af5b523d7a493b80988bfbcdd8
Author: Michael André Pearce <[hidden email]>
Date:   2017-03-13T10:49:27Z

    ARTEMIS-994 Support Netty Native Epoll on Linux
   
    Cleaner Epoll available check, and also fix for bad address caused by PartialPooledByteBufAllocator
   
   
    Remove usage of PartialPooledByteBufAllocator as per JIRA discussion, its use since 2.0.0 is no longer needed. mvn clean install ran fine with all tests passing.
   
   
    Clean up code a little, around handling failback if epoll isnt available.

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    I am running the testsuite, and if everything is ok I should merge it tomorrow...
   
    I have 50% of tests running and so far so good.. will know in 2 hours...
   
   
   
    BTW: can you run these commands please?
   
   
    ```sh
    git pull -rebase upstream master
    git push origin -f {your branch name}
    ```
   
    and then:
   
    ```sh
    git rebase -i upstream/master
    ```
   
    And squash these committs into one, with a description that matches it?
   
    I could do this myself, but I would rather have you finding the best commit name once you squash them.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    Awesome work! Testsuite is 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic I have just now rebased and squashed, hope this is what you were wanting me to 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce I don't think you really need three parameters for this... just one would do...
   
    - useEpoll...
   
   
    the other two you introduced could just use the same parameter, being the same semantic.. being just epoll.
   
   
    That way you would configure to use epoll or not with a single switch. If you need to configure different settings.. than you can just update the values.. it gets easier on users IMHO.
   
   
    WDYT?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic I though about this but annoyingly theyre named nioBlah as such just using that would be not directly indicate you're affecting epoll. Like wise if we were to strip the nio part then they would be generic but would break any compatibility with existing clients as they'd have to change their properties if used.
   
    This is why I introduce the extra duplicate props. If you think renaming the existing properties to generic in nature I can do this but as noted wouldn't be back compatible.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce I will add a new generic parameter, and deprecate the old one.
   
    Will do that on a separate commit. I just wanted to know if you had any reason.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic sounds 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic I notice this didn't get merged still. Did I miss understand and you expected me to make the configuration changes? if so no worries, just let me know.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce nope.. I just procrastinated for an afternoon :) give me till tomorrow


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic I was thinking this afternoon has 2.0.0 been cut for tagging? If not then maybe worth trying to get the property change done before that as a 2.0.0 is a major breaking release anyhow would avoid need for deprecating and creating a new generic we just would need a rename which would be cleaner


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce 2.0.0 (Artemis) is already cut. 3 days ago.. it was just the vote on...
   
    So this will be for 2.0.1.
   
   
    I was thinking about setting the default as NIO at least for a while, and having a --epoll --no-epoll property through the CLI.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce I did some performance tests with https://github.com/ssorj/quiver and the test hung... I will have to take double care before merging this.. it may take some extra time, as I don't want to make the broker unstable now.
   
    I would like to investigate what's happening before merging this.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic thanks, ill try look into also see if i spot anything.
   
    I assume this is repeatable.
    Was using the amqp client, looking at the quiver dependencies not the core or jms client.
    Also i assume the master currently doesn't suffer this?
    Lastly does this occur when set to nio also in the branch? or only when epoll is on?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce I couldn't check other options.. I was using AMQP..
   
    so, I was using this command line on quiver to replicate it:
   
    ```
    quiver q0 --sender qpid-messaging-cpp --receiver qpid-messaging-cpp --messages 1m --body-size 100 --credit 1000 --timeout 10
    ```
   
   
    you may need to use a few snapshots on quiver. but most stuff is available on fedora. Look at the list of dependencies if you like to try it.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    Hi @clebertsuconic i managed to get it running.
   
    Some notes:
    I was not able to run your command as is, using the version that comes via dnf / repo, seems some options are supported or something :S
   
    I got the below output.
   
    $ quiver q0 --sender qpid-messaging-cpp --receiver qpid-messaging-cpp --messages 1m --body-size 100 --credit 1000 --timeout 10
    usage: quiver [-h] [-m COUNT] [--impl NAME] [--body-size COUNT]
                  [--credit COUNT] [--timeout SECONDS] [--output DIRECTORY]
                  [--init-only] [--quiet] [--verbose]
                  ADDRESS
    quiver: error: unrecognized arguments: --sender qpid-messaging-cpp --receiver qpid-messaging-cpp
   
    I did however successfully run using just:
   
    $ quiver q0 --messages 1m --body-size 100 --credit 1000 --timeout 10
   
    I am using fedora 25.
   
    It ran fine with both server acceptor set to epoll and nio for me.
   
    Attached is the output i got for both runs.
   
    [quiver.epoll.txt](https://github.com/apache/activemq-artemis/files/852315/quiver.epoll.txt)
    [quiver.nio.txt](https://github.com/apache/activemq-artemis/files/852316/quiver.nio.txt)
   



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    the only comment is with epoll, i notice the producer was producing faster than the consumer during the test as such some queue depth occured, with nio producer and consumer speeds were closer as such less queue depth occured.
   
    epoll producer rate > nio producer rate by approx 200 messages a second where as consumer rates differed only by approx 50.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user franz1981 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce thanks for the results!


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @michaelandrepearce you have to follow all the dependencies on quiver. probably the cpp didn't finish compilation on make?
   
   
    There's something going on.. I will review it this week. if you run the cpp module, which generates a bit more load on AMQP and you will have a few weird errors. it just needs some testing.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1093: ARTEMIS-994 Support Netty Native Epoll on Linu...

asfgit
In reply to this post by asfgit
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1093
 
    @clebertsuconic i have re-run with march larger run size, and now able to reproduce.
   
    So it seems there is a direct memory leak, this occurs with both epoll and nio set, as suchi assume this is around removal of PartialPooledByteBufAllocator for the default netty allocator rather than epoll itself. I will dig into this area first.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
12
Loading...