Commit 3546364f authored by Paal Kvamme's avatar Paal Kvamme
Browse files

Merge branch 'kvamme62/auto-read-only-flag' into 'master'

Improved handling of read-only ZGY files.

See merge request !92
parents 1383ef1b ddbee053
Pipeline #59704 passed with stages
in 14 minutes and 21 seconds
......@@ -12,7 +12,7 @@
</head>
<body bgcolor="#ffffff">
<p class="version">This is version 0.4 of the document, last updated 2020-05-26.</p>
<p class="version">This is version 0.4 of the document, last updated 2021-07-23.</p>
<!-- <h1 style="color: red">DRAFT DOCUMENT</h1> -->
......@@ -824,5 +824,128 @@ limitations under the License.
</p>
</li>
</ul>
<h2>Read only ZGY files</h2>
<p>
Seismic Store has the ability to set a file to read-only mode.
This may help performance because less locking is needed
and more caching is possible.
</p>
<p>
The initial plan was to treat all ZGY files on the cloud as
immutable. But requirements have changed. OpenZGY now allows
updating an existing file in some situations. The requirements /
use cases that need to be supported are still not clearly
defined. So I will try to define them here and see if anybody
protests.
</p>
<ul>
<li>
<p>
Petrel needs update capability but only for files that
nave no data blocks yet.
</p>
</li>
<li>
<p>
The Ocean stability guarantee requires update to be
supported via the Ocean API.
</p>
<ul>
<li>
It may be possible to limit this to on-prem files only.
</li>
</ul>
</li>
<li>
<p>
Petrel needs read/write ZGY files for classification.
</p>
<ul>
<li>
It may be possible to limit this to on-prem files only.
</li>
<li>
As long as the file is opened for write, low resolution
data will not be available.
</li>
</ul>
</li>
<li>
<p>
Long running batch jobs need to write part of a file as a
checkpoint.
</p>
<ul>
<li>
The application needs to do a controlled close of the
file. A crash when a file is opened for write will in most
cases corrupt the file.
</li>
</ul>
</li>
<li>
<p>
Additional restrictions.
</p>
<ul>
<li>
A file that is opened both for read and write at the same
time, or open more than once for write, triggers undefined
behavior. It is the application's responsibility to
prevent this from happening.
</li>
</ul>
</li>
</ul>
<h4>Current implementation</h4>
<p>
Three additional settings have been added to the IOContext.
</p>
<dl>
<dt>setRoAfterWrite(bool)</dt>
<dd>
<p>
Set the ZGY file to read-only when done writing it. Has no
effect on files opened for read. Defaults to on. Most
applications will want to turn in this on because most
applications do not expect to update ZGY files in place.
</p>
</dd>
<dt>forceRoBeforeRead(bool)</dt>
<dd>
<p>
Sneak past the mandatory locking in SDAPI by forcing the
read-only flag to true on the ZGY file, if needed, on each
open for read. This allows for less overhead, more caching,
and use of the altUrl feature. This option is useful if the
file is logically immutable but was not flagged as such.
E.g. the creator forgot to call setRoCloseWrite(true), or
the ZGY file was not created by OpenZGY. The option has no
effect on files opened for create or update. Caveat:
Allowing a read-only open to have a permanent effect of the
file being opened is not ideal.
</p>
</dd>
<dt>forceRwBeforeWrite</dt>
<dd>
<p>
Dangerous option. Sneak past the mandatory locking in SDAPI
by forcing the read-only flag to false on the ZGY file, if
needed, that is about to be opened for update. The
application must know that the file is not open for read by
somebody else. There is also a risk that data might exists
in cache even for a closed file. The application assumes all
responsibility.
</p>
</dd>
</dl>
<p>
Files created by the old ZGY-Cloud library will still be left
writable. This means that altUrl will not work for those, unless
forceRoBeforeRead is in effect. Hopefully applications will move
away from the deprecated ZGY-Cloud fast enough that this will
not become a problem.
</p>
</body>
</html>
......@@ -33,6 +33,7 @@ ZgyEndOfFile::ZgyEndOfFile(const std::string& arg) : ZgyError(arg) {}
ZgySegmentIsClosed::ZgySegmentIsClosed(const std::string& arg) : ZgyError(arg) {}
ZgyAborted::ZgyAborted(const std::string& arg) : ZgyError(arg) {}
ZgyMissingFeature::ZgyMissingFeature(const std::string& arg) : ZgyError(arg) {}
ZgyNotReadOnlyError::ZgyNotReadOnlyError(const std::string& arg) : ZgyError(arg) {}
namespace {
static std::string get_error(const std::string& filename, int system_errno)
......
......@@ -249,6 +249,21 @@ public:
ZgyIoError(const std::string& filename, int system_errno);
};
/**
* \brief Data must be read-only.
*
* Some operations such as alturl require that the data is flagged
* read-only. This was not the case here.
*
* Thread safety: Exceptions defined in this module are safe.
*/
class OPENZGY_API ZgyNotReadOnlyError: public ZgyError
{
public:
/** \copybrief OpenZGY::Errors::ZgyNotReadOnlyError */
ZgyNotReadOnlyError(const std::string& filename);
};
/**
* @}
*/
......
......@@ -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.
......@@ -189,6 +191,18 @@ public:
std::shared_ptr<SDGenericDatasetWrapper> datasetwrapper() const {return _dataset;}
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
......@@ -777,13 +791,31 @@ public:
return info_;
}
void close()
/**
* Close the underlying SDGenericDataset.
* If an exception occurs then the wrapper will still see
* the dataset as closed. And dataset() will return empty.
*
* Caveat: There was at one point a bug where SDGenericDataset::close()
* threw an appropriate exception (e.g. credentials timed out) then
* destructing the dataset could also throw. Exceptions from a
* destructor usually causes the program to terminate. Especially in
* 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(bool set_readonly)
{
auto victim = dataset_;
OpenMode disp = disposition_;
dataset_.reset();
disposition_ = OpenMode::Closed;
victim->close();
victim.reset(); // Any throw from SDGenericDataset dtor happens here.
if (victim) {
if (set_readonly &&
(disp == OpenMode::Truncate || disp == OpenMode::ReadWrite))
victim->setReadonlyMode(true);
victim->close();
victim.reset(); // Any throw from SDGenericDataset dtor happens here.
}
}
void updateDataset(std::shared_ptr<seismicdrive::SDGenericDataset> dataset) {
......@@ -1001,42 +1033,18 @@ public:
}
}
}
/**
* Configure the exponential backoff used by Seismic Store.
* Only expected to be used for debugging.
*
* * -1 => use defaults.
* * 0 => Turn off exponential backoff completely.
* * >0 =? Set maximum repeat count.
*/
void setBackoff()
{
int retries = Environment::getNumericEnv("OPENZGY_SD_BACKOFF", -1);
if (retries >= 0) {
seismicdrive::ExponentialRetryBackoffPolicy policy;
if (retries == 0) {
policy.enabled = false;
}
else {
policy.enabled = true;
policy.maxRetry = retries;
policy.initialWaitingTimeMicroSec = 500 * 1000;
policy.maxWaitingTimeMicroSec = 32 * 1000 * 1000;
}
dataset_->setExponentialRetryBackoffPolicy(&policy);
if (this->logger_(2, "")) {
std::stringstream ss;
ss << "Backoff " << (policy.enabled ? "enabled" : "disabled")
<< " retries " << policy.maxRetry
<< " start " << (float)policy.initialWaitingTimeMicroSec*1.0e-6
<< " max " << (float)policy.maxWaitingTimeMicroSec*1.0e-6;
this->logger_(2, ss.str());
}
}
}
};
/**
* Automatically close the dataset when the last reference to it goes away.
* This is a fallback. Please do NOT rely on this behavior. Especially if
* the dataset is open for write. Instead, make sure xx_close() is called
* in a timely manner. If we get here with an open dataset then:
*
* - Exceptions will be logged and swallowed.
* - The C++ runtime might abort due to exception during unwind.
* - Cleanup such as making the dataset read-only might be skipped.
*/
SDGenericDatasetWrapper::~SDGenericDatasetWrapper()
{
info_.reset();
......@@ -1045,19 +1053,12 @@ 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_)
dataset_->close();
wrapper_close(false);
}
catch (const std::exception& ex) {
if (std::string(ex.what()).find("dataset is not open") == std::string::npos)
this->logger_(0, "SDGenericDataset::close(): " + std::string(ex.what()));
}
try {
// Catch exceptions raised inside a destructor might not work. But I tried.
dataset_.reset();
}
catch (const std::exception& ex) {
this->logger_(0, "SDGenericDataset::dtor(): " + std::string(ex.what()));
}
manager_.reset();
}
......@@ -1069,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()
......@@ -1109,40 +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)
datasetwrapper->setBackoff();
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:
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;
}
}
......@@ -1150,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,
......@@ -1168,7 +1154,7 @@ SeismicStoreFile::~SeismicStoreFile()
// the application will crash.
if (_dataset && _dataset->dataset() && _dataset->disposition() != OpenMode::Closed) {
try {
_dataset->close();
_dataset->wrapper_close(_config->_set_ro_after_write);
}
catch (const std::exception& ex) {
_logger(0, "EXCEPTION closing file: " + std::string(ex.what()));
......@@ -1210,6 +1196,148 @@ SeismicStoreFile::_sslogger(int priority, const std::ios& ss) const
return _logger(priority, sstream ? sstream->str() : std::string());
}
/**
* Configure the exponential backoff used by Seismic Store.
* Only expected to be used for debugging.
*
* * -1 => use defaults.
* * 0 => Turn off exponential backoff completely.
* * >0 =? Set maximum repeat count.
*/
void
SeismicStoreFile::_set_backoff(seismicdrive::SDGenericDataset* sdgd)
{
int retries = Environment::getNumericEnv("OPENZGY_SD_BACKOFF", -1);
if (retries >= 0) {
seismicdrive::ExponentialRetryBackoffPolicy policy;
if (retries == 0) {
policy.enabled = false;
}
else {
policy.enabled = true;
policy.maxRetry = retries;
policy.initialWaitingTimeMicroSec = 500 * 1000;
policy.maxWaitingTimeMicroSec = 32 * 1000 * 1000;
}
sdgd->setExponentialRetryBackoffPolicy(&policy);
if (_logger(2, "")) {
std::stringstream ss;
ss << "Backoff " << (policy.enabled ? "enabled" : "disabled")
<< " retries " << policy.maxRetry
<< " start " << (float)policy.initialWaitingTimeMicroSec*1.0e-6
<< " max " << (float)policy.maxWaitingTimeMicroSec*1.0e-6;
_logger(2, ss.str());
}
}
}
/**
* 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)
{
......@@ -1656,7 +1784,7 @@ SeismicStoreFile::xx_close()
<< "Ignoring exception during close: "
<< ex.what());
}
victim->close();
victim->wrapper_close(this->_config->_set_ro_after_write);
}
break;
}
......@@ -1770,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.
......@@ -1795,6 +1928,9 @@ SeismicStoreFile::altUrl(const std::string& filename) const
<< "\n");
return url;
}
catch (const seismicdrive::SDExpectedReadOnlyDatasetException& ex) {
throw OpenZGY::Errors::ZgyNotReadOnlyError(ex.what());
}
catch (const std::exception& ex) {
_dataset->throwCloudException(ex, "altUrl");
throw; // not reached, but compiler might not realize that.
......
......@@ -57,6 +57,9 @@ SeismicStoreIOContext::SeismicStoreIOContext()
legaltag(Environment::getStringEnv("OPENZGY_LEGALTAG"));
writeid(Environment::getStringEnv("OPENZGY_WRITEID"));
seismicmeta(Environment::getStringEnv("OPENZGY_SEISMICMETA"));
setRoAfterWrite(Environment::getNumericEnv("OPENZGY_RO_AFTER_WRITE", 1) > 0);
forceRoBeforeRead(Environment::getNumericEnv("OPENZGY_RO_BEFORE_READ", 0) > 0);
forceRwBeforeWrite(Environment::getNumericEnv("OPENZGY_RW_BEFORE_WRITE", 0) > 0);
}
std::string
......@@ -75,7 +78,12 @@ SeismicStoreIOContext::toString() const
<< " threads: " << _iothreads << " I/O, " << _cputhreads << " CPU.\n"
<< " legaltag: \"" << _legaltag << "\"\n"
<< " writeid: \"" << _writeid << "\"\n"
<< " seismeta: \"" << _seismicmeta << "\"\n";
<< " seismeta: \"" << _seismicmeta << "\"\n"
<< " romode: "
<< (_set_ro_after_write? "ro_after_write":" ")
<< (_force_ro_before_read? "ro_before_read":" ")
<< (_force_rw_before_write? "rw_before_write":" ")
<< "\n";
return ss.str();
}
......
......@@ -117,6 +117,9 @@ private:
tokencb_t _sdtokencb;
debugtrace_t _debug_trace;
logger_t _logger;
bool _set_ro_after_write;