mirrors/mongodb
mirrors/mongodb
0
0
Fork 0
mirror of https://github.com/mongodb/mongo.git synced 2024-11-28 07:59:02 +01:00
Code Issues Releases Activity

SERVER-17370 Fix bugs in storage engine-specific index and collection options

Browse Source
This commit is contained in:
Mathias Stearn 2015-02-24 18:52:57 -05:00
parent 96a10d146d
commit 6d8fb7f991
13 changed files with 114 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
jstests/core/apitest_db.js
View File

@ -40,10 +40,13 @@ assert(found, "found test.test in collection infos");
// storageEngine in collection options must:
// - be a document
// - contain at least one field of document type with the name of a registered storage engine.
// - all fields of the document:
// -- must have names that are registered storage engines
// -- must be objects
db.getCollection('test').drop();
var storageEngineName = db.serverStatus().storageEngine.name;
assert.commandFailed(db.createCollection('test', {storageEngine: {}}));
assert.commandFailed(db.createCollection('test', {storageEngine: 'not a document'}));
assert.commandWorked(db.createCollection('test', {storageEngine: {}}));
assert.commandFailed(db.createCollection('test', {storageEngine: {unknownStorageEngine: {}}}));
var invalidStorageEngineOptions = {}
invalidStorageEngineOptions[storageEngineName] = 12345;

10
src/mongo/db/catalog/collection_options.cpp
View File

@ -144,16 +144,8 @@ namespace mongo {
if (e.type() != mongo::Object) {
return Status(ErrorCodes::BadValue, "'storageEngine' has to be a document.");
}
BSONObjIterator j(e.Obj());
if (!j.more()) {
return Status(ErrorCodes::BadValue,
"Empty 'storageEngine' options are invalid. "
"Please remove, or include valid options.");
}
// Loop through each provided storageEngine.
while (j.more()) {
BSONElement storageEngineElement = j.next();
BSONForEach(storageEngineElement, e.Obj()) {
StringData storageEngineName = storageEngineElement.fieldNameStringData();
if (storageEngineElement.type() != mongo::Object) {
return Status(ErrorCodes::BadValue, str::stream() << "'storageEngine." <<

8
src/mongo/db/catalog/collection_options_test.cpp
View File

@ -87,15 +87,13 @@ namespace mongo {
TEST(CollectionOptions, InvalidStorageEngineField) {
// "storageEngine" field has to be an object if present.
ASSERT_NOT_OK( CollectionOptions().parse(fromjson("{storageEngine: 1}")));
ASSERT_NOT_OK(CollectionOptions().parse(fromjson("{storageEngine: 1}")));
// Every field under "storageEngine" has to be an object.
ASSERT_NOT_OK( CollectionOptions().parse(fromjson(
"{storageEngine: {storageEngine1: 1}}")));
ASSERT_NOT_OK(CollectionOptions().parse(fromjson("{storageEngine: {storageEngine1: 1}}")));
// Empty "storageEngine" not allowed
ASSERT_NOT_OK( CollectionOptions().parse(fromjson(
"{storageEngine: {}}")));
ASSERT_OK(CollectionOptions().parse(fromjson("{storageEngine: {}}")));
}
TEST(CollectionOptions, ParseEngineField) {

9
src/mongo/db/storage/devnull/devnull_init.cpp
View File

@ -53,15 +53,6 @@ namespace mongo {
return "devnull";
}
virtual Status validateCollectionStorageOptions(const BSONObj& options) const
{
return Status::OK();
}
virtual Status validateIndexStorageOptions(const BSONObj& options) const {
return Status::OK();
}
virtual Status validateMetadata(const StorageEngineMetadata& metadata,
const StorageGlobalParams& params) const {
return Status::OK();

9
src/mongo/db/storage/in_memory/in_memory_init.cpp
View File

@ -54,15 +54,6 @@ namespace mongo {
return "inMemoryExperiment";
}
virtual Status validateCollectionStorageOptions(const BSONObj& options) const
{
return Status::OK();
}
virtual Status validateIndexStorageOptions(const BSONObj& options) const {
return Status::OK();
}
virtual Status validateMetadata(const StorageEngineMetadata& metadata,
const StorageGlobalParams& params) const {
return Status::OK();

9
src/mongo/db/storage/mmap_v1/mmap_v1_init.cpp
View File

@ -52,15 +52,6 @@ namespace mongo {
return "mmapv1";
}
virtual Status validateCollectionStorageOptions(const BSONObj& options) const
{
return Status::OK();
}
virtual Status validateIndexStorageOptions(const BSONObj& options) const {
return Status::OK();
}
virtual Status validateMetadata(const StorageEngineMetadata& metadata,
const StorageGlobalParams& params) const {
Status status = metadata.validateStorageEngineOption(

19
src/mongo/db/storage/storage_engine.h
View File

@ -35,6 +35,7 @@
#include "mongo/base/status.h"
#include "mongo/bson/bsonobj.h"
#include "mongo/util/mongoutils/str.h"
namespace mongo {
@ -81,14 +82,28 @@ namespace mongo {
/**
* Validates creation options for a collection in the StorageEngine.
* Returns an error if the creation options are not valid.
*
* Default implementation only accepts empty objects (no options).
*/
virtual Status validateCollectionStorageOptions(const BSONObj& options) const = 0;
virtual Status validateCollectionStorageOptions(const BSONObj& options) const {
if (options.isEmpty()) return Status::OK();
return Status(ErrorCodes::InvalidOptions,
str::stream() << "storage engine " << getCanonicalName()
<< " does not support any collection storage options");
}
/**
* Validates creation options for an index in the StorageEngine.
* Returns an error if the creation options are not valid.
*
* Default implementation only accepts empty objects (no options).
*/
virtual Status validateIndexStorageOptions(const BSONObj& options) const = 0;
virtual Status validateIndexStorageOptions(const BSONObj& options) const {
if (options.isEmpty()) return Status::OK();
return Status(ErrorCodes::InvalidOptions,
str::stream() << "storage engine " << getCanonicalName()
<< " does not support any index storage options");
}
/**
* Validates existing metadata in the data directory against startup options.

10
src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
View File

@ -118,6 +118,7 @@ namespace {
// static
StatusWith<std::string> WiredTigerIndex::parseIndexOptions(const BSONObj& options) {
StringBuilder ss;
BSONForEach(elem, options) {
if (elem.fieldNameStringData() == "configString") {
if (elem.type() != String) {
@ -126,11 +127,7 @@ namespace {
<< "Not adding 'configString' value "
<< elem << " to index configuration");
}
if (elem.valueStringData().empty()) {
return StatusWith<std::string>(ErrorCodes::InvalidOptions,
"configString must be not be an empty string.");
}
return StatusWith<std::string>(elem.String());
ss << elem.valueStringData() << ',';
}
else {
// Return error on first unrecognized field.
@ -139,8 +136,7 @@ namespace {
<< " is not a supported option.");
}
}
return StatusWith<std::string>(ErrorCodes::BadValue,
"Storage engine options document must not be empty.");
return StatusWith<std::string>(ss.str());
}
// static

43
src/mongo/db/storage/wiredtiger/wiredtiger_index_test.cpp
View File

@ -101,49 +101,42 @@ namespace mongo {
}
TEST(WiredTigerIndexTest, GenerateCreateStringEmptyDocument) {
BSONObj spec = fromjson("{storageEngine: {wiredTiger: {}}}");
IndexDescriptor desc(NULL, "", spec);
StatusWith<std::string> result = WiredTigerIndex::generateCreateString("", desc);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::BadValue, status.code());
BSONObj spec = fromjson("{}");
StatusWith<std::string> result = WiredTigerIndex::parseIndexOptions(spec);
ASSERT_OK(result.getStatus());
ASSERT_EQ(result.getValue(), ""); // "," would also be valid.
}
TEST(WiredTigerIndexTest, GenerateCreateStringUnknownField) {
BSONObj spec = fromjson("{storageEngine: {wiredTiger: {unknownField: 1}}}");
IndexDescriptor desc(NULL, "", spec);
StatusWith<std::string> result = WiredTigerIndex::generateCreateString("", desc);
BSONObj spec = fromjson("{unknownField: 1}");
StatusWith<std::string> result = WiredTigerIndex::parseIndexOptions(spec);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::InvalidOptions, status.code());
ASSERT_EQUALS(ErrorCodes::InvalidOptions, status);
}
TEST(WiredTigerIndexTest, GenerateCreateStringNonStringConfig) {
BSONObj spec = fromjson("{storageEngine: {wiredTiger: {configString: 12345}}}");
IndexDescriptor desc(NULL, "", spec);
StatusWith<std::string> result = WiredTigerIndex::generateCreateString("", desc);
BSONObj spec = fromjson("{configString: 12345}");
StatusWith<std::string> result = WiredTigerIndex::parseIndexOptions(spec);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::TypeMismatch, status.code());
ASSERT_EQUALS(ErrorCodes::TypeMismatch, status);
}
TEST(WiredTigerIndexTest, GenerateCreateStringEmptyConfigString) {
BSONObj spec = fromjson("{storageEngine: {wiredTiger: {configString: ''}}}");
IndexDescriptor desc(NULL, "", spec);
StatusWith<std::string> result = WiredTigerIndex::generateCreateString("", desc);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::InvalidOptions, status.code());
BSONObj spec = fromjson("{configString: ''}");
StatusWith<std::string> result = WiredTigerIndex::parseIndexOptions(spec);
ASSERT_OK(result.getStatus());
ASSERT_EQ(result.getValue(), ","); // "" would also be valid.
}
TEST(WiredTigerIndexTest, GenerateCreateStringValidConfigFormat) {
BSONObj spec = fromjson("{storageEngine: {wiredTiger: {configString: 'abc=def'}}}");
IndexDescriptor desc(NULL, "", spec);
StatusWith<std::string> result = WiredTigerIndex::generateCreateString("", desc);
// TODO eventually this should fail since "abc" is not a valid WT option.
BSONObj spec = fromjson("{configString: 'abc=def'}");
StatusWith<std::string> result = WiredTigerIndex::parseIndexOptions(spec);
const Status& status = result.getStatus();
ASSERT_OK(status);
const std::string& config = result.getValue();
ASSERT_NOT_EQUALS(std::string::npos, config.find("abc=def"));
ASSERT_EQ(result.getValue(), "abc=def,");
}
} // namespace mongo

2
src/mongo/db/storage/wiredtiger/wiredtiger_init.cpp
View File

@ -82,7 +82,7 @@ namespace mongo {
}
virtual Status validateCollectionStorageOptions(const BSONObj& options) const {
return Status::OK();
return WiredTigerRecordStore::parseOptionsField(options).getStatus();
}
virtual Status validateIndexStorageOptions(const BSONObj& options) const {

54
src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
View File

@ -89,6 +89,30 @@ namespace {
const long long WiredTigerRecordStore::kCollectionScanOnCreationThreshold = 10000;
StatusWith<std::string> WiredTigerRecordStore::parseOptionsField(const BSONObj options) {
StringBuilder ss;
BSONForEach(elem, options) {
if (elem.fieldNameStringData() == "configString") {
if (elem.type() != String) {
return StatusWith<std::string>(ErrorCodes::TypeMismatch, str::stream()
<< "storageEngine.wiredTiger.configString "
<< "must be a string. "
<< "Not adding 'configString' value "
<< elem << " to collection configuration");
}
ss << elem.valueStringData() << ',';
}
else {
// Return error on first unrecognized field.
return StatusWith<std::string>(ErrorCodes::InvalidOptions, str::stream()
<< '\'' << elem.fieldNameStringData() << '\''
<< " is not a supported option in "
<< "storageEngine.wiredTiger");
}
}
return StatusWith<std::string>(ss.str());
}
// static
StatusWith<std::string> WiredTigerRecordStore::generateCreateString(
StringData ns,
@ -114,30 +138,12 @@ namespace {
ss << extraStrings << ",";
// Validate configuration object.
// Warn about unrecognized fields that may be introduced in newer versions of this
// storage engine instead of raising an error.
// Ensure that 'configString' field is a string. Warn if this is not the case.
BSONForEach(elem, options.storageEngine.getObjectField(kWiredTigerEngineName)) {
if (elem.fieldNameStringData() == "configString") {
if (elem.type() != String) {
return StatusWith<std::string>(ErrorCodes::TypeMismatch, str::stream()
<< "storageEngine.wiredTiger.configString "
<< "must be a string. "
<< "Not adding 'configString' value "
<< elem << " to collection configuration");
continue;
}
ss << elem.valueStringData() << ",";
}
else {
// Return error on first unrecognized field.
return StatusWith<std::string>(ErrorCodes::InvalidOptions, str::stream()
<< '\'' << elem.fieldNameStringData() << '\''
<< " is not a supported option in "
<< "storageEngine.wiredTiger");
}
}
StatusWith<std::string> customOptions =
parseOptionsField(options.storageEngine.getObjectField(kWiredTigerEngineName));
if (!customOptions.isOK())
return customOptions;
ss << customOptions.getValue();
if ( NamespaceString::oplog(ns) ) {
// force file for oplog

7
src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
View File

@ -68,6 +68,13 @@ namespace mongo {
*/
static const long long kCollectionScanOnCreationThreshold;
/**
* Parses collections options for wired tiger configuration string for table creation.
* The document 'options' is typically obtained from the 'wiredTiger' field of
* CollectionOptions::storageEngine.
*/
static StatusWith<std::string> parseOptionsField(const BSONObj options);
/**
* Creates a configuration string suitable for 'config' parameter in WT_SESSION::create().
* Configuration string is constructed from:

37
src/mongo/db/storage/wiredtiger/wiredtiger_record_store_test.cpp
View File

@ -150,22 +150,43 @@ namespace mongo {
return new WiredTigerHarnessHelper();
}
TEST(WiredTigerRecordStoreTest, GenerateCreateStringEmptyDocument) {
BSONObj spec = fromjson("{}");
StatusWith<std::string> result = WiredTigerRecordStore::parseOptionsField(spec);
ASSERT_OK(result.getStatus());
ASSERT_EQ(result.getValue(), ""); // "," would also be valid.
}
TEST(WiredTigerRecordStoreTest, GenerateCreateStringUnknownField) {
CollectionOptions options;
options.storageEngine = fromjson("{wiredTiger: {unknownField: 1}}");
StatusWith<std::string> result = WiredTigerRecordStore::generateCreateString("", options, "");
BSONObj spec = fromjson("{unknownField: 1}");
StatusWith<std::string> result = WiredTigerRecordStore::parseOptionsField(spec);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::InvalidOptions, status.code());
ASSERT_EQUALS(ErrorCodes::InvalidOptions, status);
}
TEST(WiredTigerRecordStoreTest, GenerateCreateStringNonStringConfig) {
CollectionOptions options;
options.storageEngine = fromjson("{wiredTiger: {configString: 12345}}");
StatusWith<std::string> result = WiredTigerRecordStore::generateCreateString("", options, "");
BSONObj spec = fromjson("{configString: 12345}");
StatusWith<std::string> result = WiredTigerRecordStore::parseOptionsField(spec);
const Status& status = result.getStatus();
ASSERT_NOT_OK(status);
ASSERT_EQUALS(ErrorCodes::TypeMismatch, status.code());
ASSERT_EQUALS(ErrorCodes::TypeMismatch, status);
}
TEST(WiredTigerRecordStoreTest, GenerateCreateStringEmptyConfigString) {
BSONObj spec = fromjson("{configString: ''}");
StatusWith<std::string> result = WiredTigerRecordStore::parseOptionsField(spec);
ASSERT_OK(result.getStatus());
ASSERT_EQ(result.getValue(), ","); // "" would also be valid.
}
TEST(WiredTigerRecordStoreTest, GenerateCreateStringValidConfigFormat) {
// TODO eventually this should fail since "abc" is not a valid WT option.
BSONObj spec = fromjson("{configString: 'abc=def'}");
StatusWith<std::string> result = WiredTigerRecordStore::parseOptionsField(spec);
const Status& status = result.getStatus();
ASSERT_OK(status);
ASSERT_EQ(result.getValue(), "abc=def,");
}
TEST(WiredTigerRecordStoreTest, Isolation1 ) {