From 2f33e6338e3897c9f51ece844e39be8b5ec355cb Mon Sep 17 00:00:00 2001 From: Henri Casanova Date: Thu, 10 Oct 2024 16:57:32 -1000 Subject: [PATCH] Code cleanup --- .../wrench/services/storage/StorageService.h | 10 - .../SimpleStorageServiceNonBufferized.h | 2 +- .../storage/proxy/StorageServiceProxy.cpp | 20 +- .../storage/simple/SimpleStorageService.cpp | 53 +----- .../simple/SimpleStorageServiceBufferized.cpp | 12 -- .../SimpleStorageServiceNonBufferized.cpp | 174 +----------------- src/wrench/services/storage/xrootd/Cache.cpp | 1 + src/wrench/simulation/Simulation.cpp | 1 - 8 files changed, 10 insertions(+), 263 deletions(-) diff --git a/include/wrench/services/storage/StorageService.h b/include/wrench/services/storage/StorageService.h index 6c41c5d1ac..0ab4be44f7 100755 --- a/include/wrench/services/storage/StorageService.h +++ b/include/wrench/services/storage/StorageService.h @@ -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 &location) { - // do nothing - } - /***********************/ /** \endcond **/ /***********************/ @@ -574,8 +566,6 @@ namespace wrench { virtual void setIsScratch(bool is_scratch); - std::unordered_map> reserved_space; - /** Fast-Access common message payloads! **/ // double StorageServiceMessagePayload_FILE_READ_REQUEST_MESSAGE_PAYLOAD; // double StorageServiceMessagePayload_FILE_READ_ANSWER_MESSAGE_PAYLOAD; diff --git a/include/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.h b/include/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.h index ec86341111..cbd28cd52f 100644 --- a/include/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.h +++ b/include/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.h @@ -97,7 +97,7 @@ namespace wrench { } }; - int getNumRunningTransactionsOnDisk(simgrid::s4u::Disk *disk); +// int getNumRunningTransactionsOnDisk(simgrid::s4u::Disk *disk); /***********************/ /** \endcond **/ diff --git a/src/wrench/services/storage/proxy/StorageServiceProxy.cpp b/src/wrench/services/storage/proxy/StorageServiceProxy.cpp index 8b7c0957c2..bc53cf6851 100644 --- a/src/wrench/services/storage/proxy/StorageServiceProxy.cpp +++ b/src/wrench/services/storage/proxy/StorageServiceProxy.cpp @@ -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 &location) { if (cache) { return cache->hasFile(location); @@ -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 diff --git a/src/wrench/services/storage/simple/SimpleStorageService.cpp b/src/wrench/services/storage/simple/SimpleStorageService.cpp index a8bc033183..eb42d404e1 100644 --- a/src/wrench/services/storage/simple/SimpleStorageService.cpp +++ b/src/wrench/services/storage/simple/SimpleStorageService.cpp @@ -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); @@ -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 &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 &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 @@ -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 */ @@ -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); } diff --git a/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp b/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp index 93fe92c488..dfaab76516 100644 --- a/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp +++ b/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp @@ -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(); @@ -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; } @@ -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( diff --git a/src/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.cpp b/src/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.cpp index f3f7b062d5..1202400d56 100644 --- a/src/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.cpp +++ b/src/wrench/services/storage/simple/SimpleStorageServiceNonBufferized.cpp @@ -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 &src_location, -// std::shared_ptr &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(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( -// 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( -// 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( -// 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 */ @@ -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, @@ -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 &src, -// std::shared_ptr &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(src), -// this->getMessagePayloadValue( -// SimpleStorageServiceMessagePayload::FILE_COPY_ANSWER_MESSAGE_PAYLOAD))); -// -// return true; -// } -// -// if (src->getStorageService() != this->getSharedPtr()) { -// 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 diff --git a/src/wrench/services/storage/xrootd/Cache.cpp b/src/wrench/services/storage/xrootd/Cache.cpp index 06b9069fe1..75bbb7769e 100755 --- a/src/wrench/services/storage/xrootd/Cache.cpp +++ b/src/wrench/services/storage/xrootd/Cache.cpp @@ -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 diff --git a/src/wrench/simulation/Simulation.cpp b/src/wrench/simulation/Simulation.cpp index 6c584d2205..ad44de038d 100644 --- a/src/wrench/simulation/Simulation.cpp +++ b/src/wrench/simulation/Simulation.cpp @@ -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) * */