From cfbd8ed93c5d51e7984a29b99dcfe292c7a0cd30 Mon Sep 17 00:00:00 2001 From: Jan Date: Fri, 23 Jun 2017 15:27:09 +0200 Subject: [PATCH] Bug fix/rocksdb autoincrement (#2648) * Added autoincrement keygen support to RocksDB engine with test. * fixed key generator state keeping for mmfiles engine --- arangod/MMFiles/MMFilesCollection.cpp | 8 ++ arangod/RocksDBEngine/RocksDBCollection.cpp | 29 +++++ arangod/RocksDBEngine/RocksDBCollection.h | 3 + .../RocksDBEngine/RocksDBCounterManager.cpp | 74 ++++++++++++- arangod/RocksDBEngine/RocksDBCounterManager.h | 14 +++ arangod/RocksDBEngine/RocksDBEngine.cpp | 1 + arangod/RocksDBEngine/RocksDBKey.cpp | 9 +- arangod/RocksDBEngine/RocksDBKey.h | 5 + arangod/RocksDBEngine/RocksDBKeyBounds.cpp | 22 ++-- arangod/RocksDBEngine/RocksDBKeyBounds.h | 23 ++-- arangod/RocksDBEngine/RocksDBTypes.cpp | 14 ++- arangod/RocksDBEngine/RocksDBTypes.h | 3 +- arangod/RocksDBEngine/RocksDBValue.cpp | 36 ++++++- arangod/RocksDBEngine/RocksDBValue.h | 19 +++- arangod/StorageEngine/PhysicalCollection.cpp | 3 + arangod/VocBase/KeyGenerator.cpp | 2 + js/server/tests/recovery/collection-keygen.js | 102 ++++++++++++++++++ 17 files changed, 336 insertions(+), 31 deletions(-) create mode 100644 js/server/tests/recovery/collection-keygen.js diff --git a/arangod/MMFiles/MMFilesCollection.cpp b/arangod/MMFiles/MMFilesCollection.cpp index 32e41be917..d469f5ba64 100644 --- a/arangod/MMFiles/MMFilesCollection.cpp +++ b/arangod/MMFiles/MMFilesCollection.cpp @@ -2727,6 +2727,14 @@ Result MMFilesCollection::insert(transaction::Methods* trx, // we can get away with the fast hash function here, as key values are // restricted to strings newSlice = slice; + + VPackSlice keySlice = newSlice.get(StaticStrings::KeyString); + if (keySlice.isString()) { + VPackValueLength l; + char const* p = keySlice.getString(l); + TRI_ASSERT(p != nullptr); + _logicalCollection->keyGenerator()->track(p, l); + } } // create marker diff --git a/arangod/RocksDBEngine/RocksDBCollection.cpp b/arangod/RocksDBEngine/RocksDBCollection.cpp index b9afd587f4..5d2c78269c 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.cpp +++ b/arangod/RocksDBEngine/RocksDBCollection.cpp @@ -25,6 +25,7 @@ #include "Basics/ReadLocker.h" #include "Basics/Result.h" #include "Basics/StaticStrings.h" +#include "Basics/StringUtils.h" #include "Basics/VelocyPackHelper.h" #include "Basics/WriteLocker.h" #include "Cache/CacheManagerFeature.h" @@ -1830,6 +1831,34 @@ void RocksDBCollection::createCache() const { TRI_ASSERT(_useCache); } +arangodb::Result RocksDBCollection::serializeKeyGenerator( + rocksdb::Transaction* rtrx) const { + VPackBuilder builder; + builder.openObject(); + _logicalCollection->keyGenerator()->toVelocyPack(builder); + builder.close(); + + RocksDBKey key = RocksDBKey::KeyGeneratorValue(_objectId); + RocksDBValue value = RocksDBValue::KeyGeneratorValue(builder.slice()); + rocksdb::Status s = rtrx->Put(key.string(), value.string()); + + if (!s.ok()) { + LOG_TOPIC(WARN, Logger::ENGINES) << "writing key generator data failed"; + rtrx->Rollback(); + return rocksutils::convertStatus(s); + } + + return {TRI_ERROR_NO_ERROR}; +} + +void RocksDBCollection::deserializeKeyGenerator(RocksDBCounterManager* mgr) { + uint64_t value = mgr->stealKeyGenerator(_objectId); + if (value > 0) { + std::string k(basics::StringUtils::itoa(value)); + _logicalCollection->keyGenerator()->track(k.data(), k.size()); + } +} + void RocksDBCollection::disableCache() const { if (!_cachePresent) { return; diff --git a/arangod/RocksDBEngine/RocksDBCollection.h b/arangod/RocksDBEngine/RocksDBCollection.h index bd73b1a1be..0c1d049331 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.h +++ b/arangod/RocksDBEngine/RocksDBCollection.h @@ -204,6 +204,9 @@ class RocksDBCollection final : public PhysicalCollection { void recalculateIndexEstimates(); + Result serializeKeyGenerator(rocksdb::Transaction*) const; + void deserializeKeyGenerator(arangodb::RocksDBCounterManager* mgr); + private: /// @brief return engine-specific figures void figuresSpecific( diff --git a/arangod/RocksDBEngine/RocksDBCounterManager.cpp b/arangod/RocksDBEngine/RocksDBCounterManager.cpp index c6690687e7..af91da579f 100644 --- a/arangod/RocksDBEngine/RocksDBCounterManager.cpp +++ b/arangod/RocksDBEngine/RocksDBCounterManager.cpp @@ -252,7 +252,7 @@ Result RocksDBCounterManager::sync(bool force) { return rocksutils::convertStatus(s); } - // Now persist the index estimates: + // Now persist the index estimates and key generators { for (auto const& pair : copy) { auto dbColPair = rocksutils::mapObjectToCollection(pair.first); @@ -290,6 +290,11 @@ Result RocksDBCounterManager::sync(bool force) { if (!res.ok()) { return res; } + + res = rocksCollection->serializeKeyGenerator(rtrx.get()); + if (!res.ok()) { + return res; + } } } @@ -375,6 +380,33 @@ void RocksDBCounterManager::readIndexEstimates() { } } +void RocksDBCounterManager::readKeyGenerators() { + WRITE_LOCKER(guard, _rwLock); + RocksDBKeyBounds bounds = RocksDBKeyBounds::KeyGenerators(); + + rocksdb::Comparator const* cmp = _db->GetOptions().comparator; + rocksdb::ReadOptions readOptions; + std::unique_ptr iter(_db->NewIterator(readOptions, + RocksDBColumnFamily::other())); + iter->Seek(bounds.start()); + + for (; iter->Valid() && cmp->Compare(iter->key(), bounds.end()) < 0; + iter->Next()) { + uint64_t objectId = RocksDBKey::objectId(iter->key()); + auto properties = RocksDBValue::data(iter->value()); + uint64_t lastValue = properties.get("lastValue").getUInt(); + + // If this hits we have two generators for the same collection + TRI_ASSERT(_generators.find(objectId) == _generators.end()); + try { + _generators.emplace( objectId, lastValue); + } catch (...) { + // Nothing to do, just validate that no corrupted memory was produced. + TRI_ASSERT(_generators.find(objectId) == _generators.end()); + } + } +} + std::unique_ptr> RocksDBCounterManager::stealIndexEstimator(uint64_t objectId) { std::unique_ptr> res(nullptr); @@ -388,6 +420,18 @@ RocksDBCounterManager::stealIndexEstimator(uint64_t objectId) { return res; } +uint64_t RocksDBCounterManager::stealKeyGenerator(uint64_t objectId) { + uint64_t res = 0; + auto it = _generators.find(objectId); + if (it != _generators.end()) { + // We swap out the stored estimate in order to move it to the caller + res = it->second; + // Drop the now empty estimator + _generators.erase(objectId); + } + return res; +} + void RocksDBCounterManager::clearIndexEstimators() { // We call this to remove all index estimators that have been stored but are // no longer read @@ -397,6 +441,10 @@ void RocksDBCounterManager::clearIndexEstimators() { _estimators.clear(); } +void RocksDBCounterManager::clearKeyGenerators() { + _generators.clear(); +} + /// Parse counter values from rocksdb void RocksDBCounterManager::readCounterValues() { WRITE_LOCKER(guard, _rwLock); @@ -430,6 +478,7 @@ public: std::pair>>>* _estimators; + std::unordered_map* _generators; rocksdb::SequenceNumber currentSeqNum; uint64_t _maxTick = 0; uint64_t _maxHLC = 0; @@ -439,8 +488,9 @@ public: uint64_t, std::pair>>>* - estimators) - : _estimators(estimators), currentSeqNum(0) {} + estimators, + std::unordered_map* generators) + : _estimators(estimators), _generators(generators), currentSeqNum(0) {} ~WBReader() { // update ticks after parsing wal @@ -477,6 +527,20 @@ public: } } + void storeLastKeyValue(uint64_t objectId, uint64_t keyValue) { + auto it = _generators->find(objectId); + if (it == _generators->end() && keyValue != 0) { + try { + _generators->emplace(objectId, keyValue); + } catch (...) {} + return; + } + + if (keyValue > it->second) { + it->second = keyValue; + } + } + void updateMaxTick(const rocksdb::Slice& key, const rocksdb::Slice& value) { // RETURN (side-effect): update _maxTick // @@ -491,6 +555,8 @@ public: if (type == RocksDBEntryType::Document) { storeMaxHLC(RocksDBKey::revisionId(key)); + storeLastKeyValue(RocksDBKey::objectId(key), + RocksDBValue::keyValue(value)); } else if (type == RocksDBEntryType::PrimaryIndexValue) { // document key StringRef ref = RocksDBKey::primaryKey(key); @@ -626,7 +692,7 @@ bool RocksDBCounterManager::parseRocksWAL() { rocksdb::SequenceNumber start = UINT64_MAX; // Tell the WriteBatch reader the transaction markers to look for - auto handler = std::make_unique(&_estimators); + auto handler = std::make_unique(&_estimators, &_generators); for (auto const& pair : _counters) { handler->seqStart.emplace(pair.first, pair.second._sequenceNum); diff --git a/arangod/RocksDBEngine/RocksDBCounterManager.h b/arangod/RocksDBEngine/RocksDBCounterManager.h index 6897fc28c8..6115919331 100644 --- a/arangod/RocksDBEngine/RocksDBCounterManager.h +++ b/arangod/RocksDBEngine/RocksDBCounterManager.h @@ -97,6 +97,9 @@ class RocksDBCounterManager { std::unique_ptr> stealIndexEstimator(uint64_t indexObjectId); + // Steal the key genenerator state that recovery has detected. + uint64_t stealKeyGenerator(uint64_t indexObjectId); + // Free up all index estimators that were not read by any index. // This is to save some memory. // NOTE: After calling this the stored estimate of all not yet @@ -106,6 +109,9 @@ class RocksDBCounterManager { // have been created in memory. void clearIndexEstimators(); + // Clear out key generator map for values not read by any collection. + void clearKeyGenerators(); + protected: struct CMValue { /// ArangoDB transaction ID @@ -124,6 +130,7 @@ class RocksDBCounterManager { void readCounterValues(); void readSettings(); void readIndexEstimates(); + void readKeyGenerators(); bool parseRocksWAL(); @@ -132,6 +139,11 @@ class RocksDBCounterManager { ////////////////////////////////////////////////////////////////////////////// std::unordered_map _counters; + ////////////////////////////////////////////////////////////////////////////// + /// @brief Key generator container + ////////////////////////////////////////////////////////////////////////////// + std::unordered_map _generators; + ////////////////////////////////////////////////////////////////////////////// /// @brief Index Estimator contianer. /// Note the elements in this container will be moved into the @@ -143,6 +155,8 @@ class RocksDBCounterManager { std::unique_ptr>>> _estimators; + + ////////////////////////////////////////////////////////////////////////////// /// @brief synced sequence numbers ////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/RocksDBEngine/RocksDBEngine.cpp b/arangod/RocksDBEngine/RocksDBEngine.cpp index 996f0053f4..d20bdddc27 100644 --- a/arangod/RocksDBEngine/RocksDBEngine.cpp +++ b/arangod/RocksDBEngine/RocksDBEngine.cpp @@ -1387,6 +1387,7 @@ TRI_vocbase_t* RocksDBEngine::openExistingDatabase(TRI_voc_tick_t id, TRI_ASSERT(physical != nullptr); physical->deserializeIndexEstimates(counterManager()); + physical->deserializeKeyGenerator(counterManager()); LOG_TOPIC(DEBUG, arangodb::Logger::FIXME) << "added document collection '" << collection->name() << "'"; } diff --git a/arangod/RocksDBEngine/RocksDBKey.cpp b/arangod/RocksDBEngine/RocksDBKey.cpp index 4c0698c714..8124ef1447 100644 --- a/arangod/RocksDBEngine/RocksDBKey.cpp +++ b/arangod/RocksDBEngine/RocksDBKey.cpp @@ -22,7 +22,7 @@ /// @author Daniel H. Larkin //////////////////////////////////////////////////////////////////////////////// -#include "RocksDBEngine/RocksDBKey.h" +#include "RocksDBKey.h" #include "Basics/Exceptions.h" #include "Logger/Logger.h" #include "RocksDBEngine/RocksDBCommon.h" @@ -116,6 +116,11 @@ RocksDBKey RocksDBKey::ReplicationApplierConfig(TRI_voc_tick_t databaseId) { RocksDBKey RocksDBKey::IndexEstimateValue(uint64_t collectionObjectId) { return RocksDBKey(RocksDBEntryType::IndexEstimateValue, collectionObjectId); } + +RocksDBKey RocksDBKey::KeyGeneratorValue(uint64_t objectId) { + return RocksDBKey(RocksDBEntryType::KeyGeneratorValue, objectId); +} + // ========================= Member methods =========================== RocksDBEntryType RocksDBKey::type(RocksDBKey const& key) { @@ -146,6 +151,7 @@ TRI_voc_cid_t RocksDBKey::collectionId(rocksdb::Slice const& slice) { uint64_t RocksDBKey::objectId(RocksDBKey const& key) { return objectId(key._buffer.data(), key._buffer.size()); } + uint64_t RocksDBKey::objectId(rocksdb::Slice const& slice) { return objectId(slice.data(), slice.size()); } @@ -221,6 +227,7 @@ RocksDBKey::RocksDBKey(RocksDBEntryType type, uint64_t first) case RocksDBEntryType::Database: case RocksDBEntryType::CounterValue: case RocksDBEntryType::IndexEstimateValue: + case RocksDBEntryType::KeyGeneratorValue: case RocksDBEntryType::ReplicationApplierConfig: { size_t length = sizeof(char) + sizeof(uint64_t); _buffer.reserve(length); diff --git a/arangod/RocksDBEngine/RocksDBKey.h b/arangod/RocksDBEngine/RocksDBKey.h index 2a1ee6b8a7..855f0e592b 100644 --- a/arangod/RocksDBEngine/RocksDBKey.h +++ b/arangod/RocksDBEngine/RocksDBKey.h @@ -150,6 +150,11 @@ class RocksDBKey { ////////////////////////////////////////////////////////////////////////////// static RocksDBKey IndexEstimateValue(uint64_t objectId); + ////////////////////////////////////////////////////////////////////////////// + /// @brief Create a fully-specified key for key generator for a collection + ////////////////////////////////////////////////////////////////////////////// + static RocksDBKey KeyGeneratorValue(uint64_t objectId); + public: ////////////////////////////////////////////////////////////////////////////// /// @brief Extracts the type from a key diff --git a/arangod/RocksDBEngine/RocksDBKeyBounds.cpp b/arangod/RocksDBEngine/RocksDBKeyBounds.cpp index e4a1ff3c36..9f3fbcd4a3 100644 --- a/arangod/RocksDBEngine/RocksDBKeyBounds.cpp +++ b/arangod/RocksDBEngine/RocksDBKeyBounds.cpp @@ -92,12 +92,12 @@ RocksDBKeyBounds RocksDBKeyBounds::GeoIndex(uint64_t indexId, bool isSlot) { uint64ToPersistent(internals.buffer(), norm); // lower endian internals.separate(); - + internals.push_back(static_cast(RocksDBEntryType::GeoIndexValue)); uint64ToPersistent(internals.buffer(), indexId); norm = norm | (0xFFFFFFFFULL << 32); uint64ToPersistent(internals.buffer(), norm); - + return b; } @@ -126,6 +126,10 @@ RocksDBKeyBounds RocksDBKeyBounds::IndexEstimateValues() { return RocksDBKeyBounds(RocksDBEntryType::IndexEstimateValue); } +RocksDBKeyBounds RocksDBKeyBounds::KeyGenerators() { + return RocksDBKeyBounds(RocksDBEntryType::KeyGeneratorValue); +} + RocksDBKeyBounds RocksDBKeyBounds::FulltextIndexPrefix( uint64_t indexId, arangodb::StringRef const& word) { // I did not want to pass a bool to the constructor for this @@ -138,7 +142,7 @@ RocksDBKeyBounds RocksDBKeyBounds::FulltextIndexPrefix( static_cast(RocksDBEntryType::FulltextIndexValue)); uint64ToPersistent(internals.buffer(), indexId); internals.buffer().append(word.data(), word.length()); - + internals.separate(); internals.push_back( static_cast(RocksDBEntryType::FulltextIndexValue)); @@ -157,11 +161,11 @@ RocksDBKeyBounds RocksDBKeyBounds::FulltextIndexComplete( // ============================ Member Methods ============================== -RocksDBKeyBounds::RocksDBKeyBounds(RocksDBKeyBounds const& other) +RocksDBKeyBounds::RocksDBKeyBounds(RocksDBKeyBounds const& other) : _type(other._type), _internals(other._internals) {} -RocksDBKeyBounds::RocksDBKeyBounds(RocksDBKeyBounds&& other) +RocksDBKeyBounds::RocksDBKeyBounds(RocksDBKeyBounds&& other) : _type(other._type), _internals(std::move(other._internals)) {} @@ -296,7 +300,7 @@ RocksDBKeyBounds::RocksDBKeyBounds(RocksDBEntryType type, uint64_t first) _internals.reserve(length); _internals.push_back(static_cast(_type)); uint64ToPersistent(_internals.buffer(), first); - + _internals.separate(); _internals.push_back(static_cast(_type)); @@ -312,7 +316,7 @@ RocksDBKeyBounds::RocksDBKeyBounds(RocksDBEntryType type, uint64_t first) _internals.reserve(length); _internals.push_back(static_cast(_type)); uint64ToPersistent(_internals.buffer(), first); - + _internals.separate(); _internals.push_back(static_cast(_type)); @@ -341,7 +345,7 @@ RocksDBKeyBounds::RocksDBKeyBounds(RocksDBEntryType type, uint64_t first, _internals.push_back(_stringSeparator); _internals.separate(); - + _internals.push_back(static_cast(_type)); uint64ToPersistent(_internals.buffer(), first); _internals.buffer().append(second.data(), second.length()); @@ -395,7 +399,7 @@ namespace arangodb { std::ostream& operator<<(std::ostream& stream, RocksDBKeyBounds const& bounds) { stream << "[bound " << arangodb::rocksDBEntryTypeName(bounds.type()) << ": "; - + auto dump = [&stream](rocksdb::Slice const& slice) { size_t const n = slice.size(); diff --git a/arangod/RocksDBEngine/RocksDBKeyBounds.h b/arangod/RocksDBEngine/RocksDBKeyBounds.h index 21e82d32a9..2adafdb153 100644 --- a/arangod/RocksDBEngine/RocksDBKeyBounds.h +++ b/arangod/RocksDBEngine/RocksDBKeyBounds.h @@ -134,6 +134,11 @@ class RocksDBKeyBounds { ////////////////////////////////////////////////////////////////////////////// static RocksDBKeyBounds IndexEstimateValues(); + ////////////////////////////////////////////////////////////////////////////// + /// @brief Bounds for all key generators + ////////////////////////////////////////////////////////////////////////////// + static RocksDBKeyBounds KeyGenerators(); + ////////////////////////////////////////////////////////////////////////////// /// @brief Bounds for all entries of a fulltext index, matching prefixes ////////////////////////////////////////////////////////////////////////////// @@ -151,7 +156,7 @@ class RocksDBKeyBounds { RocksDBKeyBounds(RocksDBKeyBounds&& other); RocksDBKeyBounds& operator=(RocksDBKeyBounds const& other); RocksDBKeyBounds& operator=(RocksDBKeyBounds&& other); - + RocksDBEntryType type() const { return _type; } ////////////////////////////////////////////////////////////////////////////// @@ -207,14 +212,14 @@ class RocksDBKeyBounds { class BoundsBuffer { friend class RocksDBKeyBounds; - public: + public: BoundsBuffer() : _separatorPosition(0) {} - - BoundsBuffer(BoundsBuffer const& other) + + BoundsBuffer(BoundsBuffer const& other) : _buffer(other._buffer), _separatorPosition(other._separatorPosition) { } - BoundsBuffer(BoundsBuffer&& other) + BoundsBuffer(BoundsBuffer&& other) : _buffer(std::move(other._buffer)), _separatorPosition(other._separatorPosition) { other._separatorPosition = 0; } @@ -237,12 +242,12 @@ class RocksDBKeyBounds { } // reserve space for bounds - void reserve(size_t length) { + void reserve(size_t length) { TRI_ASSERT(_separatorPosition == 0); TRI_ASSERT(_buffer.empty()); - _buffer.reserve(length); + _buffer.reserve(length); } - + // mark the end of the start buffer void separate() { TRI_ASSERT(_separatorPosition == 0); @@ -254,7 +259,7 @@ class RocksDBKeyBounds { void push_back(char c) { _buffer.push_back(c); } - + // return the internal buffer for modification or reading std::string& buffer() { return _buffer; } std::string const& buffer() const { return _buffer; } diff --git a/arangod/RocksDBEngine/RocksDBTypes.cpp b/arangod/RocksDBEngine/RocksDBTypes.cpp index a4e82f1c13..ba932ff003 100644 --- a/arangod/RocksDBEngine/RocksDBTypes.cpp +++ b/arangod/RocksDBEngine/RocksDBTypes.cpp @@ -72,14 +72,14 @@ static rocksdb::Slice UniqueIndexValue( reinterpret_cast::type*>( &uniqueIndexValue), 1); - + static RocksDBEntryType fulltextIndexValue = RocksDBEntryType::FulltextIndexValue; static rocksdb::Slice FulltextIndexValue( reinterpret_cast::type*>( &fulltextIndexValue), 1); - + static RocksDBEntryType geoIndexValue = RocksDBEntryType::GeoIndexValue; static rocksdb::Slice GeoIndexValue( @@ -109,6 +109,13 @@ static rocksdb::Slice IndexEstimateValue( reinterpret_cast::type*>( &indexEstimateValue), 1); + +static RocksDBEntryType keyGeneratorValue = + RocksDBEntryType::KeyGeneratorValue; +static rocksdb::Slice KeyGeneratorValue( + reinterpret_cast::type*>( + &keyGeneratorValue), + 1); } char const* arangodb::rocksDBEntryTypeName(arangodb::RocksDBEntryType type) { @@ -127,6 +134,7 @@ char const* arangodb::rocksDBEntryTypeName(arangodb::RocksDBEntryType type) { case arangodb::RocksDBEntryType::FulltextIndexValue: return "FulltextIndexValue"; case arangodb::RocksDBEntryType::GeoIndexValue: return "GeoIndexValue"; case arangodb::RocksDBEntryType::IndexEstimateValue: return "IndexEstimateValue"; + case arangodb::RocksDBEntryType::KeyGeneratorValue: return "KeyGeneratorValue"; } return "Invalid"; } @@ -184,6 +192,8 @@ rocksdb::Slice const& arangodb::rocksDBSlice(RocksDBEntryType const& type) { return ReplicationApplierConfig; case RocksDBEntryType::IndexEstimateValue: return IndexEstimateValue; + case RocksDBEntryType::KeyGeneratorValue: + return KeyGeneratorValue; } return Document; // avoids warning - errorslice instead ?! diff --git a/arangod/RocksDBEngine/RocksDBTypes.h b/arangod/RocksDBEngine/RocksDBTypes.h index 6ae3f05eaa..5ed9cda451 100644 --- a/arangod/RocksDBEngine/RocksDBTypes.h +++ b/arangod/RocksDBEngine/RocksDBTypes.h @@ -49,7 +49,8 @@ enum class RocksDBEntryType : char { ReplicationApplierConfig = ':', FulltextIndexValue = ';', GeoIndexValue = '<', - IndexEstimateValue = '=' + IndexEstimateValue = '=', + KeyGeneratorValue = '>' }; char const* rocksDBEntryTypeName(RocksDBEntryType); diff --git a/arangod/RocksDBEngine/RocksDBValue.cpp b/arangod/RocksDBEngine/RocksDBValue.cpp index 5c5dd9529b..bf51788e52 100644 --- a/arangod/RocksDBEngine/RocksDBValue.cpp +++ b/arangod/RocksDBEngine/RocksDBValue.cpp @@ -24,6 +24,8 @@ #include "RocksDBEngine/RocksDBValue.h" #include "Basics/Exceptions.h" +#include "Basics/StaticStrings.h" +#include "Basics/StringUtils.h" #include "RocksDBEngine/RocksDBCommon.h" using namespace arangodb; @@ -65,6 +67,10 @@ RocksDBValue RocksDBValue::ReplicationApplierConfig(VPackSlice const& data) { return RocksDBValue(RocksDBEntryType::ReplicationApplierConfig, data); } +RocksDBValue RocksDBValue::KeyGeneratorValue(VPackSlice const& data) { + return RocksDBValue(RocksDBEntryType::KeyGeneratorValue, data); +} + RocksDBValue RocksDBValue::Empty(RocksDBEntryType type) { return RocksDBValue(type); } @@ -97,6 +103,18 @@ VPackSlice RocksDBValue::data(std::string const& s) { return data(s.data(), s.size()); } +uint64_t RocksDBValue::keyValue(RocksDBValue const& value) { + return keyValue(value._buffer.data(), value._buffer.size()); +} + +uint64_t RocksDBValue::keyValue(rocksdb::Slice const& slice) { + return keyValue(slice.data(), slice.size()); +} + +uint64_t RocksDBValue::keyValue(std::string const& s) { + return keyValue(s.data(), s.size()); +} + RocksDBValue::RocksDBValue(RocksDBEntryType type) : _type(type), _buffer() {} RocksDBValue::RocksDBValue(RocksDBEntryType type, uint64_t data) @@ -121,6 +139,7 @@ RocksDBValue::RocksDBValue(RocksDBEntryType type, VPackSlice const& data) case RocksDBEntryType::Collection: case RocksDBEntryType::Document: case RocksDBEntryType::View: + case RocksDBEntryType::KeyGeneratorValue: case RocksDBEntryType::ReplicationApplierConfig: { _buffer.reserve(static_cast(data.byteSize())); _buffer.append(reinterpret_cast(data.begin()), @@ -141,7 +160,7 @@ RocksDBValue::RocksDBValue(RocksDBEntryType type, StringRef const& data) _buffer.append(data.data(), data.size()); break; } - + default: THROW_ARANGO_EXCEPTION(TRI_ERROR_BAD_PARAMETER); } @@ -163,3 +182,18 @@ VPackSlice RocksDBValue::data(char const* data, size_t size) { TRI_ASSERT(size >= sizeof(char)); return VPackSlice(data); } + +uint64_t RocksDBValue::keyValue(char const* data, size_t size) { + TRI_ASSERT(data != nullptr); + TRI_ASSERT(size >= sizeof(char)); + VPackSlice slice(data); + VPackSlice key = slice.get(StaticStrings::KeyString); + if (key.isString()) { + std::string s = key.copyString(); + if (s.size() > 0 && s[0] >= '0' && s[0] <= '9') { + return basics::StringUtils::uint64(s); + } + } + + return 0; +} diff --git a/arangod/RocksDBEngine/RocksDBValue.h b/arangod/RocksDBEngine/RocksDBValue.h index 21b9722346..989b2924ba 100644 --- a/arangod/RocksDBEngine/RocksDBValue.h +++ b/arangod/RocksDBEngine/RocksDBValue.h @@ -54,6 +54,7 @@ class RocksDBValue { static RocksDBValue UniqueIndexValue(TRI_voc_rid_t revisionId); static RocksDBValue View(VPackSlice const& data); static RocksDBValue ReplicationApplierConfig(VPackSlice const& data); + static RocksDBValue KeyGeneratorValue(VPackSlice const& data); ////////////////////////////////////////////////////////////////////////////// /// @brief Used to construct an empty value of the given type for retrieval @@ -77,7 +78,7 @@ class RocksDBValue { /// May be called only on EdgeIndexValue values. Other types will throw. ////////////////////////////////////////////////////////////////////////////// static StringRef vertexId(rocksdb::Slice const&); - + ////////////////////////////////////////////////////////////////////////////// /// @brief Extracts the VelocyPack data from a value /// @@ -88,6 +89,15 @@ class RocksDBValue { static VPackSlice data(rocksdb::Slice const&); static VPackSlice data(std::string const&); + ////////////////////////////////////////////////////////////////////////////// + /// @brief Extracts the numeric value from the key field of a VPackSlice + /// + /// May be called only on values of the following types: KeyGeneratorValue. + ////////////////////////////////////////////////////////////////////////////// + static uint64_t keyValue(RocksDBValue const&); + static uint64_t keyValue(rocksdb::Slice const&); + static uint64_t keyValue(std::string const&); + public: ////////////////////////////////////////////////////////////////////////////// /// @brief Returns a reference to the underlying string buffer. @@ -100,10 +110,10 @@ class RocksDBValue { RocksDBValue(RocksDBEntryType type, rocksdb::Slice slice) : _type(type), _buffer(slice.data(), slice.size()) {} - - RocksDBValue(RocksDBValue&& other) + + RocksDBValue(RocksDBValue&& other) : _type(other._type), _buffer(std::move(other._buffer)) {} - + private: RocksDBValue(); explicit RocksDBValue(RocksDBEntryType type); @@ -116,6 +126,7 @@ class RocksDBValue { static TRI_voc_rid_t revisionId(char const* data, uint64_t size); static StringRef vertexId(char const* data, size_t size); static VPackSlice data(char const* data, size_t size); + static uint64_t keyValue(char const* data, size_t size); private: RocksDBEntryType const _type; diff --git a/arangod/StorageEngine/PhysicalCollection.cpp b/arangod/StorageEngine/PhysicalCollection.cpp index 8d2c473624..6be4f9040c 100644 --- a/arangod/StorageEngine/PhysicalCollection.cpp +++ b/arangod/StorageEngine/PhysicalCollection.cpp @@ -251,6 +251,9 @@ int PhysicalCollection::newObjectForInsert( return res; } builder.add(StaticStrings::KeyString, s); + + // track the key just used + _logicalCollection->keyGenerator()->track(p, l); } // _id diff --git a/arangod/VocBase/KeyGenerator.cpp b/arangod/VocBase/KeyGenerator.cpp index 3172bd5503..093d0669a7 100644 --- a/arangod/VocBase/KeyGenerator.cpp +++ b/arangod/VocBase/KeyGenerator.cpp @@ -267,6 +267,7 @@ void TraditionalKeyGenerator::toVelocyPack(VPackBuilder& builder) const { TRI_ASSERT(!builder.isClosed()); builder.add("type", VPackValue(name())); builder.add("allowUserKeys", VPackValue(_allowUserKeys)); + builder.add("lastValue", VPackValue(_lastValue)); } /// @brief create the generator @@ -372,6 +373,7 @@ void AutoIncrementKeyGenerator::toVelocyPack(VPackBuilder& builder) const { builder.add("allowUserKeys", VPackValue(_allowUserKeys)); builder.add("offset", VPackValue(_offset)); builder.add("increment", VPackValue(_increment)); + builder.add("lastValue", VPackValue(_lastValue)); } /// @brief validate a document id (collection name + / + document key) diff --git a/js/server/tests/recovery/collection-keygen.js b/js/server/tests/recovery/collection-keygen.js new file mode 100644 index 0000000000..561a34e13c --- /dev/null +++ b/js/server/tests/recovery/collection-keygen.js @@ -0,0 +1,102 @@ +/* jshint globalstrict:false, strict:false, unused: false */ +/* global assertTrue, assertFalse, assertEqual */ +// ////////////////////////////////////////////////////////////////////////////// +// / @brief tests for transactions +// / +// / @file +// / +// / DISCLAIMER +// / +// / Copyright 2010-2012 triagens GmbH, Cologne, Germany +// / +// / Licensed under the Apache License, Version 2.0 (the "License") +// / you may not use this file except in compliance with the License. +// / You may obtain a copy of the License at +// / +// / http://www.apache.org/licenses/LICENSE-2.0 +// / +// / Unless required by applicable law or agreed to in writing, software +// / distributed under the License is distributed on an "AS IS" BASIS, +// / WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// / See the License for the specific language governing permissions and +// / limitations under the License. +// / +// / Copyright holder is triAGENS GmbH, Cologne, Germany +// / +// / @author Jan Steemann +// / @author Copyright 2013, triAGENS GmbH, Cologne, Germany +// ////////////////////////////////////////////////////////////////////////////// + +var db = require('@arangodb').db; +var internal = require('internal'); +var jsunity = require('jsunity'); + +function runSetup () { + 'use strict'; + internal.debugClearFailAt(); + var c; + + db._drop('UnitTestsRecovery1'); + c = db._create('UnitTestsRecovery1', { keyOptions: { type: 'autoincrement', + offset: 0, increment: 1 } } ); + c.save({ name: 'a' }); + c.save({ name: 'b' }); + c.save({ name: 'c' }, { waitForSync: true }); + + db._drop('UnitTestsRecovery2'); + c = db._create('UnitTestsRecovery2', { keyOptions: { type: 'autoincrement', + offset: 10, increment: 5 } } ); + c.save({ name: 'a' }); + c.save({ name: 'b' }); + c.save({ name: 'c' }, { waitForSync: true }); + + internal.debugSegfault('crashing server'); +} + +// ////////////////////////////////////////////////////////////////////////////// +// / @brief test suite +// ////////////////////////////////////////////////////////////////////////////// + +function recoverySuite () { + 'use strict'; + jsunity.jsUnity.attachAssertions(); + + return { + setUp: function () {}, + tearDown: function () {}, + + // ////////////////////////////////////////////////////////////////////////////// + // / @brief test whether we can restore the trx data + // ////////////////////////////////////////////////////////////////////////////// + + testCollectionKeyGen: function () { + var c, d; + + c = db._collection('UnitTestsRecovery1'); + assertEqual(["1", "2", "3"], c.toArray().map(function(doc) { return doc._key; }).sort()); + d = c.save({ name: "d"}); + assertEqual("4", d._key); + + c = db._collection('UnitTestsRecovery2'); + assertEqual(["10", "15", "20"], c.toArray().map(function(doc) { return doc._key; }).sort()); + d = c.save({ name: "d"}); + assertEqual("25", d._key); + } + + }; +} + +// ////////////////////////////////////////////////////////////////////////////// +// / @brief executes the test suite +// ////////////////////////////////////////////////////////////////////////////// + +function main (argv) { + 'use strict'; + if (argv[1] === 'setup') { + runSetup(); + return 0; + } else { + jsunity.run(recoverySuite); + return jsunity.done().status ? 0 : 1; + } +}