You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Bryan Duxbury (JIRA)" <ji...@apache.org> on 2008/07/12 03:17:31 UTC

[jira] Created: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
---------------------------------------------------------------------------------------------------------------------------------------------

                 Key: THRIFT-81
                 URL: https://issues.apache.org/jira/browse/THRIFT-81
             Project: Thrift
          Issue Type: New Feature
          Components: Library (Java)
            Reporter: Bryan Duxbury
            Priority: Trivial


In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-81:
--------------------------------

    Attachment: thrift-81-v3.patch

Here you go.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81-v3.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649582#action_12649582 ] 

Bryan Duxbury commented on THRIFT-81:
-------------------------------------

For the time being, I don't think that I'm really worried about the busy loop effect. It would considerably complicate the code to try to take into account sockets coming in and out of the selectable list.

I guess I'm not really opposed to upping the default memory limit, though I don't think that LONG_MAX is the right size to change to. The default JVM heap size is quite small (256 or 512MB, if I recall), and setting this value high above that just seems like a recipe for crashes. I can see the argument for just having out of memory exceptions as the result instead of silently blocking unexpectedly, though. 

I switched to TByteArrayOutputStream instead of ByteArrayOutputStream because it allows you to get at the underlying array without an array copy. It's just an optimization that was apparent to me when I was looking at this code.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-81:
--------------------------------

    Attachment: thrift-81-v2.patch

Small refinement.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649466#action_12649466 ] 

Bryan Duxbury commented on THRIFT-81:
-------------------------------------

We've been using the v2 of this patch in production for a while now, and it seems to perform quite well. Can a committer take a look so we can get this into the official codebase?

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649828#action_12649828 ] 

Bryan Duxbury commented on THRIFT-81:
-------------------------------------

Fair enough. Do you need me to add a new patch, or can you just set it to something bigger when you commit?

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury reassigned THRIFT-81:
-----------------------------------

    Assignee: Bryan Duxbury

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Reiss resolved THRIFT-81.
-------------------------------

    Resolution: Fixed

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81-v3.patch, thrift-81-v4.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649559#action_12649559 ] 

David Reiss commented on THRIFT-81:
-----------------------------------

If you want to avoid the busy wait, the best way that I can think of would be to remove the socket in question from the selectable set until some memory is freed up.  This change is not strictly necessary, but I think we should probably turn the default max buffer size up to something like LONG_MAX to avoid running into it unless you are sure you know what you are doing.

What is the purpose of the changes to response_ near the bottom of the patch?

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-81:
--------------------------------

    Patch Info: [Patch Available]

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-81:
--------------------------------

    Attachment: thrift-81.patch

Here's my first shot at fixing this. The patch adds a configurable max read buffer size and accounts for how much space is already allocated. If the next frame would go past the limit, it skips that frame until the next pass through the select loop. If the frame being sent is bigger than the limit all by itself, the connection is immediately closed.

One possible concern is that the select loop might spin really fast when all memory is used up if the requests being processed aren't going very quickly. The selector will just keep on coming back through the readable (but too large) FrameBuffers without any pausing. I'm not sure if there's a good way to deal with this, though.

I would appreciate a code review.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649829#action_12649829 ] 

David Reiss commented on THRIFT-81:
-----------------------------------

It'd be nice if you could do it since the type needs to change.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-81:
--------------------------------

    Attachment: thrift-81-v4.patch

Actually, here's a version that makes the current amount allocated a long too. That way you can use all 4TB of memory you have for read buffers if you want. 

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81-v3.patch, thrift-81-v4.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-81) TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12649824#action_12649824 ] 

David Reiss commented on THRIFT-81:
-----------------------------------

Yeah, but if someone increases the heap size, I don't want them to have to remember to increase the server buffer size also.  And OOM exceptions are much easier to debug then "all of a sudden, my server is pinning a CPU core".  I'd much rather have it be completely out of reach and let people who know what they are doing mess with it.

> TNonblockingServer should have the option to limit the number of clients or the amount of memory it will allocate to incomplete client frames
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-81
>                 URL: https://issues.apache.org/jira/browse/THRIFT-81
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Library (Java)
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Trivial
>         Attachments: thrift-81-v2.patch, thrift-81.patch
>
>
> In the current TNonblockingServer implementation, it would be possible for a large number of clients to connect to the server and send a very large 4-byte frame size, causing the server to allocate lots of memory and die. The server should have an option to protect itself against either overwhelming numbers of clients or more than a specified amount of memory at a time, or both. This would make it much more robust in the face of an unknown pool of clients.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.