Skip to content

Commit

Permalink
Modified the SimpleStorageService to reject file overwrites (safer for
Browse files Browse the repository at this point in the history
now)

Modified relevant test cases
  • Loading branch information
henricasanova committed Sep 5, 2017
1 parent 1863d57 commit 48ec07e
Show file tree
Hide file tree
Showing 8 changed files with 692 additions and 156 deletions.
4 changes: 2 additions & 2 deletions TODO.txt
Expand Up @@ -53,11 +53,11 @@ SOFTWARE ENGINEERING / TESTING:
- Add properties to the MultiCore compute service that corresponds to the
undelying properties of the StandardJobExecutors it will use

- Test for killing a standard job executor
- Test for killing a standard job executor (wait on S4U::Actor::kill() to be fixed)

- Add new tests for SimpleStorage Services
- What if one does a copy from src to src (i.e., same storage service)
- What happerns if I copy a file to a storage service that already has it
[DONE] What happerns if I copy a file to a storage service that already has it

- How come when sunmitting tasks with #cores=2 we don't get an exception on
a MulticoreComputeService (still relevant?)....
Expand Down
21 changes: 21 additions & 0 deletions include/wrench/workflow/FailureCause.h
Expand Up @@ -40,6 +40,8 @@ namespace wrench {
NO_STORAGE_SERVICE_FOR_FILE,
/** @brief The file was not found where it was supposed to be found */
FILE_NOT_FOUND,
/** @brief The file to be written was already there */
FILE_ALREADY_THERE,
/** @brief The storage service does not have enough space to support operation */
STORAGE_NO_ENOUGH_SPACE,
/** @brief The service cannot be used because it is down (likely was terminated) */
Expand Down Expand Up @@ -120,6 +122,25 @@ namespace wrench {
StorageService *storage_service;
};

/**
* @brief A "file already there" failure cause
*/
class StorageServiceFileAlreadyThere : public FailureCause {

public:
StorageServiceFileAlreadyThere(WorkflowFile *file, StorageService *storage_service);

WorkflowFile *getFile();
StorageService *getStorageService();
std::string toString();


private:
WorkflowFile *file;
StorageService *storage_service;
};


/**
* @brief A "service is down" failure cause
*/
Expand Down
1 change: 1 addition & 0 deletions src/wrench/managers/DataMovementManager.cpp
Expand Up @@ -143,6 +143,7 @@ namespace wrench {

std::unique_ptr<SimulationMessage> message = nullptr;

WRENCH_INFO("GOT MESSAGE");
try {
message = S4U_Mailbox::getMessage(this->mailbox_name);
} catch (std::shared_ptr<NetworkError> cause) {
Expand Down
1 change: 0 additions & 1 deletion src/wrench/services/storage_services/StorageService.cpp
Expand Up @@ -310,7 +310,6 @@ namespace wrench {
}

// Otherwise, synchronously send the file up!
WRENCH_INFO("SENDING THE FILE UP");
try {
S4U_Mailbox::putMessage(msg->data_write_mailbox_name, new StorageServiceFileContentMessage(file));
} catch (FailureCause *cause) {
Expand Down
Expand Up @@ -249,6 +249,26 @@ namespace wrench {
*/
bool SimpleStorageService::processFileWriteRequest(WorkflowFile *file, std::string answer_mailbox) {

// If the file is already there, send back a failure
if (this->stored_files.find(file) != this->stored_files.end()) {
try {
S4U_Mailbox::putMessage(answer_mailbox,
new StorageServiceFileWriteAnswerMessage(file,
this,
false,
std::shared_ptr<FailureCause>(
new StorageServiceFileAlreadyThere(
file,
this)),
"",
this->getPropertyValueAsDouble(
SimpleStorageServiceProperty::FILE_WRITE_ANSWER_MESSAGE_PAYLOAD)));
} catch (std::shared_ptr<NetworkError> cause) {
return true;
}
return true;
}

// Check the file size and capacity, and reply "no" if not enough space
if (file->getSize() > (this->capacity - this->occupied_space)) {
try {
Expand Down Expand Up @@ -361,6 +381,26 @@ namespace wrench {
bool
SimpleStorageService::processFileCopyRequest(WorkflowFile *file, StorageService *src, std::string answer_mailbox) {

// If the file is already there, send back a failure
if (this->stored_files.find(file) != this->stored_files.end()) {
WRENCH_INFO("Cannot perform file copy because file is already there");
try {
S4U_Mailbox::putMessage(answer_mailbox,
new StorageServiceFileCopyAnswerMessage(file,
this,
false,
std::shared_ptr<FailureCause>(
new StorageServiceFileAlreadyThere(
file,
this)),
this->getPropertyValueAsDouble(
SimpleStorageServiceProperty::FILE_COPY_ANSWER_MESSAGE_PAYLOAD)));
} catch (std::shared_ptr<NetworkError> cause) {
return true;
}
return true;
}

// Figure out whether this succeeds or not
if (file->getSize() > this->capacity - this->occupied_space) {
WRENCH_INFO("Cannot perform file copy due to lack of space");
Expand Down
36 changes: 36 additions & 0 deletions src/wrench/workflow_execution_events/FailureCause.cpp
Expand Up @@ -370,5 +370,41 @@ namespace wrench {
return "Job cannot be forgotten (because it's not completed or failed)";
};

/**
* @brief Constructor
* @param file: the file that is already there
* @param storage_service: the storage service on which it is
*/
StorageServiceFileAlreadyThere::StorageServiceFileAlreadyThere(WorkflowFile *file, StorageService *storage_service)
: FailureCause(FILE_ALREADY_THERE) {
this->file = file;
this->storage_service = storage_service;
}

/**
* @brief Getter
* @return the file
*/
WorkflowFile *StorageServiceFileAlreadyThere::getFile() {
return this->file;
}

/**
* @brief Getter
* @return the storage service
*/
StorageService *StorageServiceFileAlreadyThere::getStorageService() {
return this->storage_service;
}

/**
* @brief Get the human-readable failure message
* @return the message
*/
std::string StorageServiceFileAlreadyThere::toString() {
return "Cannot write file " + this->file->getId() + " to Storage Service " +
this->storage_service->getName() + " because it's already stored there";
}


};
76 changes: 73 additions & 3 deletions test/simulation/SimpleStorageServiceFunctionalTest.cpp
Expand Up @@ -414,14 +414,39 @@ class SimpleStorageServiceSynchronousFileCopyTestWMS : public wrench::WMS {
data_movement_manager->doSynchronousFileCopy(this->test->file_500, this->test->storage_service_1000,
this->test->storage_service_500);
} catch (wrench::WorkflowExecutionException &e) {
throw std::runtime_error("Got an exception while trying to initiate a file copy: " + std::string(e.what()));
throw std::runtime_error("Got an exception while doing a synchronous file copy: " + std::string(e.what()));
}

// Do the file copy again, which should fail
bool success = true;
try {
data_movement_manager->doSynchronousFileCopy(this->test->file_500, this->test->storage_service_1000,
this->test->storage_service_500);
} catch (wrench::WorkflowExecutionException &e) {
success = false;
if (e.getCause()->getCauseType() != wrench::FailureCause::FILE_ALREADY_THERE) {
throw std::runtime_error("Got an exception, as expected, but of the unexpected type " +
std::to_string(e.getCause()->getCauseType()));
}
// Check Exception details
wrench::StorageServiceFileAlreadyThere *real_cause = (wrench::StorageServiceFileAlreadyThere *) e.getCause().get();
if (real_cause->getFile() != this->test->file_500) {
throw std::runtime_error(
"Got the expected 'file already there' exception, but the failure cause does not point to the correct file");
}
if (real_cause->getStorageService() != this->test->storage_service_500) {
throw std::runtime_error(
"Got the expected 'file already there' exception, but the failure cause does not point to the correct storage service");
}
}
if (success) {
throw std::runtime_error("Should no be able fo write a file that's already there");
}

this->simulation->shutdownAllStorageServices();
this->simulation->shutdownAllComputeServices();
this->simulation->getFileRegistryService()->stop();


return 0;
}
};
Expand Down Expand Up @@ -535,6 +560,45 @@ class SimpleStorageServiceAsynchronousFileCopyTestWMS : public wrench::WMS {
}
}

// Do it all again, which should fail
// Initiate a file copy
try {
data_movement_manager->initiateAsynchronousFileCopy(this->test->file_500, this->test->storage_service_1000,
this->test->storage_service_500);
} catch (wrench::WorkflowExecutionException &e) {
throw std::runtime_error("Got an exception while trying to initiate a file copy: " + std::string(e.what()));
}

try {
event = workflow->waitForNextExecutionEvent();
} catch (wrench::WorkflowExecutionException &e) {
throw std::runtime_error("Error while getting an execution event: " + e.getCause()->toString());
}


switch (event->type) {
case wrench::WorkflowExecutionEvent::FILE_COPY_FAILURE: {
if (event->failure_cause->getCauseType() != wrench::FailureCause::FILE_ALREADY_THERE) {
throw std::runtime_error("Got an exception, as expected, but of the unexpected type " +
std::to_string(event->failure_cause->getCauseType()));
}
wrench::StorageServiceFileAlreadyThere *real_cause = (wrench::StorageServiceFileAlreadyThere *) event->failure_cause.get();
if (real_cause->getFile() != this->test->file_500) {
throw std::runtime_error(
"Got the expected 'file already there' exception, but the failure cause does not point to the correct file");
}
if (real_cause->getStorageService() != this->test->storage_service_500) {
throw std::runtime_error(
"Got the expected 'file already there' exception, but the failure cause does not point to the correct storage service");
}

break;
}
default: {
throw std::runtime_error("Unexpected workflow execution event: " + std::to_string((int) (event->type)));
}
}


this->simulation->shutdownAllStorageServices();
this->simulation->shutdownAllComputeServices();
Expand Down Expand Up @@ -655,7 +719,12 @@ class SimpleStorageServiceSynchronousFileCopyFailuresTestWMS : public wrench::WM
throw std::runtime_error("Should have gotten a 'not enough space' exception");
}


// Do the file copy for a file that's not there

// First delete the file to avoid the (already there error)
this->test->storage_service_1000->deleteFile(this->test->file_500);

success = true;
try {
data_movement_manager->doSynchronousFileCopy(this->test->file_500, this->test->storage_service_500,
Expand All @@ -664,7 +733,7 @@ class SimpleStorageServiceSynchronousFileCopyFailuresTestWMS : public wrench::WM
success = false;
// Check Exception
if (e.getCause()->getCauseType() != wrench::FailureCause::FILE_NOT_FOUND) {
throw std::runtime_error("Got an exception, as expected, but of the unexpected type " +
throw std::runtime_error("XXX Got an exception, as expected, but of the unexpected type " +
std::to_string(e.getCause()->getCauseType()));
}
// Check Exception details
Expand All @@ -682,6 +751,7 @@ class SimpleStorageServiceSynchronousFileCopyFailuresTestWMS : public wrench::WM
throw std::runtime_error("Should have gotten a 'file not found' exception");
}


// Do the file copy from a dst storage service that's down
this->test->storage_service_1000->stop();

Expand Down

0 comments on commit 48ec07e

Please sign in to comment.