bytemark/flexnbd-c
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

listen: Turn off CLIENT_MAX_WAIT_SECS

Browse Source 
The idea behind this feature was to avoid the client thread in a listen
server getting stuck forever if the mirroring thread in the source died.
However, it breaks any sane implementation of max_Bps in that thread,
and there are lingering concerns over how it might operate under normal
conditions anyway.

Specifically, if iterating over the bitmap takes a long time, or even just
reading the requisite 8MB from the disc in order to send it, then the
5-second timeout could be hit, causing mirroring to fail unnecessarily.
This commit is contained in:
nick
2013-08-14 16:09:55 +01:00
parent d0022402ae
commit efdd613968
3 changed files with 18 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/client.c
View File

@@ -192,14 +192,19 @@ int client_read_request( struct client * client , struct nbd_request *out_reques
struct nbd_request_raw request_raw; struct nbd_request_raw request_raw;
fd_set fds; fd_set fds;
struct timeval tv = {CLIENT_MAX_WAIT_SECS, 0}; struct timeval * ptv = NULL;
struct timeval * ptv;
int fd_count; int fd_count;
/* We want a timeout if this is an inbound migration, but not /* We want a timeout if this is an inbound migration, but not otherwise.
* otherwise * This is compile-time selectable, as it will break mirror max_bps
*/ */
ptv = server_is_in_control( client->serve ) ? NULL : &tv; #ifdef HAS_LISTEN_TIMEOUT
struct timeval tv = {CLIENT_MAX_WAIT_SECS, 0};
if ( !server_is_in_control( client->serve ) ) {
ptv = &tv;
}
#endif
FD_ZERO(&fds); FD_ZERO(&fds);
FD_SET(client->socket, &fds); FD_SET(client->socket, &fds);

4
src/client.h
View File

@@ -4,6 +4,8 @@
#include <signal.h> #include <signal.h>
#include <time.h> #include <time.h>
#ifdef HAS_LISTEN_TIMEOUT
/** CLIENT_MAX_WAIT_SECS /** CLIENT_MAX_WAIT_SECS
* This is the length of time an inbound migration will wait for a fresh * This is the length of time an inbound migration will wait for a fresh
* write before assuming the source has Gone Away. Note: it is *not* * write before assuming the source has Gone Away. Note: it is *not*
@@ -12,6 +14,8 @@
*/ */
#define CLIENT_MAX_WAIT_SECS 5 #define CLIENT_MAX_WAIT_SECS 5
#endif
/** CLIENT_HANDLER_TIMEOUT /** CLIENT_HANDLER_TIMEOUT
* This is the length of time (in seconds) any request can be outstanding for. * This is the length of time (in seconds) any request can be outstanding for.
* If we spend longer than this in a request, the whole server is killed. * If we spend longer than this in a request, the whole server is killed.

6
tests/acceptance/test_dest_error_handling.rb
View File

@@ -21,12 +21,13 @@ class TestDestErrorHandling < Test::Unit::TestCase
assert_no_control assert_no_control
end end
=begin
# This is disabled while CLIENT_MAX_WAIT_SECS is removed
def test_hello_goes_astray_causes_timeout_error def test_hello_goes_astray_causes_timeout_error
run_fake( "source/hang_after_hello" ) run_fake( "source/hang_after_hello" )
assert_no_control assert_no_control
end end
=end
def test_sigterm_has_bad_exit_status def test_sigterm_has_bad_exit_status
@env.nbd1.can_die(1) @env.nbd1.can_die(1)
@@ -99,3 +100,4 @@ class TestDestErrorHandling < Test::Unit::TestCase
end end
end # class TestDestErrorHandling end # class TestDestErrorHandling