Use flock on root directory to acquire exclusive locks to guarantee consitency of files/zk model.

6 files changed, 42 insertions, 11 deletions

diff --git a/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h b/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h
index 36ccd6a332a..3b749642b1d 100644
--- a/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h
+++ b/filedistribution/src/vespa/filedistribution/distributor/filedownloader.h
@@ -69,6 +69,7 @@ public:
                    const boost::filesystem::path& dbPath,
                    const boost::shared_ptr<ExceptionRethrower>& exceptionRethrower);
     ~FileDownloader();
+    DirectoryGuard::UP getGuard() { return std::make_unique<DirectoryGuard>(_dbPath); }
 
     void runEventLoop();
     void addTorrent(const std::string& fileReference, const Buffer& buffer);
diff --git a/filedistribution/src/vespa/filedistribution/distributor/filedownloadermanager.cpp b/filedistribution/src/vespa/filedistribution/distributor/filedownloadermanager.cpp
index af66ed86afa..7bc57c57dd1 100644
--- a/filedistribution/src/vespa/filedistribution/distributor/filedownloadermanager.cpp
+++ b/filedistribution/src/vespa/filedistribution/distributor/filedownloadermanager.cpp
@@ -89,8 +89,7 @@ FileDownloaderManager::removePeerStatus(const std::string& fileReference) {
 void
 FileDownloaderManager::StartDownloads::downloadFile(const std::string& fileReference) {
     if (!_parent._fileDownloader->hasTorrent(fileReference)) {
-        Buffer torrent(
-                _parent._fileDistributionModel->getFileDBModel().getFile(fileReference));
+        Buffer torrent(_parent._fileDistributionModel->getFileDBModel().getFile(fileReference));
 
         _parent._fileDistributionModel->addPeer(fileReference);
         _parent._fileDownloader->addTorrent(fileReference, torrent);
@@ -102,10 +101,10 @@ void
 FileDownloaderManager::StartDownloads::operator()() {
     namespace ll = boost::lambda;
 
+    DirectoryGuard::UP guard = _parent._fileDownloader->getGuard();
     LockGuard updateFilesToDownloadGuard(_parent._updateFilesToDownloadMutex);
 
-    std::set<std::string> filesToDownload =
-            _parent._fileDistributionModel->getFilesToDownload();
+    std::set<std::string> filesToDownload = _parent._fileDistributionModel->getFilesToDownload();
     logStartDownload(filesToDownload);
 
     std::for_each(filesToDownload.begin(), filesToDownload.end(),
diff --git a/filedistribution/src/vespa/filedistribution/manager/filedb.h b/filedistribution/src/vespa/filedistribution/manager/filedb.h
index d44436f9e7f..8c84eccdae3 100644
--- a/filedistribution/src/vespa/filedistribution/manager/filedb.h
+++ b/filedistribution/src/vespa/filedistribution/manager/filedb.h
@@ -3,14 +3,15 @@
 
 #include <string>
 #include <boost/filesystem/path.hpp>
+#include "filedbmodel.h"
 
 namespace filedistribution {
 
 class FileDB {
     boost::filesystem::path _dbPath;
-    int _fd;
 public:
     FileDB(boost::filesystem::path dbPath);
+    DirectoryGuard::UP getGuard() { return std::make_unique<DirectoryGuard>(_dbPath); }
     void add(boost::filesystem::path original, const std::string& name);
 };
 
diff --git a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
index fde4302eee8..34b67083714 100644
--- a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
+++ b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
@@ -130,6 +130,7 @@ Java_com_yahoo_vespa_filedistribution_FileDistributionManager_addFileImpl(
         std::string fileReference = createTorrent.fileReference();
         NativeFileDistributionManager& manager = *nativeFileDistributionManagerField.get(self, env);
 
+        DirectoryGuard::UP guard = manager._fileDB->getGuard();// This prevents the filedistributor from working in an inconsistent state.
         manager._fileDB->add(completePath._value, fileReference);
 
         FileDBModel& model = *manager._fileDBModel;
diff --git a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
index 1c8ad181306..ea564668e45 100644
--- a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
+++ b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h
@@ -8,6 +8,15 @@
 
 namespace filedistribution {
 
+class DirectoryGuard {
+public:
+    typedef std::unique_ptr<DirectoryGuard> UP;
+    DirectoryGuard(boost::filesystem::path path);
+    ~DirectoryGuard();
+private:
+    int _fd;
+};
+
 struct InvalidProgressException : public Exception {
     const char* what() const throw() {
         return "Invalid progress information reported by one of the filedistributors";
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
index 3483a4eb359..e823b3300ba 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
+++ b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp
@@ -15,17 +15,17 @@
 
 namespace fs = boost::filesystem;
 
-using filedistribution::ZKFileDBModel;
+namespace filedistribution {
 
 namespace {
 
 fs::path
 createPath(const std::string& fileReference) {
-    return filedistribution::ZKFileDBModel::_fileDBPath / fileReference;
+    return ZKFileDBModel::_fileDBPath / fileReference;
 }
 
 void
-createNode(const fs::path & path, filedistribution::ZKFacade& zk) {
+createNode(const fs::path & path, ZKFacade& zk) {
     if (!zk.hasNode(path))
         zk.setData(path, "", 0);
 }
@@ -38,7 +38,7 @@ isEntryForHost(const std::string& host, const std::string& peerEntry) {
 }
 
 std::vector<std::string>
-getSortedChildren(filedistribution::ZKFacade& zk, const ZKFileDBModel::Path& path) {
+getSortedChildren(ZKFacade& zk, const ZKFileDBModel::Path& path) {
     std::vector<std::string> children = zk.getChildren(path);
     std::sort(children.begin(), children.end());
     return children;
@@ -60,7 +60,7 @@ ZKFileDBModel::addFile(const std::string& fileReference, const Buffer& buffer) {
     return _zk->setData(createPath(fileReference), buffer);
 }
 
-filedistribution::Move<filedistribution::Buffer>
+Move<Buffer>
 ZKFileDBModel::getFile(const std::string& fileReference) {
     try {
         return _zk->getData(createPath(fileReference));
@@ -295,4 +295,24 @@ ZKFileDBModel::getProgress(const std::string& fileReference,
     return progress;
 }
 
-filedistribution::FileDBModel::~FileDBModel() {}
+FileDBModel::~FileDBModel() {}
+
+DirectoryGuard::DirectoryGuard(boost::filesystem::path path) :
+    _fd(-1)
+{
+    _fd = open(path, O_RDONLY);
+    assert(_fd != -1);
+    int retval = flock(_fd, LOCK_EX);
+    assert(retval == 0);
+}
+
+DirectoryGuard::~DirectoryGuard() {
+    if (_fd != -1) {
+        int retval = flock(_fd, LOCK_UN);
+        assert(retval == 0);
+        retval = close(_fd);
+        assert(retval ==0);
+    }
+}
+
+}