Merge lp:~athomason/libmemcached/test-big-binary-mget into lp:~tangent-org/libmemcached/trunk

[Adam Thomason]

[Adam Thomason (athomason)]

Test for large binary mget issue: http://lists.tangent.org/pipermail/libmemcached/2009-August/000918.html

[Trond Norbye (trond-norbye)]

1) The test case doesn't use the memcached servers started by the test suite, but tries to connect to localhost:11211. You should connect to the servers started by the test suite instead. If you want to limit the number of servers being used, you should create a clone with memcached_clone and set memc_clone->number_of_hosts to the number of hosts you want (BUT ADD A COMMENT THAT THIS IS A DIRTY HACK JUST USED IN THE TEST FRAMEWORK AND SHOULDN'T BE USED OUTSIDE THE TEST FRAMEWORK!!)

2) Test cases should return TEST_SUCCESS/TEST_FAIL and not MEMCACHED_SUCCESS (different enums)

3) You can use calloc instead of malloc followed by a memset call, and you can assign a void* to everything according to C99 standard so you don't need any casting there...

review: Needs Fixing

[Adam Thomason (athomason)]

Trond Norbye wrote:
> Review: Needs Fixing
> 1) The test case doesn't use the memcached servers started by the test suite, but tries to connect to localhost:11211. You should connect to the servers started by the test suite instead.
Actually not, it's 11221, the first one the tests start. I didn't see an
obvious bit of what I wanted to do elsewhere in the tests, but point taken.
> If you want to limit the number of servers being used, you should create a clone with memcached_clone and set memc_clone->number_of_hosts to the number of hosts you want (BUT ADD A COMMENT THAT THIS IS A DIRTY HACK JUST USED IN THE TEST FRAMEWORK AND SHOULDN'T BE USED OUTSIDE THE TEST FRAMEWORK!!)
>
No need to yell at your own suggestion ;). The number of servers is
actually irrelevant, I just need a fresh memcached_st. memcached_clone
does seem simpler and more correct, so I'll use that.
> 2) Test cases should return TEST_SUCCESS/TEST_FAIL and not MEMCACHED_SUCCESS (different enums)
>
Sure. To prevent future annoyances I'd suggest cleaning up the rest of
the tests to do the same since newbies like me are likely to just copy
surrounding examples :).
> 3) You can use calloc instead of malloc followed by a memset call, and you can assign a void* to everything according to C99 standard so you don't need any casting there...
>
Similarly, that's code just borrowed from user_supplied_bug3. I'll fix
up all the malloc/memset instances that I see.

[lp:~athomason/libmemcached/test-big-binary-mget]

582. By Adam Thomason on 2009-09-22

Address trond's comments

583. By Adam Thomason on 2009-09-22

Fix return codes

584. By Adam Thomason on 2009-09-22

Factor out fetch-all-requests code marked "Turn this into a help function" into fetch_all_results help function

[Brian Aker (brianaker)]

Hi!

I just wanted to update you. We are still working on this... we have a
new interface we will be putting together to solve this problem.

We have not forgotten you :)

Cheers,
    -Brian

=== modified file 'tests/function.c'
--- tests/function.c	2009-09-10 10:57:11 +0000
+++ tests/function.c	2009-09-21 20:29:20 +0000
@@ -11,6 +11,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <signal.h>
 #include <unistd.h>
 #include <time.h>
 #include "server.h"
@@ -2457,6 +2458,105 @@
   return 0;
 }
 
+/* Large mget() of missing keys with binary proto
+ * See http://lists.tangent.org/pipermail/libmemcached/2009-August/000918.html
+ */
+
+void fail(int);
+
+static test_return  _user_supplied_bug21(size_t key_count)
+{
+  memcached_return rc;
+  unsigned int x;
+  char **keys;
+  size_t* key_lengths;
+  void (*oldalarm)(int);
+  memcached_st *memc;
+
+  key_lengths = malloc(sizeof(size_t) * key_count);
+
+  memc= memcached_create(NULL);
+  assert(memc);
+
+  /* only binproto uses getq for mget */
+  memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL, 1);
+
+  rc= memcached_server_add(memc, "localhost", 11221);
+
+  /* empty the cache to ensure misses (hence non-responses) */
+  rc= memcached_flush(memc, 0);
+  assert(rc == MEMCACHED_SUCCESS);
+
+  keys= (char **)malloc(sizeof(char *) * key_count);
+  assert(keys);
+  memset(keys, 0, (sizeof(char *) * key_count));
+  for (x= 0; x < key_count; x++)
+  {
+    char buffer[30];
+
+    snprintf(buffer, 30, "%u", x);
+    keys[x]= strdup(buffer);
+    key_lengths[x]= strlen(keys[x]);
+  }
+
+  oldalarm = signal(SIGALRM, fail);
+  alarm(5);
+
+  rc= memcached_mget(memc, (const char **)keys, key_lengths, key_count);
+  assert(rc == MEMCACHED_SUCCESS);
+
+  alarm(0);
+  signal(SIGALRM, oldalarm);
+
+  /* Turn this into a help function */
+  {
+    char return_key[MEMCACHED_MAX_KEY];
+    size_t return_key_length;
+    char *return_value;
+    size_t return_value_length;
+    uint32_t flags;
+
+    while ((return_value= memcached_fetch(memc, return_key, &return_key_length, 
+                                          &return_value_length, &flags, &rc)))
+    {
+      assert(return_value);
+      assert(rc == MEMCACHED_SUCCESS);
+      free(return_value);
+    }
+  }
+
+  for (x= 0; x < key_count; x++)
+    free(keys[x]);
+  free(keys);
+
+  memcached_free(memc);
+
+  free(key_lengths);
+
+  return MEMCACHED_SUCCESS;
+}
+
+static test_return  user_supplied_bug21(memcached_st *trash)
+{
+  (void)trash;
+  memcached_return rc;
+
+  /* should work as of r580 */
+  rc = _user_supplied_bug21(10);
+  assert(rc == MEMCACHED_SUCCESS);
+
+  /* should fail as of r580 */
+  rc = _user_supplied_bug21(1000);
+  assert(rc == MEMCACHED_SUCCESS);
+
+  return MEMCACHED_SUCCESS;
+}
+
+void fail(int unused __attribute__((unused)))
+{
+  assert(0);
+}
+
 static test_return auto_eject_hosts(memcached_st *trash)
 {
   (void) trash;
@@ -4526,6 +4626,7 @@
   {"user_supplied_bug18", 1, user_supplied_bug18 },
   {"user_supplied_bug19", 1, user_supplied_bug19 },
   {"user_supplied_bug20", 1, user_supplied_bug20 },
+  {"user_supplied_bug21", 0, user_supplied_bug21 },
   {0, 0, 0}
 };