You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "Geoff Cadien (JIRA)" <ji...@apache.org> on 2008/06/05 19:37:45 UTC
[jira] Created: (DIRMINA-601) Add sendfile support to transport-apr
Add sendfile support to transport-apr
-------------------------------------
Key: DIRMINA-601
URL: https://issues.apache.org/jira/browse/DIRMINA-601
Project: MINA
Issue Type: Improvement
Components: Core, Transport
Affects Versions: 2.0.0-M2
Reporter: Geoff Cadien
I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Geoff Cadien updated DIRMINA-601:
---------------------------------
Attachment: (was: sendfile-patches.zip)
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DIRMINA-601) Add sendfile support to
transport-apr
Posted by "Mike Heath (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12603767#action_12603767 ]
Mike Heath commented on DIRMINA-601:
------------------------------------
I see a number of difficulties with this situation. With the existing code, it is possible to use a previously open FileChannel. This makes it possible to serve a few files to many IoSessions without opening lots of file descriptors (which can be a limited resource in highly scalable applications.) With the provided patch, this is not possible.
With APR, would it be possible to create a org.apache.tomcat.jni.File with a java.io.FileDescriptor. I used a java.io.FileDescriptor for the asynchronous File I/O stuff I did with POSIX AIO a year or so ago and it worked well. If so, the user could still send an open FileChannel in APR and we wouldn't have to open the file ourselves.
Geoff have you patched the existing NIO IoProcessor code? I didn't see any of that in the attached patch.
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DIRMINA-601) Add sendfile support to
transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12604282#action_12604282 ]
Geoff Cadien commented on DIRMINA-601:
--------------------------------------
Mike, You are correct that the ability to reuse a FileChannel is lost. I'm not sure open file descriptors is really that big of an issue. If your handling lots of open connections my guess is you'll saturate the network before you have problems with too many open files. org.apache.tomcat.jni.File doesn't have a constructor, just a bunch of static methods. The return value form File.open() is a long which I believe is a pointer to a struct which contains the fd and not the fd itself. Now I suppose if we added IoSession to the SendableFile.send() method you could keep the SendableFile around and reuse it lowering the number of open file descriptors.
You're correct I did forget to add the patch to NioProcessor. I add that shortly.
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Geoff Cadien updated DIRMINA-601:
---------------------------------
Attachment: sendfile-patches.zip
Added patch for NioProcessor to use SendableFile
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Geoff Cadien updated DIRMINA-601:
---------------------------------
Attachment: (was: sendfile-patches.zip)
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Emmanuel Lecharny (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Emmanuel Lecharny updated DIRMINA-601:
--------------------------------------
Fix Version/s: 3.0.0-M1
Postponed
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Fix For: 3.0.0-M1
>
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Geoff Cadien updated DIRMINA-601:
---------------------------------
Attachment: sendfile-patches.zip
I've attached the new classes/interfaces and diffs to exising classes to implement sendifle using the apr transport.
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DIRMINA-601) Add sendfile support to
transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12603089#action_12603089 ]
Geoff Cadien commented on DIRMINA-601:
--------------------------------------
I agree. That is exactly what I have done in my code. I have a factory to produce the correct type of SendableFile, using the type of IoSession as discriminator. It might be nice to have that functionality in MINA to avoid everyone having to roll their own.
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DIRMINA-601) Add sendfile support to
transport-apr
Posted by "Julien Vermillard (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12602945#action_12602945 ]
Julien Vermillard commented on DIRMINA-601:
-------------------------------------------
Nice patch !
I think your SendableFile is a good abstraction for providing sendFile() implementation for transports.
As I understand if you want to send a file using sendFile of an IoService, you need to instantiate the good SendableFile concrete class ? like AprSendable file if you use an AprIoacceptor.
We need to be able to change from Nio IoService to Apr based IoService without changing the code.
So I think it miss a per IoService SendableFile factory for creating SendableFile independently of the used transport.
WDYT ?
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DIRMINA-601) Add sendfile support to transport-apr
Posted by "Geoff Cadien (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DIRMINA-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Geoff Cadien updated DIRMINA-601:
---------------------------------
Attachment: sendfile-patches.zip
Forgot one patch
> Add sendfile support to transport-apr
> -------------------------------------
>
> Key: DIRMINA-601
> URL: https://issues.apache.org/jira/browse/DIRMINA-601
> Project: MINA
> Issue Type: Improvement
> Components: Core, Transport
> Affects Versions: 2.0.0-M2
> Reporter: Geoff Cadien
> Attachments: sendfile-patches.zip
>
>
> I've take a shot at adding support for sendfile to transport-apr. I've had to make several changes because in the current code FileRegion is NIO specific because of it's reliance on FileChannel. I've made the following changes:
> I've created an interface called SendableFile to take the place of using FileChannel in FileRegion. I've also created two implementations of this interface, AprSendableFile and NioSendableFile. The method of interest is send(long position, long length) which is used to send the file. Notice it doesn't take an IoSession or a Channel as a parameter. I tried using generics to include the proper subclass of IoSession but that didn't work out very well. :-)
> There are still a couple of problems that need to be dealt with. First and foremost is that AbstractIoSession will create instances of DefaultFileRegion in the write method when passed either a File or a FileChannel. This causes a problem because FileRegion is no longer NIO specific. I don't have a solution right now other than removing the code.
> The other problem is with FileRegionWriteFilter (and unit test), again because FileRegion is NIO specific and exposes a FileChannel.
> I'm hoping somebody can take a look at the code and provide some feedback if they believe this is headed in the right direction or have a better idea. I'll attach the patches shortly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.