Fixed a few random things.

src/Cache.cpp

@@ -384,7 +384,12 @@ void Cache::rename(const string &oldKey, const string &newKey, ssize_t sizediff)
 {
     boost::unique_lock<boost::mutex> s(lru_mutex);
     auto it = m_lru.find(oldKey);
-    assert(it != m_lru.end());
+    //assert(it != m_lru.end());
+    if (it == m_lru.end())
+    {
+        logger->log(LOG_ERR, "Cache: was told to rename %s, but it does not exist", oldKey.string().c_str());
+        assert(0);
+    }
     
     auto lit = it->lit;
     m_lru.erase(it);

src/IOCoordinator.cpp

@@ -578,7 +578,12 @@ void IOCoordinator::deleteMetaFile(const bf::path &file)
     */
 
     Synchronizer *synchronizer = Synchronizer::get();
-    ScopedWriteLock lock(this, file.string());
+
+    
+    // this is kind of ugly.  We need to lock on file relative to metaPath
+    string lockKey = file.string().substr(metaPath.string().length() + 1);
+    cout << "Deletemetafile locking on " << lockKey << endl;
+    ScopedWriteLock lock(this, lockKey);
     
     MetadataFile meta(file);
     replicator->remove(file);
@@ -809,26 +814,6 @@ int IOCoordinator::copyFile(const char *filename1, const char *filename2)
     for (auto &jEntry : newJournalEntries)
         sync->newJournalEntry(jEntry);
     return 0;
-
-#if 0
-    SMLogging* logger = SMLogging::get();
-    int err = 0, l_errno;
-    try {
-        bf::copy_file(filename1, filename2);
-    }
-    catch (bf::filesystem_error &e) {
-        err = -1;
-        l_errno = e.code().value();   // why not.
-        // eh, not going to translate all of boost's errors into our errors for this.
-        // log the error
-        logger->log(LOG_ERR,"IOCoordinator::copy(): got %s",e.what());
-    }
-    catch (...) {
-        err = -1;
-        l_errno = EIO;
-    }
-    return err;
-#endif
 }
 
 const bf::path &IOCoordinator::getCachePath() const
@@ -1003,7 +988,7 @@ int IOCoordinator::mergeJournalInMem(boost::shared_array<uint8_t> &objData, size
     ss << headertxt.get();
     boost::property_tree::ptree header;
     boost::property_tree::json_parser::read_json(ss, header);
-    assert(header.get<int>("version") == "1");
+    assert(header.get<int>("version") == 1);
     size_t maxJournalOffset = header.get<size_t>("max_offset"); 
     
     if (maxJournalOffset > *len)

src/Replicator.cpp

@@ -187,6 +187,9 @@ int Replicator::remove(const boost::filesystem::path &filename, Flags flags)
 {
     int ret = 0;
     
+    if (flags & NO_LOCAL)
+        return 0;  // not implemented yet
+        
     try
     {
         boost::filesystem::remove_all(filename);
@@ -202,6 +205,9 @@ int Replicator::remove(const boost::filesystem::path &filename, Flags flags)
 
 int Replicator::remove(const char *filename, Flags flags)
 {
+    if (flags & NO_LOCAL)
+        return 0;   // not implemented yet
+        
     boost::filesystem::path p(filename);
     return remove(p);
 }

src/Synchronizer.cpp

@@ -229,13 +229,17 @@ void Synchronizer::process(list<string>::iterator name, bool callerHoldsLock)
         mutex.unlock();
     
     bool success = false;
-    
-    // in testing, this seems to only run once when an exception is caught.  Not obvious why yet....  ??
     while (!success)
     {    
         try {
-            /* Exceptions should only happen b/c of cloud service errors.  Rather than retry here endlessly,
-               probably a better idea to have cloudstorage classes do the retrying */
+            // Exceptions should only happen b/c of cloud service errors that can't be retried.
+            // This code is intentionally racy to avoid having to grab big locks.
+            // In particular, it's possible that by the time synchronize() runs,
+            // the file to sync has already been deleted.  When one of these functions
+            // encounters a state that doesn't make sense, such as being told to upload a file
+            // that doesn't exist, it will return silently under the assumption that
+            // things are working as they should upstream, and a syncDelete() call will be coming
+            // shortly.
             if (pending->opFlags & DELETE)
                 synchronizeDelete(sourceFile, name);
             else if (pending->opFlags & JOURNAL)
@@ -283,23 +287,25 @@ void Synchronizer::synchronize(const string &sourceFile, list<string>::iterator
     if (exists)
         return;
 
-    // can this run after a delete op?
     exists = bf::exists(cachePath / key);
     if (!exists)
     {
-        logger->log(LOG_WARNING, "synchronize(): was told to upload %s but it does not exist locally", key.c_str());
+        logger->log(LOG_DEBUG, "synchronize(): was told to upload %s but it does not exist locally", key.c_str());
         return;
     }
 
     err = cs->putObject((cachePath / key).string(), key);
     if (err)
         throw runtime_error(string("synchronize(): uploading ") + key + ", got " + strerror_r(errno, buf, 80));
-    replicator->remove(key.c_str(), Replicator::NO_LOCAL);
+    replicator->remove((cachePath/key).string().c_str(), Replicator::NO_LOCAL);
 }
 
 void Synchronizer::synchronizeDelete(const string &sourceFile, list<string>::iterator &it)
 {
-    ScopedWriteLock s(ioc, sourceFile);
+    /* Don't think it's necessary to lock here.  Sync is being told to delete a file on cloud storage.
+       Presumably the caller has removed it from metadata & the cache, so it is no longer referencable.  
+    */
+    //ScopedWriteLock s(ioc, sourceFile);
     cs->deleteObject(*it);
 }
 
@@ -313,8 +319,9 @@ void Synchronizer::synchronizeWithJournal(const string &sourceFile, list<string>
     
     if (!bf::exists(journalName))
     {
-        logger->log(LOG_WARNING, "synchronizeWithJournal(): no journal file found for %s", key.c_str());
-        // I don't think this should happen, maybe throw a logic_error here
+        logger->log(LOG_DEBUG, "synchronizeWithJournal(): no journal file found for %s", key.c_str());
+        // I don't think this should happen, maybe throw a logic_error here.
+        // Revision ^^.  It can happen if the object was deleted after the op was latched but before it runs.
         return;
     }
 
@@ -330,7 +337,14 @@ void Synchronizer::synchronizeWithJournal(const string &sourceFile, list<string>
     {
         err = cs->getObject(key, &data, &size);
         if (err)
+        {
+            if (errno == ENOENT)
+            {
+                logger->log(LOG_DEBUG, "synchronizeWithJournal(): %s does not exist in cache nor in cloud storage", key.c_str());
+                return;
+            }
             throw runtime_error(string("Synchronizer: getObject() failed: ") + strerror_r(errno, buf, 80));
+        }
         err = ioc->mergeJournalInMem(data, &size, journalName.c_str());
         assert(!err);
     }
@@ -342,7 +356,12 @@ void Synchronizer::synchronizeWithJournal(const string &sourceFile, list<string>
     string newKey = MetadataFile::getNewKeyFromOldKey(key, size);
     err = cs->putObject(data, size, newKey);
     if (err)
+    {
+        // try to delete it in cloud storage... unlikely it is there in the first place, and if it is
+        // this probably won't work
+        cs->deleteObject(newKey);    
         throw runtime_error(string("Synchronizer: putObject() failed: ") + strerror_r(errno, buf, 80));
+    }
     
     // if the object was cached...
     //   write the new data to disk,
@@ -364,13 +383,15 @@ void Synchronizer::synchronizeWithJournal(const string &sourceFile, list<string>
         {
             err = ::write(newFD, data.get(), size - count);
             if (err < 0)
+            {
+                ::unlink(newCachePath.string().c_str());
                 throw runtime_error(string("Synchronizer: Failed to write to a new object in local storage!  Got ") 
                   + strerror_r(errno, buf, 80));
+            }
             count += err;
         }
-        
         cache->rename(key, newKey, size - bf::file_size(oldCachePath));
-        replicator->remove(key.c_str());   // should this be the file to remove, or the key?
+        replicator->remove(oldCachePath.string().c_str());
     }
     
     // update the metadata for the source file

src/unit_tests.cpp

@@ -1367,10 +1367,10 @@ void IOCCopyFile3()
     bf::path metaPath = ioc->getMetadataPath();
     bf::path journalPath = ioc->getJournalPath();
     bf::path cachePath = ioc->getCachePath();
-    bf::path sourcePath = metaPath/"copyfile1"/"source.meta";
-    bf::path destPath = metaPath/"copyfile2"/"dest.meta";
-    const char *l_sourceFile = "copyfile1/source";
-    const char *l_destFile = "copyfile2/dest";
+    bf::path sourcePath = metaPath/"copyfile3"/"source.meta";
+    bf::path destPath = metaPath/"copyfile4"/"dest.meta";
+    const char *l_sourceFile = "copyfile3/source";
+    const char *l_destFile = "copyfile4/dest";
     
     cache->reset();
     
@@ -1381,7 +1381,7 @@ void IOCCopyFile3()
     cache->newObject(testObjKey, bf::file_size(cachePath/testObjKey));
     cache->newJournalEntry(bf::file_size(journalPath/(string(testObjKey) + ".journal")));
     
-    int err = ioc->copyFile("copyfile1/source", "copyfile2/dest");
+    int err = ioc->copyFile("copyfile3/source", "copyfile4/dest");
     assert(!err);
     uint8_t buf1[8192], buf2[8192];
     err = ioc->read(l_sourceFile, buf1, 0, 8192);
@@ -1390,8 +1390,8 @@ void IOCCopyFile3()
     assert(err == 8192);
     assert(memcmp(buf1, buf2, 8192) == 0);
     
-    ioc->unlink("copyfile1");
-    ioc->unlink("copyfile2");
+    ioc->unlink("copyfile3");
+    ioc->unlink("copyfile4");
     assert(cache->getCurrentCacheSize() == 0);
     cout << "IOC copy file 3 OK" << endl;
 }