Commit ffdc0cb0 authored by Eric Cano's avatar Eric Cano
Browse files

Added actual locking to the getMountInfo call.

parent 816920a7
......@@ -29,7 +29,10 @@
#include <unistd.h>
#include <sys/socket.h>
#include <sys/stat.h>
#undef DEBUG_PRINT_LOGS
#ifdef DEBUG_PRINT_LOGS
#include <iostream>
#endif
namespace cta { namespace objectstore {
......@@ -179,14 +182,16 @@ void BackendVFS::ScopedLock::release() {
::flock(m_fd, LOCK_UN);
::close(m_fd);
m_fdSet = false;
//std::cout << "Unlocked " << name << " with fd=" << fd << " @" << where << " tid=" << syscall(SYS_gettid) << std::endl;
#ifdef DEBUG_PRINT_LOGS
std::cout << "Unlocked " << m_path << " with fd=" << m_fd << " tid=" << syscall(SYS_gettid) << std::endl;
#endif
}
BackendVFS::ScopedLock * BackendVFS::lockHelper(
std::string name, int type) {
std::string path = m_root + "/." + name + ".lock";
std::unique_ptr<ScopedLock> ret(new ScopedLock);
ret->set(::open(path.c_str(), O_RDONLY));
ret->set(::open(path.c_str(), O_RDONLY), path);
cta::exception::Errnum::throwOnMinusOne(ret->m_fd,
"In BackendStoreVFS::lockHelper, failed to open the lock file.");
cta::exception::Errnum::throwOnMinusOne(::flock(ret->m_fd, LOCK_EX),
......@@ -195,13 +200,21 @@ BackendVFS::ScopedLock * BackendVFS::lockHelper(
}
BackendVFS::ScopedLock * BackendVFS::lockExclusive(std::string name) {
return lockHelper(name, LOCK_EX);
//std::cout << "LockedExclusive " << name << " with fd=" << context.get(0) << " @" << where << " tid=" << syscall(SYS_gettid) << std::endl;
std::unique_ptr<BackendVFS::ScopedLock> ret(lockHelper(name, LOCK_EX));
#ifdef DEBUG_PRINT_LOGS
std::cout << "LockedExclusive " << name << " with fd=" << ret->m_fd
<< " path=" << ret->m_path << " tid=" << syscall(SYS_gettid) << std::endl;
#endif
return ret.release();
}
BackendVFS::ScopedLock * BackendVFS::lockShared(std::string name) {
return lockHelper(name, LOCK_SH);
//std::cout << "LockedShared " << name << " with fd=" << context.get(0) << " @" << where << " tid=" << syscall(SYS_gettid) << std::endl;
std::unique_ptr<BackendVFS::ScopedLock> ret(lockHelper(name, LOCK_SH));
#ifdef DEBUG_PRINT_LOGS
std::cout << "LockedShared " << name << " with fd=" << ret->m_fd
<< " path=" << ret->m_path << " tid=" << syscall(SYS_gettid) << std::endl;
#endif
return ret.release();
}
std::string BackendVFS::Parameters::toStr() {
......
......@@ -76,8 +76,9 @@ public:
virtual ~ScopedLock() { release(); }
private:
ScopedLock(): m_fdSet(false) {}
void set(int fd) { m_fd=fd; m_fdSet=true; }
void set(int fd, const std::string & path) { m_fd=fd; m_fdSet=true; m_path=path; }
bool m_fdSet;
std::string m_path;
int m_fd;
};
......
......@@ -73,6 +73,11 @@ std::unique_ptr<SchedulerDatabase::TapeMountDecisionInfo>
objectstore::RootEntry re(m_objectStore);
objectstore::ScopedSharedLock rel(re);
re.fetch();
// Take an exclusive lock on the scheduling
tmdi.m_schedulerGlobalLock.reset(
new SchedulerGlobalLock(re.getSchedulerGlobalLock(), m_objectStore));
tmdi.m_lockOnSchedulerGlobalLock.lock(*tmdi.m_schedulerGlobalLock);
tmdi.m_lockTaken = true;
auto tpl = re.dumpTapePools();
for (auto tpp=tpl.begin(); tpp!=tpl.end(); tpp++) {
// Get the tape pool object
......@@ -1048,7 +1053,12 @@ std::unique_ptr<SchedulerDatabase::RetrieveMount>
}
OStoreDB::TapeMountDecisionInfo::~TapeMountDecisionInfo() {
// TODO: manage the locking of the scheduling
// The lock should be released before the objectstore object
// m_schedulerGlobalLock gets destroyed. We explicitely release the lock,
// and then destroy the object
if (m_lockTaken)
m_lockOnSchedulerGlobalLock.release();
m_schedulerGlobalLock.reset(NULL);
}
......
......@@ -21,6 +21,7 @@
#include "scheduler/SchedulerDatabase.hpp"
#include "objectstore/Agent.hpp"
#include "objectstore/ArchiveToFileRequest.hpp"
#include "objectstore/SchedulerGlobalLock.hpp"
namespace cta {
......@@ -44,12 +45,17 @@ public:
CTA_GENERATE_EXCEPTION_CLASS(NotImplemented);
/* === Session handling =================================================== */
class TapeMountDecisionInfo: public SchedulerDatabase::TapeMountDecisionInfo {
friend class OStoreDB;
public:
virtual std::unique_ptr<SchedulerDatabase::ArchiveMount> createArchiveMount(const std::string & vid,
const std::string driveName);
virtual std::unique_ptr<SchedulerDatabase::RetrieveMount> createRetrieveMount(const std::string & vid,
const std::string driveName);
virtual ~TapeMountDecisionInfo();
private:
bool m_lockTaken;
objectstore::ScopedExclusiveLock m_lockOnSchedulerGlobalLock;
std::unique_ptr<objectstore::SchedulerGlobalLock> m_schedulerGlobalLock;
};
virtual std::unique_ptr<SchedulerDatabase::TapeMountDecisionInfo> getMountInfo();
......
......@@ -63,6 +63,7 @@ public:
m_agent.insertAndRegisterSelf();
rel.lock(re);
re.addOrGetDriveRegisterPointerAndCommit(m_agent, cl);
re.addOrGetSchedulerGlobalLockAndCommit(m_agent, cl);
rel.release();
m_OStoreDB.setAgent(m_agent);
}
......
......@@ -437,6 +437,10 @@ TEST_P(SchedulerDatabaseTest, getMountInfo) {
10, cl);
std::unique_ptr<cta::SchedulerDatabase::ArchiveToFileRequestCreation> creation(db.queue(atfr));
creation->complete();
// Make sure we clear and reset mountCandidate before evaluating getMountInfo
// again (which would block otherwise), as it gets called before the operator=,
// which would implicitly destroy the TapeMountDecision
mountCandidates.reset(NULL);
ASSERT_NO_THROW(mountCandidates = db.getMountInfo());
{
cta::SchedulerDatabase::TapeMountDecisionInfo & tmdi = *mountCandidates;
......@@ -450,6 +454,7 @@ TEST_P(SchedulerDatabaseTest, getMountInfo) {
// Add one more job to the queue: the summary should change accordingly
std::unique_ptr<cta::SchedulerDatabase::ArchiveToFileRequestCreation> creation2(db.queue(atfr));
creation2->complete();
mountCandidates.reset(NULL);
ASSERT_NO_THROW(mountCandidates = db.getMountInfo());
{
cta::SchedulerDatabase::TapeMountDecisionInfo & tmdi = *mountCandidates;
......@@ -488,6 +493,7 @@ TEST_P(SchedulerDatabaseTest, getMountInfo) {
tcl.back().vid = "Tape3";
tcl.back().copyNumber = 2;
ASSERT_NO_THROW(db.queue(cta::RetrieveToFileRequest("cta:://cta/myfile", 1234, tcl, "eos://myeos/myeosfile", 10, cl)));
mountCandidates.reset(NULL);
ASSERT_NO_THROW(mountCandidates = db.getMountInfo());
{
cta::SchedulerDatabase::TapeMountDecisionInfo & tmdi = *mountCandidates;
......@@ -535,6 +541,7 @@ TEST_P(SchedulerDatabaseTest, getMountInfo) {
tcl2.back().vid = "Tape2";
tcl2.back().copyNumber = 2;
db.queue(cta::RetrieveToFileRequest("cta:://cta/myfile2", 1234, tcl2, "eos://myeos/myeosfile2", 10, cl));
mountCandidates.reset(NULL);
ASSERT_NO_THROW(mountCandidates = db.getMountInfo());
{
cta::SchedulerDatabase::TapeMountDecisionInfo & tmdi = *mountCandidates;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment