[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481443#comment-14481443 ] Ben Craig commented on THRIFT-3084: --- That logic allows for maxConnections + 1 to be accepted. It's worse than that. if you don't do anything fancy with TServerSocket, the current logic allows for maxConnections + 1024 connections. TServerSocket sets the backlog in the listen system call to 1024 by default. Sure, you may not actually call 'accept()' on all 1024 of those, but the client on the other end has received a SYN-ACK, and has sent an ACK and some other traffic to the server. Those 1024 sockets in the servers backlog are also consuming resources. So let's look at the problem a different way... Suppose 'maxConnections' clients have already connected. What do you want the behavior to be on the client side when they try to make the next connection? The current behavior is that they basically stay in limbo until one of the other connections clears up. What would you prefer the behavior be? If you want the client to be disconnected, then that suggests that you want to call accept(), but then immediately close() it if the server is at capacity. Both the current implementation (for TThreadPoolServer at least) and the suggested approach will do a reasonable job at preventing excessive threads and file descriptors from being created. TThreadedServer could stand to be improved either way though. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481519#comment-14481519 ] James E. King, III commented on THRIFT-3084: I was strictly referring to the user space process resource allocation. You are correct that the backlog on the server socket would fill up and it is up to the implementer to decide how much of a backlog they want to service. This is even a problem with TSimpleServer; the default backlog is 1024 but it can only do one thing at a time. Providing backpressure seems like a reasonable solution and that is what the backlog does, because those connections in the backlog will eventually timeout if they are not serviced. None of this is in user space however, because we haven't accepted them, so that is okay. A web server will provide backpressure by replying with a HTTP 503 response and closing the connection. We have no such low level handshake where the server could send a TTransportException(TOO_BUSY) to the client today. Having a barrier as I suggested will ensure that in the user-space process only that many concurrent client sockets will be allowed to be connected and no more (unless the limit is reduced while there are more clients connected than the new limit, then they will drain off), whereas without the barrier we will allow maxConnections + 1 for a TThreadPoolServer, which is incorrect, and an unlimited number of connections for a TThreadedServer, which is also nasty. Instead of implementing it in the way I proposed, we could add a semaphore to TThreadedServer such that it has the same behavior as TThreadPoolServer, whereby it is clearly documented that whatever maximum for threads or connections are specified, it will be possible to have one more in user space that is blocking on the maximum. This is perhaps a smaller overall change, but less correct. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481544#comment-14481544 ] James E. King, III edited comment on THRIFT-3084 at 4/6/15 6:18 PM: Good points, although TSimpleServer and TThreadPoolServer have essentially the same serve() loop in them that only differs where clients are handled If you look at my proposal in THRIFT-3083 it should be clear this would be extracted such that TSimpleServer and TThreadPoolServer become small classes that implement onClientConnected and onClientDisconnected, and nothing more. Extracting the common run loop for serve() will simplify maintenance and standardize server behavior in the same way THRIFT-3081 standardized the client run loop. I would disagree that TSimpleServer should be free of object inheritance. It is already a TServer so it is doing that. We should consolidate the common serve loop as an example of good code reuse and maintainability practices to others. was (Author: jking3): Good points, although TSimpleServer and TThreadPoolServer have essentially the same serve() loop in them that only differs where clients are handled If you look at my proposal in THRIFT-3083 it should be clear this would be extracted such that TSimpleServer and TThreadPoolServer become small classes that implement onClientConnected and onClientDisconnected, and nothing more. Extracting the common run loop for serve() will simplify maintenance and standardize server behavior in the same way THRIFT-3081 standardized the client run loop. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481530#comment-14481530 ] Randy Abernethy commented on THRIFT-3084: - Prior to addressing these issues I suggest considering the removal of TThreadedServer. If a server is not truly differentiating then we are just confusing users by having it and adding work for the contributors/maintainers. What value add does TThreadedServer provide over TThreadPoolServer? With a few lines of code we could easily provide features that makde TThreadPoolServer behave like TThreadedServer when desired (you can do it presently by adding threads to the pool with server events as Ben suggests). Recommendations: - We remove TThreadedServer from the system and focus on getting a single threaded server right (TThreadPoolServer seems like the correct starting point). By avoiding undifferentiated servers we also eliminate any need for complicating the server code with shared base implementations. TNonBlockingServer is differentiating and should share the TServer interface and little else, TSimpleServer should be simple and free of code hierarchies by design and TThreadPoolServer should stand as the threaded server and encapsulate good threading practices in one simple package, with pooling as an optimization. Thoughts? C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481544#comment-14481544 ] James E. King, III commented on THRIFT-3084: Good points, although TSimpleServer and TThreadPoolServer have essentially the same serve() loop in them that only differs where clients are handled If you look at my proposal in THRIFT-3083 it should be clear this would be extracted such that TSimpleServer and TThreadPoolServer become small classes that implement onClientConnected and onClientDisconnected, and nothing more. Extracting the common run loop for serve() will simplify maintenance and standardize server behavior in the same way THRIFT-3081 standardized the client run loop. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14482250#comment-14482250 ] Ben Craig commented on THRIFT-3084: --- I was going to argue more, but then I argued myself into agreeing with you. :p I'm fine with there being some synchronization object sitting just before an accept() call, and that synchronization object can block until threads are made available. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14482236#comment-14482236 ] Ben Craig commented on THRIFT-3084: --- I'm great with gutting TThreadedServer, but I don't want to remove it. That is breaking source compatibility without a great reason. Perhaps make TThreadedServer such that it is implemented entirely in terms of TThreadPoolServer (i.e. TThreadedServer methods forward to an embedded TThreadPoolServer member). Or make a typedef alias that is still source compatible. I'd rather not break source compatibility though. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] thrift pull request: [THRIFT-3086] fix a few minor valgrind identi...
GitHub user jeking3 opened a pull request: https://github.com/apache/thrift/pull/429 [THRIFT-3086] fix a few minor valgrind identified issues You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-3086-minor-valgrind-issues Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/429.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 #429 commit 86889762402795b78e4c1c9381d5b699e74c5fae Author: Jim King jim.k...@simplivity.com Date: 2015-04-07T01:38:06Z [THRIFT-3086] fix a few minor valgrind identified issues --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (THRIFT-3086) C++ Valgrind Error Cleanup
[ https://issues.apache.org/jira/browse/THRIFT-3086?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14482381#comment-14482381 ] ASF GitHub Bot commented on THRIFT-3086: GitHub user jeking3 opened a pull request: https://github.com/apache/thrift/pull/429 [THRIFT-3086] fix a few minor valgrind identified issues You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-3086-minor-valgrind-issues Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/429.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 #429 commit 86889762402795b78e4c1c9381d5b699e74c5fae Author: Jim King jim.k...@simplivity.com Date: 2015-04-07T01:38:06Z [THRIFT-3086] fix a few minor valgrind identified issues C++ Valgrind Error Cleanup -- Key: THRIFT-3086 URL: https://issues.apache.org/jira/browse/THRIFT-3086 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.9.2 Reporter: James E. King, III Priority: Minor valgrind identified a few issues in the C++ library: 1. In TFileTransport, inconsistent use of malloc then delete[] 2. In ZLibTest, leaking allocations 3. In RecursiveTest, leaking allocation These are minor, but a clean valgrind run is a happy valgrind run. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14482396#comment-14482396 ] James E. King, III edited comment on THRIFT-3084 at 4/7/15 4:07 AM: Refer to THRIFT-3083, it can be implemented as a very simple subclass with overrides on serve, onClientConnected, and onClientDisconnected: 1. Serve calls the base serve() (which does everything up to closing the sevrer transport) then waits on the monitor for notify. 2. onClientConnected adds to the tracking thread set; to satisfy this particular Jira enhancement, if the concurrent limit is reached following adding to the set, then enable the barrier. 3. onClientDisconnected removes from the tracking thread set, disables the barrier if the new set size is below the maxConnections limit, and notifies if it is fully drained to unblock the stop process (item 1). was (Author: jking3): Refer to THRIFT-3083, it can be implemented as a very simple subclass with overrides on serve, onClientConnected, and onClientDisconnected: 1. Serve calls the base serve() (which does everything up to closing the sevrer transport) then waits on the monitor for notify. 2. onClientConnected adds to the tracking thread set 3. onClientDisconnected removes from the tracking thread set, and notifies if it is fully drained C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14482396#comment-14482396 ] James E. King, III commented on THRIFT-3084: Refer to THRIFT-3083, it can be implemented as a very simple subclass with overrides on serve, onClientConnected, and onClientDisconnected: 1. Serve calls the base serve() (which does everything up to closing the sevrer transport) then waits on the monitor for notify. 2. onClientConnected adds to the tracking thread set 3. onClientDisconnected removes from the tracking thread set, and notifies if it is fully drained C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (THRIFT-3086) C++ Valgrind Error Cleanup
James E. King, III created THRIFT-3086: -- Summary: C++ Valgrind Error Cleanup Key: THRIFT-3086 URL: https://issues.apache.org/jira/browse/THRIFT-3086 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.9.2 Reporter: James E. King, III Priority: Minor valgrind identified a few issues in the C++ library: 1. In TFileTransport, inconsistent use of malloc then delete[] 2. In ZLibTest, leaking allocations 3. In RecursiveTest, leaking allocation These are minor, but a clean valgrind run is a happy valgrind run. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] thrift pull request: THRIFT-3085: thrift_reconnecting_client never...
GitHub user NOMORECOFFEE opened a pull request:
https://github.com/apache/thrift/pull/427
THRIFT-3085: thrift_reconnecting_client never try to reconnect
gen_server does not handle message try_connect after unsuccessful
connection, and gen_server always return {error, noconn}
THRIFT-3085
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/NOMORECOFFEE/thrift master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/427.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 #427
commit 17d416aca5c5037ff233d4c2e7afa0ea36df5f06
Author: NOMORECOFFEE github.cof...@hotmail.com
Date: 2015-04-06T10:55:08Z
thrift_reconnecting_client never try to reconnect
gen_server does not handle message try_connect after unsuccessful
connection, and gen_server always return {error, noconn}
THRIFT-3085
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Created] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
Veselov Andrey created THRIFT-3085:
--
Summary: thrift_reconnecting_client never try to reconnect
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Priority: Minor
gen_server does not handle message try_reconnect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Veselov Andrey updated THRIFT-3085:
---
Description: gen_server does not handle message try_connect after
unsuccessful connection, and gen_server always return \{error, noconn\} (was:
gen_server does not handle message try_reconnect after unsuccessful connection,
and gen_server always return \{error, noconn\})
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Priority: Minor
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_connect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Veselov Andrey updated THRIFT-3085:
---
Attachment: thrift-3085-thrift_reconnecting_client.path
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Priority: Minor
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_reconnect after unsuccessful
connection, and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-3081) C++ Consolidate client processing loops in TServers
[
https://issues.apache.org/jira/browse/THRIFT-3081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481318#comment-14481318
]
James E. King, III commented on THRIFT-3081:
Well, I mentioned THRIFT-2441 and THRIFT-3801 in my pull request and I think I
confused the github bot.
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/428
Bugfix/thrift 3081 consolidate client processing loops
The pull request consolidates the client processing loop contained within
TSimpleServer, TThreadedServer, TThreadPoolServer that were all similar but not
functionally identical. This will improve maintainability.
This pull request and the open one for THRIFT-2441 will collide slightly.
Whichever one gets pulled in first, I will update the other...
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jeking3/thrift
bugfix/THRIFT-3081-consolidate-client-processing-loops
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/428.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 #428
commit 44713020d03c38776432642d0798ea8d9f3e4168
Author: Jim King jim.k...@simplivity.com
Date: 2015-04-05T16:32:03Z
THRIFT-3081 consolidate C++ client processing loops
commit 3cc72ce101f2214b5b9698473523a93820219388
Author: Jim King jim.k...@simplivity.com
Date: 2015-04-06T15:10:50Z
Merge branch 'master' of https://github.com/apache/thrift into
bugfix/THRIFT-3081-consolidate-client-processing-loops
Reply
C++ Consolidate client processing loops in TServers
---
Key: THRIFT-3081
URL: https://issues.apache.org/jira/browse/THRIFT-3081
Project: Thrift
Issue Type: Improvement
Components: C++ - Library
Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2
Reporter: James E. King, III
Currently each of TSimpleServer, TThreadedServer, and TThreadPoolServer have
their own very similar but not quite identical way of processing a client's
lifetime. The code has been copied around and changed; for example a
TThreadPoolServer handles TTransportExceptions from process() differently
than a TThreadedServer does.
There are certain requirements for this processing loop that needs to be met
by every client. Consolidating these three disparate implementations of the
client processing loop into one will provide consistency as well as easier
maintenance, as there will be one common client processing loop that will
contain all the logic from {{eventHandler-createContext}} through
{{client-close}}.
It was also discovered that all three implementations call peek() in each
loop which causes more recv calls than are really needed. Will experiment
with removing peek entirely; expectation is that it is sufficient to have
exception handling around process() and/or have process() return false to end
the processing loop, and peek() is likely an unnecessary temporary band-aid
that got left there.
This was inspired by changes in THRIFT-2441 and I was encouraged to make this
a separate body of work from that change so that it can be reviewed in
isolation from other changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-2441) Cannot shutdown TThreadedServer when clients are still connected
[ https://issues.apache.org/jira/browse/THRIFT-2441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481296#comment-14481296 ] ASF GitHub Bot commented on THRIFT-2441: GitHub user jeking3 opened a pull request: https://github.com/apache/thrift/pull/428 Bugfix/thrift 3081 consolidate client processing loops The pull request consolidates the client processing loop contained within TSimpleServer, TThreadedServer, TThreadPoolServer that were all similar but not functionally identical. This will improve maintainability. This pull request and the open one for THRIFT-2441 will collide slightly. Whichever one gets pulled in first, I will update the other... You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-3081-consolidate-client-processing-loops Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/428.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 #428 commit 44713020d03c38776432642d0798ea8d9f3e4168 Author: Jim King jim.k...@simplivity.com Date: 2015-04-05T16:32:03Z [THRIFT-3081] consolidate C++ client processing loops commit 3cc72ce101f2214b5b9698473523a93820219388 Author: Jim King jim.k...@simplivity.com Date: 2015-04-06T15:10:50Z Merge branch 'master' of https://github.com/apache/thrift into bugfix/THRIFT-3081-consolidate-client-processing-loops Cannot shutdown TThreadedServer when clients are still connected Key: THRIFT-2441 URL: https://issues.apache.org/jira/browse/THRIFT-2441 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.9.1 Reporter: Chris Stylianou Assignee: Ben Craig Attachments: THRIFT-2441-prelim.patch When calling stop() on the TThreadedServer no interrupts are sent to the client threads. This means the stop() call blocks on tasksMonitor.wait() until all client naturally disconnect. How can we tell the client thread connections to close/exit during the TThreadedServer::stop() call? -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-2441) Cannot shutdown TThreadedServer when clients are still connected
[ https://issues.apache.org/jira/browse/THRIFT-2441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481322#comment-14481322 ] James E. King, III commented on THRIFT-2441: Please ignore item 428, it is for THRIFT-3081 however I mentioned this one in the pull request notes and the github asf bot tagged it to this instead. Cannot shutdown TThreadedServer when clients are still connected Key: THRIFT-2441 URL: https://issues.apache.org/jira/browse/THRIFT-2441 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.9.1 Reporter: Chris Stylianou Assignee: Ben Craig Attachments: THRIFT-2441-prelim.patch When calling stop() on the TThreadedServer no interrupts are sent to the client threads. This means the stop() call blocks on tasksMonitor.wait() until all client naturally disconnect. How can we tell the client thread connections to close/exit during the TThreadedServer::stop() call? -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Assigned] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer reassigned THRIFT-3085:
--
Assignee: Jens Geyer
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Assignee: Jens Geyer
Priority: Minor
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_connect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request: THRIFT-3085: thrift_reconnecting_client never...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/427 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Resolved] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-3085.
Resolution: Fixed
Fix Version/s: 0.9.3
Committed.
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Assignee: Jens Geyer
Priority: Minor
Fix For: 0.9.3
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_connect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481339#comment-14481339
]
ASF GitHub Bot commented on THRIFT-3085:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/427
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Assignee: Jens Geyer
Priority: Minor
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_connect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request: Bugfix/thrift 3081 consolidate client process...
GitHub user jeking3 opened a pull request: https://github.com/apache/thrift/pull/428 Bugfix/thrift 3081 consolidate client processing loops The pull request consolidates the client processing loop contained within TSimpleServer, TThreadedServer, TThreadPoolServer that were all similar but not functionally identical. This will improve maintainability. This pull request and the open one for THRIFT-2441 will collide slightly. Whichever one gets pulled in first, I will update the other... You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-3081-consolidate-client-processing-loops Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/428.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 #428 commit 44713020d03c38776432642d0798ea8d9f3e4168 Author: Jim King jim.k...@simplivity.com Date: 2015-04-05T16:32:03Z [THRIFT-3081] consolidate C++ client processing loops commit 3cc72ce101f2214b5b9698473523a93820219388 Author: Jim King jim.k...@simplivity.com Date: 2015-04-06T15:10:50Z Merge branch 'master' of https://github.com/apache/thrift into bugfix/THRIFT-3081-consolidate-client-processing-loops --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481350#comment-14481350 ] Ben Craig commented on THRIFT-3084: --- I think that the ThreadManager class already provides the desired functionality, at least for the TThreadPoolServer. ThreadManager::newSimpleThreadManager() creates a threadManager with a maximum thread count. Users of TThreadPoolServer can provide their own implementation of ThreadManager. Alternatively, the simpleThreadManager can be manipulated / invoked by other classes. It is useful to do so from a handler interface factory for instance. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3084) C++ add concurrent client limit to threaded servers
[ https://issues.apache.org/jira/browse/THRIFT-3084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481375#comment-14481375 ] James E. King, III commented on THRIFT-3084: That will not limit the number of concurrent clients allowed to connect at once properly: * Server is at maxConnections clients. * serve() accept()s another TTransport (client) * serve() blocks(?) waiting to insert the new client into the thread pool. That logic allows for maxConnections + 1 to be accepted. Further, it does not help the TThreadedServer maintain any limits. Having a controllable barrier immediately before accept() would allow any implementation to enable or disable the accept barrier and subsequently exactly control the upper bound on the number of concurrent accepted connections to the server. C++ add concurrent client limit to threaded servers --- Key: THRIFT-3084 URL: https://issues.apache.org/jira/browse/THRIFT-3084 Project: Thrift Issue Type: Improvement Components: C++ - Library Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 Reporter: James E. King, III The TThreadedServer and TThreadPoolServer do not impose limits on the number of simultaneous connections, which is not useful in production as bad clients can drive a server to consume too many file descriptors or have too many threads. 1. Add a barrier to TServerTransport that will be checked before accept(). 2. In the onClientConnected override (see THRIFT-3083) if the server reaches the limit of the number of accepted clients, enable the barrier. 3. In the onClientDisconnected override if the count of connected clients falls below the maximum concurrent limit, clear the barrier. This will allow the limit to be changed dynamically at runtime (lowered) with drain off clients until more can be accepted. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3085) thrift_reconnecting_client never try to reconnect
[
https://issues.apache.org/jira/browse/THRIFT-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14481368#comment-14481368
]
Hudson commented on THRIFT-3085:
SUCCESS: Integrated in Thrift #1493 (See
[https://builds.apache.org/job/Thrift/1493/])
THRIFT-3085 thrift_reconnecting_client never tries to reconnect (jensg: rev
7fc33be18cdf995ac8b0845897f9b4ea3228c50f)
* lib/erl/src/thrift_reconnecting_client.erl
thrift_reconnecting_client never try to reconnect
-
Key: THRIFT-3085
URL: https://issues.apache.org/jira/browse/THRIFT-3085
Project: Thrift
Issue Type: Bug
Components: Erlang - Library
Affects Versions: 0.9.2
Reporter: Veselov Andrey
Assignee: Jens Geyer
Priority: Minor
Fix For: 0.9.3
Attachments: thrift-3085-thrift_reconnecting_client.path
gen_server does not handle message try_connect after unsuccessful connection,
and gen_server always return \{error, noconn\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
