[
https://issues.apache.org/jira/browse/THRIFT-3081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)
