Merge lp:~jc.redoutey/libmemcached/io_flush into lp:~tangent-org/libmemcached/trunk

Proposed by JC Redoutey
Status: Work in progress
Proposed branch: lp:~jc.redoutey/libmemcached/io_flush
Merge into: lp:~tangent-org/libmemcached/trunk
Diff against target: 206 lines (+88/-39)
3 files modified
libmemcached/memcached_io.c (+40/-37)
libmemcached/memcached_server.h (+1/-0)
tests/function.c (+47/-2)
To merge this branch: bzr merge lp:~jc.redoutey/libmemcached/io_flush
Reviewer Review Type Date Requested Status
Libmemcached-developers Pending
Review via email: mp+15453@code.launchpad.net
To post a comment you must log in.
Revision history for this message
JC Redoutey (jc.redoutey) wrote :

the test case does not work as good as I hoped, I guess because network calls are in fact local. But anyway, io_flush needs to somehow support recursive calls, so here is a version proposal.

624. By Jean-Charles Redoutey <jc@Jayce2009lnx>

Removed wrongly introduced non portable call

Revision history for this message
Brian Aker (brianaker) wrote :

Anything still relevant in this?

Unmerged revisions

624. By Jean-Charles Redoutey <jc@Jayce2009lnx>

Removed wrongly introduced non portable call

623. By Jean-Charles Redoutey <jc@Jayce2009lnx>

Made io_flush reentrant, or at least enough for existing cases

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libmemcached/memcached_io.c'
--- libmemcached/memcached_io.c 2009-10-14 11:40:42 +0000
+++ libmemcached/memcached_io.c 2009-12-02 17:36:12 +0000
@@ -424,14 +424,13 @@
424 ssize_t sent_length;424 ssize_t sent_length;
425 size_t return_length;425 size_t return_length;
426 char *local_write_ptr= ptr->write_buffer;426 char *local_write_ptr= ptr->write_buffer;
427 size_t write_length= ptr->write_buffer_offset;
428427
429 *error= MEMCACHED_SUCCESS;428 *error= MEMCACHED_SUCCESS;
430429
431 WATCHPOINT_ASSERT(ptr->fd != -1);430 WATCHPOINT_ASSERT(ptr->fd != -1);
432431
433 // UDP Sanity check, make sure that we are not sending somthing too big432 // UDP Sanity check, make sure that we are not sending somthing too big
434 if (ptr->type == MEMCACHED_CONNECTION_UDP && write_length > MAX_UDP_DATAGRAM_LENGTH)433 if (ptr->type == MEMCACHED_CONNECTION_UDP && ptr->write_buffer_offset > MAX_UDP_DATAGRAM_LENGTH)
435 return -1;434 return -1;
436435
437 if (ptr->write_buffer_offset == 0 || (ptr->type == MEMCACHED_CONNECTION_UDP436 if (ptr->write_buffer_offset == 0 || (ptr->type == MEMCACHED_CONNECTION_UDP
@@ -439,24 +438,32 @@
439 return 0;438 return 0;
440439
441 /* Looking for memory overflows */440 /* Looking for memory overflows */
442#if defined(DEBUG)441 #if defined(DEBUG)
443 if (write_length == MEMCACHED_MAX_BUFFER)442 if (ptr->write_buffer_offset == MEMCACHED_MAX_BUFFER)
444 WATCHPOINT_ASSERT(ptr->write_buffer == local_write_ptr);443 WATCHPOINT_ASSERT(ptr->write_buffer == local_write_ptr);
445 WATCHPOINT_ASSERT((ptr->write_buffer + MEMCACHED_MAX_BUFFER) >= (local_write_ptr + write_length));444 WATCHPOINT_ASSERT((ptr->write_buffer + MEMCACHED_MAX_BUFFER) >= (local_write_ptr + ptr->write_buffer_offset));
446#endif445 #endif
447446
448 return_length= 0;447 return_length= 0;
449 while (write_length)448 while (ptr->write_buffer_offset)
450 {449 {
451 WATCHPOINT_ASSERT(ptr->fd != -1);450 WATCHPOINT_ASSERT(ptr->fd != -1);
452 WATCHPOINT_ASSERT(write_length > 0);451 WATCHPOINT_ASSERT(ptr->write_buffer_offset > 0);
453 sent_length= 0;452 if (ptr->type == MEMCACHED_CONNECTION_UDP)
454 if (ptr->type == MEMCACHED_CONNECTION_UDP)453 increment_udp_message_id(ptr);
455 increment_udp_message_id(ptr);454
456 sent_length= write(ptr->fd, local_write_ptr, write_length);455 sent_length= write(ptr->fd, local_write_ptr + ptr->write_buffer_start_offset, ptr->write_buffer_offset);
457456
458 if (sent_length == -1)457 if(sent_length > 0)
459 {458 {
459 ptr->io_bytes_sent += (uint32_t) sent_length;
460 ptr->write_buffer_start_offset += sent_length;
461 ptr->write_buffer_offset -= (uint32_t) sent_length;
462 return_length+= (uint32_t) sent_length;
463 }
464
465 if (sent_length == -1)
466 {
460 ptr->cached_errno= errno;467 ptr->cached_errno= errno;
461 switch (errno)468 switch (errno)
462 {469 {
@@ -490,23 +497,18 @@
490 }497 }
491 }498 }
492499
493 if (ptr->type == MEMCACHED_CONNECTION_UDP &&500 if (ptr->type == MEMCACHED_CONNECTION_UDP &&
494 (size_t)sent_length != write_length)501 (size_t)sent_length != ptr->write_buffer_offset)
495 {502 {
496 memcached_quit_server(ptr, 1);503 memcached_quit_server(ptr, 1);
497 return -1;504 return -1;
498 }505 }
499506
500 ptr->io_bytes_sent += (uint32_t) sent_length;507 }
501508
502 local_write_ptr+= sent_length;509 WATCHPOINT_ASSERT(ptr->write_buffer_offset == 0);
503 write_length-= (uint32_t) sent_length;510 // Need to study this assert() WATCHPOINT_ASSERT(return_length ==
504 return_length+= (uint32_t) sent_length;511 // ptr->write_buffer_offset);
505 }
506
507 WATCHPOINT_ASSERT(write_length == 0);
508 // Need to study this assert() WATCHPOINT_ASSERT(return_length ==
509 // ptr->write_buffer_offset);
510512
511 // if we are a udp server, the begining of the buffer is reserverd for513 // if we are a udp server, the begining of the buffer is reserverd for
512 // the upd frame header514 // the upd frame header
@@ -515,6 +517,7 @@
515 else517 else
516 ptr->write_buffer_offset= 0;518 ptr->write_buffer_offset= 0;
517519
520 ptr->write_buffer_start_offset = 0;
518 return (ssize_t) return_length;521 return (ssize_t) return_length;
519}522}
520523
521524
=== modified file 'libmemcached/memcached_server.h'
--- libmemcached/memcached_server.h 2009-10-10 11:57:03 +0000
+++ libmemcached/memcached_server.h 2009-12-02 17:36:12 +0000
@@ -33,6 +33,7 @@
33 size_t read_buffer_length;33 size_t read_buffer_length;
34 size_t read_data_length;34 size_t read_data_length;
35 size_t write_buffer_offset;35 size_t write_buffer_offset;
36 size_t write_buffer_start_offset;
36 struct addrinfo *address_info;37 struct addrinfo *address_info;
37 time_t next_retry;38 time_t next_retry;
38 memcached_st *root;39 memcached_st *root;
3940
=== modified file 'tests/function.c'
--- tests/function.c 2009-11-27 17:05:51 +0000
+++ tests/function.c 2009-12-02 17:36:12 +0000
@@ -3922,7 +3922,7 @@
3922}3922}
39233923
3924#ifdef HAVE_LIBMEMCACHEDUTIL3924#ifdef HAVE_LIBMEMCACHEDUTIL
3925static void* connection_release(void *arg) 3925static void* connection_release(void *arg)
3926{3926{
3927 struct {3927 struct {
3928 memcached_pool_st* pool;3928 memcached_pool_st* pool;
@@ -4169,7 +4169,7 @@
4169 rc= memcached_set(memc, keys[x], len[x], "1", 1, 0, 0);4169 rc= memcached_set(memc, keys[x], len[x], "1", 1, 0, 0);
4170 test_truth(rc == MEMCACHED_SUCCESS);4170 test_truth(rc == MEMCACHED_SUCCESS);
4171 }4171 }
4172 4172
4173 memcached_quit(memc);4173 memcached_quit(memc);
41744174
4175 for (int x=0; x< 7; ++x) {4175 for (int x=0; x< 7; ++x) {
@@ -5391,6 +5391,50 @@
5391 return TEST_SUCCESS;5391 return TEST_SUCCESS;
5392}5392}
53935393
5394
5395
5396/*
5397 * Test that ensures that buffered set to not trigger problems during io_flush
5398 */
5399static test_return_t regression_bug_490520(memcached_st *memc)
5400{
5401
5402 memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NO_BLOCK,1);
5403 memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BUFFER_REQUESTS,1);
5404 memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_POLL_TIMEOUT,1000);
5405 memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_SERVER_FAILURE_LIMIT,1);
5406 memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_RETRY_TIMEOUT,3600);
5407
5408 uint32_t number_of_hosts= memc->number_of_hosts;
5409 memc->number_of_hosts= 1;
5410 int max_keys= 200480;
5411
5412
5413 char **keys= calloc((size_t)max_keys, sizeof(char*));
5414 size_t *key_length=calloc((size_t)max_keys, sizeof(size_t));
5415
5416 /* First add all of the items.. */
5417 char blob[3333] = {0};
5418 memcached_return rc;
5419 for (int x= 0; x < max_keys; ++x)
5420 {
5421 char k[251];
5422 key_length[x]= (size_t)snprintf(k, sizeof(k), "0200%u", x);
5423 keys[x]= strdup(k);
5424 assert(keys[x] != NULL);
5425 rc= memcached_set(memc, keys[x], key_length[x], blob, sizeof(blob), 0, 0);
5426 assert(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED);
5427 }
5428
5429
5430 memc->number_of_hosts= number_of_hosts;
5431 return TEST_SUCCESS;
5432}
5433
5434
5435
5436
5437
5394test_st udp_setup_server_tests[] ={5438test_st udp_setup_server_tests[] ={
5395 {"set_udp_behavior_test", 0, set_udp_behavior_test},5439 {"set_udp_behavior_test", 0, set_udp_behavior_test},
5396 {"add_tcp_server_udp_client_test", 0, add_tcp_server_udp_client_test},5440 {"add_tcp_server_udp_client_test", 0, add_tcp_server_udp_client_test},
@@ -5567,6 +5611,7 @@
5567 {"lp:442914", 1, regression_bug_442914 },5611 {"lp:442914", 1, regression_bug_442914 },
5568 {"lp:447342", 1, regression_bug_447342 },5612 {"lp:447342", 1, regression_bug_447342 },
5569 {"lp:463297", 1, regression_bug_463297 },5613 {"lp:463297", 1, regression_bug_463297 },
5614 {"lp:490520", 1, regression_bug_490520 },
5570 {0, 0, 0}5615 {0, 0, 0}
5571};5616};
55725617

Subscribers

People subscribed via source and target branches

to all changes: