[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 I?m clising this in the meantime proper tests will be performed :) ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 That an histogram comparing 2.5 and this PR with 1 MB messages, default consumer window and a target throughput of 50 msg/sec:  ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @michaelandrepearce @mtaylor @clebertsuconic @gaohoward Guys I was looking to the improvements made by this PR andI found that there is a log of Journal activity with it! I meanASYNCIO ones...it makes sense? That activity seems the bottleneck on consumer side ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user orpiske commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @franz1981 we can definitely work out some numbers for this! ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @mtaylor , ive had an offline discussion with @franz1981 he kindly went through in detail this change with me, know i better understand +1 from me. ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @mtaylor @michaelandrepearce Yep and it isn't changing the way the mapped journal works: it uses a different code path from the journal, without changing the MappedSequentialFile in any of its parts. I agree that I could provide relative measurements to show which impact it has vs the original version: with @orpiske probably we could start providing some relative numbers before merging anything. @orpiske wdyt? ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user mtaylor commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @michaelandrepearce If you are using non large messages, and without paging, this should have 0 impact. @franz1981 can you confirm. ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @michaelandrepearce I'm thinking with @orpiske which kind of test should make sense in order to emulate the correct use case to be stressed... Please, can you share your throughts on that? I'm concerned about how much is OS configuration sensitive running those tests as well ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @michaelandrepearce > we are using it for latency reasons Nice!! Happy to hear that :):) no datasync, no party! :) ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @michaelandrepearce Good points: paging (the consuming side) , this PR and a mapped journal should contend the OS page cache , so it depends on the dirty/dirty background ratio and total system memory. Probably should makes sense to allow this feature to be configurable (and paging as well) in order to avoid contention between them or just tune the OS according (tricky IMO). Wdyt? ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @franz1981 can you provide a perf test before and after for normal usage of the mapped journal e.g. as the main journal. I ask as we have ended up using it for a specific use case internally, where we are using it for latency reasons, so i want to be sure theres no degrade for the current usage. ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @gaohoward > I think using mapped file will get better performance over the NIO channel read. @orpiske have some nice idea to test it for sure :P > It would be better to have a unit test with it. @clebertsuconic wdyt? @clebertsuconic @gaohoward If you 2 will vote for the chunked version I will squash the 2 commits and provide a unit test to cover this new implementation: ATM it is just using the tests related to large messages. @clebertsuconic > I think so too.. but I would do it in the context of a refactoring in AMQP also That means that the same large body encoder should be reused for it as well? ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @franz1981 I think so too.. but I would do it in the context of a refactoring in AMQP also. we need to properly implement large messages in AMQP. ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 I think using mapped file will get better performance over the NIO channel read. It would be better to have a unit test with it. @clebertsuconic wdyt? ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 `JournalStorageManager::addBytesToLargeMessage` is another point that need to be improved to avoid OOM and better performances too ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @clebertsuconic I'm thinking to re-implement it by dividing the mmap in several mapped chunks instead of just 1 big one to help the OS to be able to map it when it is > Integer.MAX_VALUE ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @clebertsuconic Do not merge it yet, I have an exception on the test `BridgeTest.testBridgeWithVeryLargeMessage` ---
[GitHub] activemq-artemis issue #2015: ARTEMIS-1807 File-based Large Message encoding...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2015 @clebertsuconic @gaohoward AFAIK you've worked recently on Large messages: please share your thoughts and if you see other points where this improvement should be used. The optimal solution would be by using [DefaultFileRegion](https://netty.io/4.1/api/io/netty/channel/DefaultFileRegion.html) to zero-copy data through the wire, but it involves to change `Packet` in order to use a `FileRegion` body instead of `byte[]`: this solution I've proposed seems less invasive, while providing very good perf improvements. ---
