From 8c1c18eda8f56a23a88ddfe7e9cc3d58e461af22 Mon Sep 17 00:00:00 2001
From: dormando <dormando@rydia.net>
Date: Tue, 3 Jan 2012 18:27:52 -0800
Subject: [PATCH] no same-class reassignment, better errors

Add human parseable strings to the errors for slabs ressign. Also prevent
reassigning memory to the same source and destination.
---
 memcached.c           | 13 ++++++++-----
 scripts/mc_slab_mover |  2 +-
 slabs.c               |  6 +++++-
 slabs.h               |  2 +-
 t/slabs_reassign.t    |  6 +++---
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/memcached.c b/memcached.c
index 496ec13ce4..a86248f339 100644
--- a/memcached.c
+++ b/memcached.c
@@ -3346,19 +3346,22 @@ static void process_command(conn *c, char *command) {
                 out_string(c, "OK");
                 break;
             case REASSIGN_RUNNING:
-                out_string(c, "BUSY");
+                out_string(c, "BUSY currently processing reassign request");
                 break;
             case REASSIGN_BADCLASS:
-                out_string(c, "BADCLASS");
+                out_string(c, "BADCLASS invalid src or dst class id");
                 break;
             case REASSIGN_NOSPARE:
-                out_string(c, "NOSPARE");
+                out_string(c, "NOSPARE source class has no spare pages");
                 break;
             case REASSIGN_DEST_NOT_FULL:
-                out_string(c, "NOTFULL");
+                out_string(c, "NOTFULL dest class has spare memory");
                 break;
             case REASSIGN_SRC_NOT_SAFE:
-                out_string(c, "UNSAFE");
+                out_string(c, "UNSAFE src class is in an unsafe state");
+                break;
+            case REASSIGN_SRC_DST_SAME:
+                out_string(c, "SAME src and dst class are identical");
                 break;
             }
             return;
diff --git a/scripts/mc_slab_mover b/scripts/mc_slab_mover
index d7bd5e4839..ac984d7b7d 100755
--- a/scripts/mc_slab_mover
+++ b/scripts/mc_slab_mover
@@ -134,7 +134,7 @@ sub run {
             printf "  %02s: %-8s (pct  ) %-10s (pct    ) %-6s (pct  ) get_hits (pct   ) cmd_set (pct  )\n",
                 'sb', 'evicted', 'items', 'pages';
             for my $slab (@$sorted) {
-                printf "  %02d: %-8d (%.2f%%) %-10s (%.4f%%) %-6d (%.2f%%) %-8d (%.3f%%) %-7d (%.2f%%)\n", 
+                printf "  %02d: %-8d (%.2f%%) %-10s (%.4f%%) %-6d (%.2f%%) %-8d (%.3f%%) %-7d (%.2f%%)\n",
                     $slab->{slab}, $slab->{evicted_d},
                     $pct->($slab->{evicted_d}, $totals->{evicted_d}),
                     $slab->{number},
diff --git a/slabs.c b/slabs.c
index 10b12b9dcf..c56b678f10 100644
--- a/slabs.c
+++ b/slabs.c
@@ -474,7 +474,8 @@ static int slab_rebalance_start(void) {
     if (slab_rebal.s_clsid < POWER_SMALLEST ||
         slab_rebal.s_clsid > power_largest  ||
         slab_rebal.d_clsid < POWER_SMALLEST ||
-        slab_rebal.d_clsid > power_largest)
+        slab_rebal.d_clsid > power_largest  ||
+        slab_rebal.s_clsid == slab_rebal.d_clsid)
         no_go = -2;
 
     s_cls = &slabclass[slab_rebal.s_clsid];
@@ -733,6 +734,9 @@ static enum reassign_result_type do_slabs_reassign(int src, int dst) {
     if (slab_rebalance_signal != 0)
         return REASSIGN_RUNNING;
 
+    if (src == dst)
+        return REASSIGN_SRC_DST_SAME;
+
     if (src < POWER_SMALLEST || src > power_largest ||
         dst < POWER_SMALLEST || dst > power_largest)
         return REASSIGN_BADCLASS;
diff --git a/slabs.h b/slabs.h
index ea83e45573..730474ba16 100644
--- a/slabs.h
+++ b/slabs.h
@@ -38,7 +38,7 @@ void stop_slab_maintenance_thread(void);
 
 enum reassign_result_type {
     REASSIGN_OK=0, REASSIGN_RUNNING, REASSIGN_BADCLASS, REASSIGN_NOSPARE,
-    REASSIGN_DEST_NOT_FULL, REASSIGN_SRC_NOT_SAFE
+    REASSIGN_DEST_NOT_FULL, REASSIGN_SRC_NOT_SAFE, REASSIGN_SRC_DST_SAME
 };
 
 enum reassign_result_type slabs_reassign(int src, int dst);
diff --git a/t/slabs_reassign.t b/t/slabs_reassign.t
index 85a8c2bcfe..aac83fcf07 100644
--- a/t/slabs_reassign.t
+++ b/t/slabs_reassign.t
@@ -42,7 +42,7 @@ sleep 2;
 
 # Check that stats counters increased
 my $slabs_after = mem_stats($sock, "slabs");
-my $stats = mem_stats($sock);
+$stats = mem_stats($sock);
 
 isnt($stats->{slabs_moved}, 0, "slabs moved is nonzero");
 
@@ -54,12 +54,12 @@ ok($slabs_before->{"25:total_pages"} != $slabs_after->{"25:total_pages"},
 
 # Try to move another slab, see that it complains
 print $sock "slabs reassign 31 25\r\n";
-is(scalar <$sock>, "NOTFULL\r\n", "Cannot re-run against class with empty space");
+is(scalar <$sock>, "NOTFULL dest class has spare memory\r\n", "Cannot re-run against class with empty space");
 
 # Try to move a page backwards. Should complain that source class isn't "safe"
 # to move from.
 print $sock "slabs reassign 25 31\r\n";
-is(scalar <$sock>, "UNSAFE\r\n", "Cannot move an unsafe slab back");
+is(scalar <$sock>, "UNSAFE src class is in an unsafe state\r\n", "Cannot move an unsafe slab back");
 
 # Try to insert items into both slabs
 print $sock "set bfoo51 0 0 70000\r\n", $bigdata, "\r\n";