You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Dave Watson (Created) (JIRA)" <ji...@apache.org> on 2011/12/02 00:34:40 UTC
[jira] [Created] (THRIFT-1442) ] TNonblockingServer: Refactor to
allow multiple IO
] TNonblockingServer: Refactor to allow multiple IO
----------------------------------------------------
Key: THRIFT-1442
URL: https://issues.apache.org/jira/browse/THRIFT-1442
Project: Thrift
Issue Type: Improvement
Components: C++ - Library
Reporter: Dave Watson
Priority: Minor
Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
From: Mark Rabkin <mr...@fb.com>
Date: Tue, 18 May 2010 22:16:09 +0000
Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
threads, not just one
davejwatson: This diff ads multiple IO threads to TNonblocking Server.
We use it extensively, it's pretty well tested other than merge errors.
This diff reverts THRIFT-1184, allowing re-use of an existing
event base. With multiple IO threads, re-using a single event base doesn't
make sense, so this seemed ok
Summary:
The diff creates multiple IO threads at startup -- the number of threads in
this diff is fixed at server start and cannot change for simplicity. The
first thread (id = 0) also doubles as the listen/accept thread, so there is
still only a single thread doing accepts. The other IO threads listen only
on their notification pipe and the actual connection sockets.
Also, for simplicity, each accepted connection is simply assigned in a
round-robin fashion to the next IO thread and lives on that IO thread
permanently.
Note that there are only trivial changes to TConnection to get it to work, so
most of the tricky state transitions and buffer management are unchanged.
There is still a single worker pool for processing tasks, so that code is
unchanged as well.
The trickiest part of the diff requiring the most careful review is the new
synchronization code in the TNonblockingServer to manage the connection stack
and counters of active/inactive connections. We now lock a mutex when
incrementing/decrementing server counters, which is less than ideal for
extremely high-QPS servers -- should I switch to atomic ops?
One important change here is that while connections are created and initialized
by the listen thread (IO thread #0), they may now be assigned to an event_base
owned by a different IO thread. To work safely, TConnection::init() no longer
calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
This results in a notification-fd event in the correct event base, which then
calls TConnection::transition() which sets up correct read flags. This means
that a TConnection now calls notifyIOThread() once upon creation and assignment
to its event_base, and thereafter after each processing call completes.
TNonblocking server: Allow high-priority scheduler for IO threads
Summary:
Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
a high scheduling priority for the IO thread(s) -- this is a POSIX api and
should be safe on most operating systems.
Please let me know if this is a known terrible idea, but we're experimenting
to see if this helps the situation where we have 40 worker threads and 1 IO
thread and the IO thread doesn't get scheduled nearly often enough.
Reviewers: dreiss,edhall,putivsky
Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Pavlin Radoslavov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavlin Radoslavov updated THRIFT-1442:
--------------------------------------
Attachment: (was: thrift_nonblocking_server_single_thread.patch)
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Pavlin Radoslavov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavlin Radoslavov updated THRIFT-1442:
--------------------------------------
Attachment: thrift_nonblocking_server_single_thread_v2.patch
Resubmitting the patch thrift_nonblocking_server_single_thread_v2.patch by regenerating it after the fix for THRIFT-1064 was applied to file TNonblockingServer.cpp
The patch is against SVN trunk r1362773
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch, thrift_nonblocking_server_single_thread_v2.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Dave Watson (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Watson updated THRIFT-1442:
--------------------------------
Summary: TNonblockingServer: Refactor to allow multiple IO Threads (was: ] TNonblockingServer: Refactor to allow multiple IO
)
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Priority: Minor
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13163251#comment-13163251 ]
Hudson commented on THRIFT-1442:
--------------------------------
Integrated in Thrift #347 (See [https://builds.apache.org/job/Thrift/347/])
Thrift-1442: TNonblockingServer: Refactor to allow multiple IO Threads
Client: cpp
Patch: Dave Watson
Ads multiple IO threads to TNonblocking Server
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Roger Meier (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13165499#comment-13165499 ]
Roger Meier commented on THRIFT-1442:
-------------------------------------
Thanks Dave!
I have still some issues with generated code...
I think we should use a defined standard, e.g. -std=c++0x
In addition I requested llvm on the build slaves, but it does not work...
https://builds.apache.org/job/Thrift-llvm/
I'm able to compile the Thrift Compiler with clang++ on my 32-bit Debian squeeze
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Reopened] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Pavlin Radoslavov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavlin Radoslavov reopened THRIFT-1442:
---------------------------------------
I am reopening the issue, because it breaks the case of using
TNonblockingServer in a single-thread setup (i.e., no threads), and
event_base provided by the user. In such setup, the main eventloop can
be run/loop outside Thrift, while with the original fix for THRIFT-1442
it was not possible: loop over the libevent handler must happen inside
TNonblockingServer::serve().
The provided patch does the following:
(a) Preserves the existing multiple I/O Threads semantics as implemented
by the original fix for THRIFT-1442:
_server = new TNonblockingServer(_processor, _service_port);
_server->setNumIOThreads(5);
_server->serve(); // Main workhorse function
(b) Add-back support for using TNonblockingServer as a single thread
and eventloop outside the server:
_server = new TNonblockingServer(_processor, _service_port);
_server->setServerEventHandler(_my_server_event_handler);
_server->registerEvents(_my_event_base);
// Continue execution and loop over my_event_base elsewhere
* Add-back support for (b), and allowing TNonblockingServer to use a socket
that was created/set in advance:
_server = new TNonblockingServer(_processor, _service_port);
_server->listenSocket(_socket);
_server->registerEvents(_my_event_base);
// Continue execution and loop over my_event_base elsewhere
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Closed] (THRIFT-1442) TNonblockingServer: Refactor to allow
multiple IO Threads
Posted by "Jake Farrell (Closed) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jake Farrell closed THRIFT-1442.
--------------------------------
Resolution: Fixed
Fix Version/s: 0.9
Assignee: Dave Watson
Thanks Dave, Committed
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Dave Watson (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Watson updated THRIFT-1442:
--------------------------------
Attachment: nonblocking_unsigned.patch
fixes unsigned vs signed warning
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Pavlin Radoslavov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavlin Radoslavov updated THRIFT-1442:
--------------------------------------
Attachment: thrift_nonblocking_server_single_thread.patch
The provided patch thrift_nonblocking_server_single_thread.patch
adds-back the semantics of using
TNonblockingServer in a single-thread setup
(i.e., no threads), and event_base provided by the user.
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch, thrift_nonblocking_server_single_thread.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (THRIFT-1442) TNonblockingServer:
Refactor to allow multiple IO Threads
Posted by "Roger Meier (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13165499#comment-13165499 ]
Roger Meier edited comment on THRIFT-1442 at 12/8/11 8:25 PM:
--------------------------------------------------------------
Thanks Dave!
I have still some issues with generated code... introduced with THRIFT-1454
I think we should start to use a defined standard, e.g. -std=c++0x
In addition I requested llvm on the build slaves, but it does not work...
https://builds.apache.org/job/Thrift-llvm/
I'm able to compile the Thrift Compiler with clang++ on my 32-bit Debian squeeze
was (Author: roger.meier):
Thanks Dave!
I have still some issues with generated code...
I think we should use a defined standard, e.g. -std=c++0x
In addition I requested llvm on the build slaves, but it does not work...
https://builds.apache.org/job/Thrift-llvm/
I'm able to compile the Thrift Compiler with clang++ on my 32-bit Debian squeeze
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Dave Watson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13165432#comment-13165432 ]
Dave Watson commented on THRIFT-1442:
-------------------------------------
Ah thanks for the link, I've updated my test environment to include those flags.
I've attached patch to fix that particular warning, however my environment (gcc 4.6) I get a lot more. Here's some random warning fixes to make mine build, not sure if you need any of them
Fix unused variable if a struct contains no members:
{code}
diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc
index 0354a5f..e923be1 100755
--- a/compiler/cpp/src/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/generate/t_cpp_generator.cc
@@ -1505,13 +1505,16 @@ void t_cpp_generator::generate_struct_swap(ofstream& out, t_struct* tstruct) {
out <<
indent() << "using ::std::swap;" << endl;
+ // Prevent unused variable warnings
+ indent(out) << "(void)a;" << endl;
+ indent(out) << "(void)b;" << endl;
+
bool has_nonrequired_fields = false;
const vector<t_field*>& fields = tstruct->get_members();
for (vector<t_field*>::const_iterator f_iter = fields.begin();
f_iter != fields.end();
++f_iter) {
t_field *tfield = *f_iter;
- t_type *ttype = get_true_type(tfield->get_type());
if (tfield->get_req() != t_field::T_REQUIRED) {
has_nonrequired_fields = true;
{code}
I'm not sure what is correct here for unnamed_oprot_seqid regarding THRIFT-1165, it still warns for me:
{code}
diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc
index 0354a5f..6974f84 100755
--- a/compiler/cpp/src/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/generate/t_cpp_generator.cc
@@ -3164,8 +3164,7 @@ void t_cpp_generator::generate_process_function(t_service* tservice,
out <<
indent() << "template <class Protocol_>" << endl;
}
- const bool unnamed_oprot_seqid = tfunction->is_oneway() &&
- !(gen_templates_ && !specialized);
+ const bool unnamed_oprot_seqid = tfunction->is_oneway();
out <<
"void " << tservice->get_name() << "Processor" << class_suffix << "::" <<
"process_" << tfunction->get_name() << "(" <<
{code}
depending on ifdefs, milliseconds var is unused:
{code}
diff --git a/lib/cpp/src/concurrency/Mutex.cpp b/lib/cpp/src/concurrency/Mutex.cpp
index 0cfa0ad..332d415 100644
--- a/lib/cpp/src/concurrency/Mutex.cpp
+++ b/lib/cpp/src/concurrency/Mutex.cpp
@@ -154,6 +154,7 @@ class Mutex::impl {
PROFILE_MUTEX_NOT_LOCKED();
return false;
#else
+ (void)milliseconds;
// If pthread_mutex_timedlock isn't supported, the safest thing to do
// is just do a nonblocking trylock.
return trylock();
{code}
printf seems to be wrong type:
{code}
diff --git a/lib/cpp/src/transport/TFileTransport.cpp b/lib/cpp/src/transport/TFileTransport.cpp
index 47dc328..351f950 100644
--- a/lib/cpp/src/transport/TFileTransport.cpp
+++ b/lib/cpp/src/transport/TFileTransport.cpp
@@ -68,6 +68,7 @@ using namespace apache::thrift::concurrency;
#define CLOCK_REALTIME 0
static int clock_gettime(int clk_id /*ignored*/, struct timespec *tp) {
struct timeval now;
+ (void)clk_id;
int rv = gettimeofday(&now, NULL);
if (rv != 0) {
@@ -783,7 +784,7 @@ bool TFileTransport::isEventCorrupted() {
} else if( ((offset_ + readState_.bufferPtr_ - 4)/chunkSize_) !=
((offset_ + readState_.bufferPtr_ + readState_.event_->eventSize_ - 1)/chunkSize_) ) {
// 3. size indicates that event crosses chunk boundary
- T_ERROR("Read corrupt event. Event crosses chunk boundary. Event size:%u Offset:%lu",
+ T_ERROR("Read corrupt event. Event crosses chunk boundary. Event size:%u Offset:%lli",
readState_.event_->eventSize_,
(offset_ + readState_.bufferPtr_ + 4));
@@ -825,7 +826,7 @@ void TFileTransport::performRecovery() {
readState_.resetState(readState_.lastDispatchPtr_);
currentEvent_ = NULL;
char errorMsg[1024];
- sprintf(errorMsg, "TFileTransport: log file corrupted at offset: %lu",
+ sprintf(errorMsg, "TFileTransport: log file corrupted at offset: %lli",
(offset_ + readState_.lastDispatchPtr_));
GlobalOutput(errorMsg);
{code}
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Roger Meier (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Roger Meier resolved THRIFT-1442.
---------------------------------
Resolution: Fixed
Thanks Dave, warnings are fixed.
I created a new issue THRIFT-1458 to focus on C++ standard.
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13165502#comment-13165502 ]
Hudson commented on THRIFT-1442:
--------------------------------
Integrated in Thrift #354 (See [https://builds.apache.org/job/Thrift/354/])
THRIFT-1442 TNonblockingServer: Refactor to allow multiple IO Threads
Fix Warnings
Patch: Dave Watson
roger : http://svn.apache.org/viewvc/?view=rev&rev=1212067
Files :
* /thrift/trunk/lib/cpp/src/concurrency/Mutex.cpp
* /thrift/trunk/lib/cpp/src/server/TNonblockingServer.cpp
* /thrift/trunk/lib/cpp/src/server/TNonblockingServer.h
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch, nonblocking_unsigned.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (THRIFT-1442) ] TNonblockingServer: Refactor to
allow multiple IO
Posted by "Dave Watson (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dave Watson updated THRIFT-1442:
--------------------------------
Attachment: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
> ] TNonblockingServer: Refactor to allow multiple IO
> ----------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Priority: Minor
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Reopened] (THRIFT-1442) TNonblockingServer: Refactor to
allow multiple IO Threads
Posted by "Roger Meier (Reopened) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Roger Meier reopened THRIFT-1442:
---------------------------------
Dave, could you please fix the warning issues as well?
{noformat}
src/server/TNonblockingServer.cpp: In member function 'apache::thrift::server::TNonblockingServer::TConnection* apache::thrift::server::TNonblockingServer::createConnection(int, const sockaddr*, socklen_t)':
src/server/TNonblockingServer.cpp:870: error: comparison between signed and unsigned integer expressions
src/server/TNonblockingServer.cpp: In member function 'virtual void apache::thrift::server::TNonblockingServer::stop()':
src/server/TNonblockingServer.cpp:1161: error: comparison between signed and unsigned integer expressions
src/server/TNonblockingServer.cpp: In member function 'virtual void apache::thrift::server::TNonblockingServer::serve()':
src/server/TNonblockingServer.cpp:1180: error: comparison between signed and unsigned integer expressions
src/server/TNonblockingServer.cpp:1214: error: comparison between signed and unsigned integer expressions
src/server/TNonblockingServer.cpp:1226: error: comparison between signed and unsigned integer expressions
src/server/TNonblockingServer.cpp: At global scope:
src/server/TNonblockingServer.cpp:1353: error: unused parameter 'which'
{noformat}
We've setup a build job which enables more compiuler warnings to improve the code quality, see:
https://builds.apache.org/view/S-Z/view/Thrift/job/Thrift-cpp/
Thanks,
Roger
> TNonblockingServer: Refactor to allow multiple IO Threads
> ---------------------------------------------------------
>
> Key: THRIFT-1442
> URL: https://issues.apache.org/jira/browse/THRIFT-1442
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Reporter: Dave Watson
> Assignee: Dave Watson
> Priority: Minor
> Fix For: 0.9
>
> Attachments: 0001-TNonblockingServer-Refactor-to-allow-multiple-IO-thr.patch
>
>
> From 04fc8cbc24c64e1b68a23a1df2c46056785c269d Mon Sep 17 00:00:00 2001
> From: Mark Rabkin <mr...@fb.com>
> Date: Tue, 18 May 2010 22:16:09 +0000
> Subject: [PATCH 01/56] TNonblockingServer: Refactor to allow multiple IO
> threads, not just one
> davejwatson: This diff ads multiple IO threads to TNonblocking Server.
> We use it extensively, it's pretty well tested other than merge errors.
> This diff reverts THRIFT-1184, allowing re-use of an existing
> event base. With multiple IO threads, re-using a single event base doesn't
> make sense, so this seemed ok
> Summary:
> The diff creates multiple IO threads at startup -- the number of threads in
> this diff is fixed at server start and cannot change for simplicity. The
> first thread (id = 0) also doubles as the listen/accept thread, so there is
> still only a single thread doing accepts. The other IO threads listen only
> on their notification pipe and the actual connection sockets.
> Also, for simplicity, each accepted connection is simply assigned in a
> round-robin fashion to the next IO thread and lives on that IO thread
> permanently.
> Note that there are only trivial changes to TConnection to get it to work, so
> most of the tricky state transitions and buffer management are unchanged.
> There is still a single worker pool for processing tasks, so that code is
> unchanged as well.
> The trickiest part of the diff requiring the most careful review is the new
> synchronization code in the TNonblockingServer to manage the connection stack
> and counters of active/inactive connections. We now lock a mutex when
> incrementing/decrementing server counters, which is less than ideal for
> extremely high-QPS servers -- should I switch to atomic ops?
> One important change here is that while connections are created and initialized
> by the listen thread (IO thread #0), they may now be assigned to an event_base
> owned by a different IO thread. To work safely, TConnection::init() no longer
> calls setFlags() - instead, it immediately calls TConnection::notifyIOThread().
> This results in a notification-fd event in the correct event base, which then
> calls TConnection::transition() which sets up correct read flags. This means
> that a TConnection now calls notifyIOThread() once upon creation and assignment
> to its event_base, and thereafter after each processing call completes.
> TNonblocking server: Allow high-priority scheduler for IO threads
> Summary:
> Adds a boolean option to TNonblockingServer to use sched_setscheduler() to set
> a high scheduling priority for the IO thread(s) -- this is a POSIX api and
> should be safe on most operating systems.
> Please let me know if this is a known terrible idea, but we're experimenting
> to see if this helps the situation where we have 40 worker threads and 1 IO
> thread and the IO thread doesn't get scheduled nearly often enough.
> Reviewers: dreiss,edhall,putivsky
> Test Plan: Need to work with Ed to run his capacity-loadtesting scripts to verify performance.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira