Commit b1bd12b8 authored by Paal Kvamme's avatar Paal Kvamme
Browse files

User opt-in for the readonly kludge.

parent ad1a5222
......@@ -48,12 +48,14 @@
#include <SDUtils.h>
#include <Constants.h>
#include <HttpContext.h>
#include <SDException.h>
#else
#include <SDAPI/SDManager.h>
#include <SDAPI/SDGenericDataset.h>
#include <SDAPI/SDUtils.h>
#include <SDAPI/Constants.h>
#include <SDAPI/HttpContext.h>
#include <SDAPI/SDException.h>
#endif
// SDAPI footprint, might not be accurate.
......@@ -190,6 +192,17 @@ public:
bool _mylogger(int priority, const std::string& message) const;
bool _sslogger(int priority, const std::ios& ss) const;
void _set_backoff(seismicdrive::SDGenericDataset* sdgd);
std::shared_ptr<seismicdrive::SDGenericDataset>_open_dataset_ro(
const std::shared_ptr<seismicdrive::SDManager>& manager,
const std::string& filename,
const std::unordered_map<std::string, std::string>& extra,
bool sd_ds_log);
std::shared_ptr<seismicdrive::SDGenericDataset>_open_dataset_rw(
const std::shared_ptr<seismicdrive::SDManager>& manager,
const std::string& filename,
bool truncate,
const std::unordered_map<std::string, std::string>& extra,
bool sd_ds_log);
private:
/**
* This class is used by _split_by_segment to describe a request as seen by
......@@ -790,23 +803,19 @@ public:
* C++11 and later. There are workarounds with multiple try/catch that
* *might* help but it should be a lot easier to fix the underlying bug.
*/
void wrapper_close()
void wrapper_close(bool set_readonly)
{
auto victim = dataset_;
OpenMode disp = disposition_;
dataset_.reset();
disposition_ = OpenMode::Closed;
switch (disp) {
case OpenMode::Truncate:
case OpenMode::ReadWrite:
// TODO-Low: Maybe not if the file isn't finalized?
if (victim) {
if (set_readonly &&
(disp == OpenMode::Truncate || disp == OpenMode::ReadWrite))
victim->setReadonlyMode(true);
break;
default:
break;
victim->close();
victim.reset(); // Any throw from SDGenericDataset dtor happens here.
}
victim->close();
victim.reset(); // Any throw from SDGenericDataset dtor happens here.
}
void updateDataset(std::shared_ptr<seismicdrive::SDGenericDataset> dataset) {
......@@ -1044,7 +1053,7 @@ SDGenericDatasetWrapper::~SDGenericDatasetWrapper()
// Hopefully the destructors won't try to close again when they see
// that we tried to do so ourselves. Note: currently they will try that.
if (dataset_)
wrapper_close();
wrapper_close(false);
}
catch (const std::exception& ex) {
if (std::string(ex.what()).find("dataset is not open") == std::string::npos)
......@@ -1061,11 +1070,6 @@ FileUtilsSeismicStore::~FileUtilsSeismicStore()
{
}
extern "C" void BREAK()
{
std::cerr << "\nBROKE\n" << std::endl;
}
SeismicStoreFile::SeismicStoreFile(const std::string& filename, OpenMode mode, const IOContext *iocontext)
: FileUtilsSeismicStore()
, _config()
......@@ -1101,57 +1105,29 @@ SeismicStoreFile::SeismicStoreFile(const std::string& filename, OpenMode mode, c
auto manager = std::make_shared<seismicdrive::SDManager>
(context->_sdurl, context->_sdapikey);
manager->setLogStatus(sd_mgr_log);
std::shared_ptr<seismicdrive::SDGenericDataset> dataset;
if (mode != OpenMode::Closed) {
if (_logger(5, ""))
_sslogger(5, std::stringstream()
<< "make dataset using manager "
<< std::hex << (std::uint64_t)manager.get());
dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_logger(5, "dataset ok");
}
// TODO-Low: Cache the manager and possibly the SDUtils instance.
auto datasetwrapper = std::make_shared<SDGenericDatasetWrapper>
(manager, dataset, mode, _logger);
(manager, nullptr, mode, _logger);
datasetwrapper->authorizeManager
(context->_sdtoken, context->_sdtokentype,
context->_sdtokencb, context->_sdtoken_cbtype);
std::shared_ptr<seismicdrive::SDGenericDataset> dataset;
try {
if (mode != OpenMode::Closed)
_set_backoff(dataset.get());
switch (mode) {
case OpenMode::Closed:
break;
case OpenMode::ReadOnly:
dataset->open(seismicdrive::SDDatasetDisposition::READ_ONLY, extra);
dataset = _open_dataset_ro(manager, filename, extra, sd_ds_log);
break;
case OpenMode::ReadWrite:
{
// Assume the zgy file was marked as readonly.
// Change it back to writable. The application takes all
// responsibility for this not being a very bad idea.
// TODO-Low: Should the application explicitly request this?
auto sgds = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
sgds->open(seismicdrive::SDDatasetDisposition::READ_ONLY);
if (sgds->getReadonlyMode()) {
sgds->setReadonlyMode(false);
_logger(0, "Cleared readonly flag on \"" + filename + "\"");
}
else {
_logger(0, "No readonly flag on \"" + filename + "\"");
}
sgds->close();
}
dataset->open(seismicdrive::SDDatasetDisposition::READ_WRITE, extra);
dataset = _open_dataset_rw(manager, filename, false, extra, sd_ds_log);
break;
case OpenMode::Truncate:
dataset->open(seismicdrive::SDDatasetDisposition::OVERWRITE, extra);
dataset = _open_dataset_rw(manager, filename, true, extra, sd_ds_log);
break;
}
}
......@@ -1159,6 +1135,7 @@ SeismicStoreFile::SeismicStoreFile(const std::string& filename, OpenMode mode, c
datasetwrapper->throwCloudException(ex, "Open");
}
datasetwrapper->updateDataset(dataset);
this->_dataset = datasetwrapper;
// Removed this because it causes info() to be populated early,
// thereby negating the benefit of lazy evaluation. And, worse,
......@@ -1177,7 +1154,7 @@ SeismicStoreFile::~SeismicStoreFile()
// the application will crash.
if (_dataset && _dataset->dataset() && _dataset->disposition() != OpenMode::Closed) {
try {
_dataset->wrapper_close();
_dataset->wrapper_close(_config->_set_ro_after_write);
}
catch (const std::exception& ex) {
_logger(0, "EXCEPTION closing file: " + std::string(ex.what()));
......@@ -1254,6 +1231,113 @@ SeismicStoreFile::_set_backoff(seismicdrive::SDGenericDataset* sdgd)
}
}
/**
* Allocate and open a SDGenericDataset for read.
* If dictated by the iocontext, turn on the read-only flag first.
*/
std::shared_ptr<seismicdrive::SDGenericDataset>
SeismicStoreFile::_open_dataset_ro(const std::shared_ptr<seismicdrive::SDManager>& manager, const std::string& filename, const std::unordered_map<std::string, std::string>& extra, bool sd_ds_log)
{
if (_logger(5, ""))
_sslogger(5, std::stringstream()
<< "make dataset for reading using manager "
<< std::hex << (std::uint64_t)manager.get());
auto dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_set_backoff(dataset.get());
dataset->open(seismicdrive::SDDatasetDisposition::READ_ONLY, extra);
if (this->_config->_force_ro_before_read) {
if (!dataset->getReadonlyMode()) {
dataset->setReadonlyMode(true);
// For robustness, assume SDAPI needs a re-open to clear the read lock.
// For robustness, assume the dataset instance cannot be re-used.
dataset->close();
dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_set_backoff(dataset.get());
dataset->open(seismicdrive::SDDatasetDisposition::READ_ONLY, extra);
_logger(2, "Readonly flag forced on for \"" + filename + "\"");
}
else {
_logger(2, "Readonly flag already on for \"" + filename + "\"");
}
}
if (_logger(5, ""))
_sslogger(5, std::stringstream()
<< "dataset for reading is "
<< std::hex << (std::uint64_t)dataset.get());
return dataset;
}
/**
* Allocate and open a SDGenericDataset for write.*
*
* If the file is already open for write elsewhere then SDAPI will throw.
* If the file already has a read lock this implies that the read-only
* flag is already off, and the SDAPI will throw.
*
* If dictated by the iocontext, turn off the read-only flag first.
* This change will only happen if the file is currently unlocked.
* Otherwise the read-only flag has to be off already.
* This might still be a bad idea. The application assumes all responsibility.
*/
std::shared_ptr<seismicdrive::SDGenericDataset>
SeismicStoreFile::_open_dataset_rw(const std::shared_ptr<seismicdrive::SDManager>& manager, const std::string& filename, bool truncate, const std::unordered_map<std::string, std::string>& extra, bool sd_ds_log)
{
if (_logger(5, ""))
_sslogger(5, std::stringstream()
<< "make dataset for writing using manager "
<< std::hex << (std::uint64_t)manager.get());
const seismicdrive::SDDatasetDisposition disp =
truncate ?
seismicdrive::SDDatasetDisposition::OVERWRITE :
seismicdrive::SDDatasetDisposition::READ_WRITE;
auto dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_set_backoff(dataset.get());
if (!this->_config->_force_rw_before_write) {
dataset->open(disp, extra);
}
else {
try {
// Unlike the open for read case, incorrect read-only state will throw.
dataset->open(disp, extra);
_logger(2, "Readonly flag already off for \"" + filename + "\"");
}
catch (const seismicdrive::SDException& ex) {
// TODO-Low: A specific SDAPI exception "read-only dataset"
// Currently a SDExceptionSDAccessorError is thrown, which is
// more about *where* the error occured and not *what* went wrong.
// So the catch might as well be on SDException so that fixing
// SDAPI won't break this code. Or maybe just std::exception?
dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_set_backoff(dataset.get());
// This might throw if there is a current write lock.
dataset->open(seismicdrive::SDDatasetDisposition::READ_ONLY, extra);
if (dataset->getReadonlyMode()) {
dataset->setReadonlyMode(false);
dataset->close();
dataset = std::make_shared<seismicdrive::SDGenericDataset>
(manager.get(), filename, sd_ds_log);
_set_backoff(dataset.get());
// Any second throw will be passed on to the caller.
dataset->open(disp, extra);
_logger(2, "Readonly flag forced off for \"" + filename + "\"");
}
else {
_logger(2, "Readonly flag already on? for \"" + filename + "\"");
throw;
}
}
}
if (_logger(5, ""))
_sslogger(5, std::stringstream()
<< "dataset for writing is "
<< std::hex << (std::uint64_t)dataset.get());
return dataset;
}
std::shared_ptr<FileADT>
SeismicStoreFile::xx_make_instance(const std::string& filename, OpenMode mode, const IOContext *iocontext)
{
......@@ -1700,7 +1784,7 @@ SeismicStoreFile::xx_close()
<< "Ignoring exception during close: "
<< ex.what());
}
victim->wrapper_close();
victim->wrapper_close(this->_config->_set_ro_after_write);
}
break;
}
......@@ -1814,13 +1898,18 @@ SeismicStoreFile::altUrl(const std::string& filename) const
try {
// Make sure the returned smart pointer doesn't go out of scope.
std::shared_ptr<seismicdrive::SDManager> smart_manager = _dataset->manager();
seismicdrive::SDGenericDataset dataset(smart_manager.get(), filename);
dataset.open(seismicdrive::SDDatasetDisposition::READ_ONLY);
const std::string wid = dataset.getConsistencyID();
const std::string ctx = dataset.getSerializedContext();
std::shared_ptr<seismicdrive::SDGenericDataset> dataset =
// The ugly const-cast reflects that opening for read will in this
// case have a permanent effect on the file. There is a reason the
// flag starts with "force".
const_cast<SeismicStoreFile*>(this)->
_open_dataset_ro(smart_manager, filename,
std::unordered_map<std::string, std::string>(), false);
const std::string wid = dataset->getConsistencyID();
const std::string ctx = dataset->getSerializedContext();
const std::string url = filename.substr(0, filename.find("?")) + "?context=" + ctx;
try {
dataset.close();
dataset->close();
}
catch (const std::exception& ex) {
// Workaround for what is probably a bug in SDAPI.
......
......@@ -2785,6 +2785,164 @@ test_bat_sd_zfp()
do_testbat<float, 64, true>(cad.name());
}
static void
showex(const std::exception& ex)
{
//if (verbose())
// std::cout << "Caught " << typeid(ex).name() << ": " << ex.what() << std::endl;
}
static bool
ok_alturl(const std::string& filename)
{
std::shared_ptr<IZgyUtils> utils =
IZgyUtils::utils("sd://", Test_Utils::default_sd_context());
try {
std::string alturl = utils->alturl(filename);
if (alturl.size() < 3*filename.size()) {
if (verbose())
std::cout << "No alturl for \"" << filename << "\". Too short.\n";
return false;
}
}
catch (const std::exception& ex)
{
showex(ex);
if (verbose())
std::cout << "No alturl for \"" << filename << "\". " << ex.what()<< "\n";
return false;
}
if (verbose())
std::cout << "Good alturl for \"" << filename << "\".\n";
return true;
}
static bool
ok_writable(const std::string& filename)
{
try {
std::shared_ptr<OpenZGY::IZgyWriter> writer =
IZgyWriter::reopen(ZgyWriterArgs()
.iocontext(Test_Utils::default_sd_context())
.filename(filename));
if (!TEST_CHECK(bool(writer)))
return false;
if (verbose())
std::cout << "Writable \"" << filename << "\".\n";
return true;
}
catch (const std::exception& ex)
{
showex(ex);
if (verbose())
std::cout << "Read-only \"" << filename << "\". " << ex.what()<< "\n";
return false;
}
}
/**
* Check handling of the read-only state, given the 3 boolean settings
* in the IOContext.
*
* The following is NOT tested:
*
* - Confirm that opening a file does not set a lock if it is
* read-only and vice versa. Problematic to test here because we
* have no direct SDAPI access. Also, that test rightly belongs in
* SDAPI, not here.
*
* - Investigate whether toggling the read-only flag takes effect
* immediately or (more likely) requires a close and re-open. When
* a file is open for write and set to read-only mode, can the
* writer still output to it? Test is problematic for the same
* reasons as above.
*/
static void
test_roflag()
{
CloudFileAutoDelete cad("roflag.zgy", Test_Utils::default_sd_context());
SeismicStoreIOContext ctx(*Test_Utils::default_sd_context());
// Create a read-only file. AltUrl should work, update should not.
ctx.setRoAfterWrite(true).forceRoBeforeRead(false).forceRwBeforeWrite(false);
{
std::shared_ptr<OpenZGY::IZgyWriter> writer =
IZgyWriter::open(ZgyWriterArgs()
.iocontext(&ctx)
.size(128, 42, 555)
.filename(cad.name()));
if (!TEST_CHECK(bool(writer)))
return;
writer->close();
writer.reset();
TEST_CHECK(ok_alturl(cad.name()));
TEST_CHECK(!ok_writable(cad.name()));
}
// Create a read/write file. Uses the same file name but this will be
// a delete and re-create. AltUrl should not work, update should.
// Assumption: No need to use the forceRwBeforeWrite hack, i.e.
// SDAPI allows TRUNCATE open even when the file already exists
// and is read-only. The test also verifies that assumption.
{
std::shared_ptr<OpenZGY::IZgyWriter> writer =
IZgyWriter::open(ZgyWriterArgs()
.iocontext(Test_Utils::default_sd_context())
.size(128, 42, 555)
.filename(cad.name()));
if (!TEST_CHECK(bool(writer)))
return;
writer->close();
writer.reset();
TEST_CHECK(!ok_alturl(cad.name()));
TEST_CHECK(ok_writable(cad.name()));
}
// The file is now writable. See if we can automatically make it readable
// when needed because we want to open it without any locking.
ctx.setRoAfterWrite(false).forceRoBeforeRead(true).forceRwBeforeWrite(false);
{
std::shared_ptr<OpenZGY::IZgyReader> reader =
IZgyReader::open(cad.name(), &ctx);
if (!TEST_CHECK(bool(reader)))
return;
reader->close();
reader.reset();
TEST_CHECK(ok_alturl(cad.name()));
TEST_CHECK(!ok_writable(cad.name()));
}
// The file is now readable. See if we can automatically make it writable
// when the applicaton wants to update it. Leave it writable when done.
ctx.setRoAfterWrite(false).forceRoBeforeRead(false).forceRwBeforeWrite(true);
{
std::shared_ptr<OpenZGY::IZgyWriter> writer =
IZgyWriter::reopen(ZgyWriterArgs()
.iocontext(&ctx)
.filename(cad.name()));
if (!TEST_CHECK(bool(writer)))
return;
writer->close();
writer.reset();
TEST_CHECK(!ok_alturl(cad.name()));
TEST_CHECK(ok_writable(cad.name()));
}
// The file is now writable. See if we can automatically make it readable
// when needed because we want to get an AltUrl.
ctx.setRoAfterWrite(false).forceRoBeforeRead(true).forceRwBeforeWrite(false);
{
std::shared_ptr<OpenZGY::IZgyReader> reader =
IZgyReader::open(cad.name(), &ctx);
if (!TEST_CHECK(bool(reader)))
return;
reader->close();
reader.reset();
TEST_CHECK(ok_alturl(cad.name()));
TEST_CHECK(!ok_writable(cad.name()));
}
}
#if 0 // Only for ad-hoc debugging
namespace {
......@@ -2914,6 +3072,7 @@ public:
register_sd_test("api.bat_sd_2", test_bat_sd_2);
register_sd_test("api.bat_sd_4", test_bat_sd_4);
register_sd_test("api.bat_sd_zfp", test_bat_sd_zfp);
register_sd_test("api.roflag", test_roflag);
#if 0 // Not usable as an automated test
register_sd_test("bug.671969", test_bug_671969);
#endif
......
Markdown is supported
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