Fixed a never experienced, theoretical bug that can actually happen in practice....

diff --git a/TODO b/TODO
index 1ddf6f5d935cc88c60a1363f544d66780152c129..5e592c99a827b41334516b5c18b457e35143a860 100644 (file)
--- a/TODO
+++ b/TODO
@@ -18,6 +18,7 @@ Virtual Memory sub-TODO:
 * it should be possible to give the vm-max-memory option in megabyte, gigabyte, ..., just using 2GB, 100MB, and so forth.
 * Make sure to wait all the IO threads are done before to fork() for BGSAVE and BGREWRITEAOF
 * Enlarge the stack of threads, to problems when calling LZF lib.
+* redis-cli vmstat, calling INFO every second and printing VM stats ala vmstat.
 
 VERSION 1.6 TODO (Virtual memory)
 =================================

diff --git a/redis.c b/redis.c
index ce047a406bf570075a6b97f7f52e40ab8fa2d447..f1ce18fbe33ce9c1a95dd6b7d5a2f49a2940f86c 100644 (file)
--- a/redis.c
+++ b/redis.c
@@ -7573,6 +7573,7 @@ static void vmCancelThreadedIOJob(robj *o) {
     int i;
 
     assert(o->storage == REDIS_VM_LOADING || o->storage == REDIS_VM_SWAPPING);
+again:
     lockThreadedIO();
     /* Search for a matching key in one of the queues */
     for (i = 0; i < 3; i++) {
@@ -7585,8 +7586,8 @@ static void vmCancelThreadedIOJob(robj *o) {
 
             if (job->canceled) continue; /* Skip this, already canceled. */
             if (compareStringObjects(job->key,o) == 0) {
-                redisLog(REDIS_DEBUG,"*** CANCELED %p (%s)\n",
-                    (void*)job, (char*)o->ptr);
+                redisLog(REDIS_DEBUG,"*** CANCELED %p (%s) (LIST ID %d)\n",
+                    (void*)job, (char*)o->ptr, i);
                 /* Mark the pages as free since the swap didn't happened
                  * or happened but is now discarded. */
                 if (job->type == REDIS_IOJOB_DO_SWAP)
@@ -7601,7 +7602,27 @@ static void vmCancelThreadedIOJob(robj *o) {
                     listDelNode(lists[i],ln);
                     break;
                 case 1: /* io_processing */
+                    /* Oh Shi- the thread is messing with the Job, and
+                     * probably with the object if this is a
+                     * PREPARE_SWAP or DO_SWAP job. Better to wait for the
+                     * job to move into the next queue... */
+                    if (job->type != REDIS_IOJOB_LOAD) {
+                        /* Yes, we try again and again until the job
+                         * is completed. */
+                        unlockThreadedIO();
+                        /* But let's wait some time for the I/O thread
+                         * to finish with this job. After all this condition
+                         * should be very rare. */
+                        usleep(1);
+                        goto again;
+                    } else {
+                        job->canceled = 1;
+                        break;
+                    }
                 case 2: /* io_processed */
+                    /* The job was already processed, that's easy...
+                     * just mark it as canceled so that we'll ignore it
+                     * when processing completed jobs. */
                     job->canceled = 1;
                     break;
                 }