Skip to content

Commit

Permalink
Code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
henricasanova committed Oct 11, 2024
1 parent 80c9922 commit 2f33e63
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 263 deletions.
10 changes: 0 additions & 10 deletions include/wrench/services/storage/StorageService.h
Original file line number Diff line number Diff line change
Expand Up @@ -550,14 +550,6 @@ namespace wrench {
// do nothing
}

/**
* @brief Increment the number of operations for a location
* @param location: a location
**/
virtual void incrementNumRunningOperationsForLocation(const std::shared_ptr<FileLocation> &location) {
// do nothing
}

/***********************/
/** \endcond **/
/***********************/
Expand All @@ -574,8 +566,6 @@ namespace wrench {

virtual void setIsScratch(bool is_scratch);

std::unordered_map<std::string, std::shared_ptr<simgrid::fsmod::File>> reserved_space;

/** Fast-Access common message payloads! **/
// double StorageServiceMessagePayload_FILE_READ_REQUEST_MESSAGE_PAYLOAD;
// double StorageServiceMessagePayload_FILE_READ_ANSWER_MESSAGE_PAYLOAD;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ namespace wrench {
}
};

int getNumRunningTransactionsOnDisk(simgrid::s4u::Disk *disk);
// int getNumRunningTransactionsOnDisk(simgrid::s4u::Disk *disk);

/***********************/
/** \endcond **/
Expand Down
20 changes: 5 additions & 15 deletions src/wrench/services/storage/proxy/StorageServiceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ namespace wrench {
}


/**
* @brief Determine whether the proxy has a file
*
* @param location: a file location
*/
bool StorageServiceProxy::hasFile(const std::shared_ptr<FileLocation> &location) {
if (cache) {
return cache->hasFile(location);
Expand Down Expand Up @@ -283,21 +288,6 @@ namespace wrench {
throw runtime_error("Proxy with no default location does not support getTotalSpace()");
}


// /**
// * @brief Get the mount point of the remote server (will throw is more than one). If there isnt a default, returns DEV_NUL
// * @return the (sole) mount point of the service
// */
// std::string StorageServiceProxy::getMountPoint() {
// if (remote) {
// return remote->getMountPoint();
// }
// if (cache) {
// return cache->getMountPoint();
// }
// return LogicalFileSystem::DEV_NULL;
// }

/**
* @brief Returns true if the cache is bufferized, false otherwise
* @return true or false
Expand Down
53 changes: 2 additions & 51 deletions src/wrench/services/storage/simple/SimpleStorageService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ namespace wrench {
throw std::invalid_argument("SimpleStorageService::SimpleStorageService(): A storage service must have at least one mount point");
}

// TODO: Can we pass infinity as the second parameter?
this->file_system = sgfs::FileSystem::create(this->getName() + "_fs", 1024*1024*1024);
this->file_system = sgfs::FileSystem::create(this->getName() + "_fs", INT_MAX);
for (const auto &mp: mount_points) {
// Find the disk
auto disk = S4U_Simulation::hostHasMountPoint(this->hostname, mp);
Expand Down Expand Up @@ -416,52 +415,6 @@ namespace wrench {
}
}


// /**
// * @brief Helper method to split a path into mountpoint:path_at_mount_point
// * @param path: a path string
// * @param mount_point: the mountpoint
// * @param path_at_mount_point: the path at the mount point
// * @return true on success, false on failure (i.e., mount point not found)
// */
// bool SimpleStorageService::splitPath(const std::string &path, std::string &mount_point, std::string &path_at_mount_point) {
// auto sanitized_path = FileLocation::sanitizePath(path);
// for (auto const &fs: this->file_systems) {
// auto mp = fs.first;
// if (FileLocation::properPathPrefix(mp, sanitized_path)) {
// mount_point = mp;
// path_at_mount_point = sanitized_path.erase(0, mp.length());
// return true;
// }
// }
// return false;
// }

// /**
// * @brief Decrement the number of operations for a location
// * @param location: a location
// */
// void SimpleStorageService::decrementNumRunningOperationsForLocation(const std::shared_ptr<FileLocation> &location) {
// std::string mount_point;
// std::string path_at_mount_point;
//
// this->splitPath(location->getPath(), mount_point, path_at_mount_point);
// this->file_systems[mount_point]->decrementNumRunningTransactionsForFileInDirectory(location->getFile(), path_at_mount_point);
// }
//
// /**
// * @brief increment the number of operations for a location
// * @param location: a location
// */
// void SimpleStorageService::incrementNumRunningOperationsForLocation(const std::shared_ptr<FileLocation> &location) {
// std::string mount_point;
// std::string path_at_mount_point;
//
// this->splitPath(location->getPath(), mount_point, path_at_mount_point);
// this->file_systems[mount_point]->incrementNumRunningTransactionsForFileInDirectory(location->getFile(), path_at_mount_point);
// }


/**
* @brief Create a file at the storage service (in zero simulated time)
* @param location: a location
Expand Down Expand Up @@ -537,7 +490,7 @@ namespace wrench {
/**
* @brief Helper method to validate a file write request
* @param location: the location to read
* @param file_already_there: indicates (on output) whether the file is already there or not
* @param num_bytes_to_write: number of bytes to write
* @param opened_file: an opened file (if success)
* @return a FailureCause or nullptr if success
*/
Expand Down Expand Up @@ -616,13 +569,11 @@ namespace wrench {
try {
bool dst_file_already_there = dst_file_system->file_exists(dst_location->getFilePath());
if (not dst_file_already_there) { // Open dot file
// std::cerr << "FILE NOT ALREADY THERE, OPENING A DOT FILE \n";
std::string dot_file_path = dst_location->getADotFilePath();
dst_file_system->create_file(dot_file_path, dst_location->getFile()->getSize());
dst_opened_file = dst_file_system->open(dot_file_path, "r+");
dst_opened_file->seek(0, SEEK_SET);
} else { // Open the file
// std::cerr << "FILE ALREADY THERE, JUST OPENING IT\n";
dst_opened_file = dst_file_system->open(dst_location->getFilePath(), "r+");
dst_opened_file->seek(0, SEEK_SET);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,6 @@ namespace wrench {
return true;
}

// // Create directory if need be
// if (not this->file_system->directory_exists(location->getDirectoryPath())) {
// this->file_system->create_directory(location->getDirectoryPath());
// }

// Generate a commport name on which to receive the file
auto file_reception_commport = S4U_CommPort::getTemporaryCommPort();

Expand Down Expand Up @@ -380,8 +375,6 @@ namespace wrench {
ftt->setSimulation(this->simulation);
this->pending_file_transfer_threads.push_back(ftt);

// src_location->getStorageService()->incrementNumRunningOperationsForLocation(src_location);

return true;
}

Expand Down Expand Up @@ -508,15 +501,10 @@ namespace wrench {
// WRENCH_INFO("Sending back an ack for a successful file read");
ftt->answer_commport_if_read->dputMessage(new StorageServiceAckMessage(ftt->src_location));
} else if (ftt->src_location == nullptr) {
// StorageService::createFileAtLocation(ftt->dst_location);
WRENCH_INFO("File %s stored", ftt->dst_location->getFile()->getID().c_str());
// Deal with time stamps, previously we could test whether a real timestamp was passed, now this.
// Maybe no corresponding timestamp.
// WRENCH_INFO("Sending back an ack for a successful file read");
ftt->answer_commport_if_write->dputMessage(new StorageServiceAckMessage(ftt->dst_location));
} else {
if (ftt->dst_location->getStorageService() == shared_from_this()) {
// this->createFile(ftt->dst_location);
WRENCH_INFO("File %s stored", ftt->dst_location->getFile()->getID().c_str());
try {
this->simulation->getOutput().addTimestampFileCopyCompletion(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,130 +532,6 @@ namespace wrench {
return true;
}


// /**
// * @brief Handle a file copy request
// * @param src_location: the source location
// * @param dst_location: the destination location
// * @param answer_commport: the commport to which the answer should be sent
// * @return
// */
// bool SimpleStorageServiceNonBufferized::processFileCopyRequestIAmTheSource(
// std::shared_ptr<FileLocation> &src_location,
// std::shared_ptr<FileLocation> &dst_location,
// S4U_CommPort *answer_commport) {
//
// WRENCH_INFO("FileCopyRequest: %s -> %s",
// src_location->toString().c_str(),
// dst_location->toString().c_str());
//
// // TODO: This code is duplicated with the IAmNotTheSource version of this method
// auto src_host = src_location->getStorageService()->getHost();
// auto dst_host = dst_location->getStorageService()->getHost();
//
// auto src_disk = src_location->getDiskOrNull();
// if (src_disk == nullptr) {
// throw std::runtime_error("SimpleStorageServiceNonBufferized::processFileCopyRequestIAmTheSource(): source disk not found - internal error");
// }
// auto dst_disk = dst_location->getDiskOrNull();
// if (dst_disk == nullptr) {
// throw std::runtime_error("SimpleStorageServiceNonBufferized::processFileCopyRequestIAmTheSource(): destination disk not found - internal error");
// }
//
// auto file = src_location->getFile();
//
// std::string src_mount_point;
// std::string src_path_at_mount_point;
//
// if (not this->file_system->directory_exists(src_location->getPath())) {
// try {
// answer_commport->putMessage(
// new StorageServiceFileCopyAnswerMessage(
// src_location,
// dst_location,
// false,
// std::make_shared<InvalidDirectoryPath>(src_location),
// this->getMessagePayloadValue(
// SimpleStorageServiceMessagePayload::FILE_COPY_ANSWER_MESSAGE_PAYLOAD)));
//
// } catch (ExecutionException &e) {
// return true;
// }
// return true;
// }
//
// // Do I have the file
// if (not this->file_system->file_exists(src_location->getPath() + "/" + src_location->getFile()->getID())) {
// try {
// answer_commport->putMessage(
// new StorageServiceFileCopyAnswerMessage(
// src_location,
// dst_location,
// false,
// std::shared_ptr<FailureCause>(
// new FileNotFound(
// src_location)),
// this->getMessagePayloadValue(
// SimpleStorageServiceMessagePayload::FILE_COPY_ANSWER_MESSAGE_PAYLOAD)));
//
// } catch (ExecutionException &e) {
// return true;
// }
// return true;
// }
//
// // At this point, I have the file
//
// // Can file fit at the destination?
// // auto dst_file_system = dst_location->getStorageService()->file_systems[dst_location->getMountPoint()].get();
// // bool file_already_at_destination = dst_file_system->isFileInDirectory(dst_location->getFile(), dst_location->getAbsolutePathAtMountPoint());
// bool file_already_at_destination = StorageService::hasFileAtLocation(dst_location);
//
// // If not already at destination make space for it, and if not possible, then return an error
// if (not file_already_at_destination) {
// if (not dst_location->getStorageService()->reserveSpace(dst_location)) {
// try {
// answer_commport->putMessage(
// new StorageServiceFileCopyAnswerMessage(
// src_location,
// dst_location,
// false,
// std::shared_ptr<FailureCause>(
// new StorageServiceNotEnoughSpace(dst_location->getFile(),
// dst_location->getStorageService())),
// this->getMessagePayloadValue(
// SimpleStorageServiceMessagePayload::FILE_COPY_ANSWER_MESSAGE_PAYLOAD)));
//
// } catch (ExecutionException &e) {
// return true;
// }
// return true;
// }
// }
//
// // At this point we're all good
// uint64_t transfer_size;
// transfer_size = (uint64_t) (file->getSize());
//
// src_location->getStorageService()->incrementNumRunningOperationsForLocation(src_location);
//
// // Create a Transaction
// auto transaction = std::make_shared<Transaction>(
// src_location,
// src_host,
// src_disk,
// dst_location,
// dst_host,
// dst_disk,
// answer_commport,
// transfer_size);
//
// // Add it to the Pool of pending data communications
// this->pending_transactions.push_back(transaction);
//
// return true;
// }

/**
* @brief Start pending file transfer threads if any and if possible
*/
Expand All @@ -668,7 +544,7 @@ namespace wrench {
auto transaction = this->pending_transactions.front();
this->pending_transactions.pop_front();

// Sadly we cannot do this with simgrid::fsmod...
// TODO: Sadly we cannot do this with simgrid::fsmod...
auto sg_iostream = simgrid::s4u::Io::streamto_init(transaction->src_host,
transaction->src_disk,
transaction->dst_host,
Expand All @@ -692,52 +568,4 @@ namespace wrench {
}


// /**
// * @brief Process a file copy request
// * @param src: the source location
// * @param dst: the dst location
// * @param answer_commport: the answer commport
// * @return true is the service should continue;
// */
// bool SimpleStorageServiceNonBufferized::processFileCopyRequest(std::shared_ptr<FileLocation> &src,
// std::shared_ptr<FileLocation> &dst,
// S4U_CommPort *answer_commport) {
//
// // Check that src has the file
// if (not StorageService::hasFileAtLocation(src)) {
// answer_commport->dputMessage(
// new StorageServiceFileCopyAnswerMessage(
// src,
// dst,
// false,
// std::make_shared<FileNotFound>(src),
// this->getMessagePayloadValue(
// SimpleStorageServiceMessagePayload::FILE_COPY_ANSWER_MESSAGE_PAYLOAD)));
//
// return true;
// }
//
// if (src->getStorageService() != this->getSharedPtr<StorageService>()) {
// return processFileCopyRequestIAmNotTheSource(src, dst, answer_commport);
// } else {
// return processFileCopyRequestIAmTheSource(src, dst, answer_commport);
// }
// }

// /**
// * @brief Returns the number of running transactions on a disk, as far as this
// * storage service know
// * @param disk: a disk
// * @return a number of transactions
// */
// int SimpleStorageServiceNonBufferized::getNumRunningTransactionsOnDisk(simgrid::s4u::Disk *disk) {
// int count = 0;
// for (const auto &t: this->running_transactions) {
// if ((t->src_disk == disk) or (t->dst_disk == disk)) {
// count++;
// }
// }
// return count;
// }

}// namespace wrench
1 change: 1 addition & 0 deletions src/wrench/services/storage/xrootd/Cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "wrench/simgrid_S4U_util/S4U_Simulation.h"
namespace wrench {
namespace XRootD {

/**
* @brief Check the cache for a file
* @param file: The file to check the cache for
Expand Down
1 change: 0 additions & 1 deletion src/wrench/simulation/Simulation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,6 @@ namespace wrench {
*
* @param num_bytes: number of bytes read
* @param hostname: hostname to read from
* @param mount_point: mount point of disk to read from
* @param disk: disk to read from (nullptr if not known)
*
*/
Expand Down

0 comments on commit 2f33e63

Please sign in to comment.