sapling/eden/fs/store/RocksDbLocalStore.cpp

681 lines
22 KiB
C++
Raw Normal View History

/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This software may be used and distributed according to the terms of the
* GNU General Public License version 2.
*/
#include "eden/fs/store/RocksDbLocalStore.h"
#include <array>
#include <fb303/ServiceData.h>
#include <folly/Format.h>
#include <folly/String.h>
#include <folly/container/Enumerate.h>
#include <folly/futures/Future.h>
#include <folly/io/Cursor.h>
#include <folly/io/IOBuf.h>
#include <folly/lang/Bits.h>
#include <folly/logging/xlog.h>
#include <rocksdb/convenience.h>
#include <rocksdb/db.h>
#include <rocksdb/filter_policy.h>
#include <rocksdb/table.h>
#include "eden/fs/config/EdenConfig.h"
#include "eden/fs/rocksdb/RocksException.h"
#include "eden/fs/rocksdb/RocksHandles.h"
#include "eden/fs/store/KeySpaces.h"
#include "eden/fs/store/StoreResult.h"
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
#include "eden/fs/utils/FaultInjector.h"
using facebook::eden::Hash;
using folly::ByteRange;
using folly::IOBuf;
using folly::StringPiece;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
using folly::Synchronized;
using folly::io::Cursor;
using rocksdb::FlushOptions;
using rocksdb::ReadOptions;
using rocksdb::Slice;
using rocksdb::SliceParts;
using rocksdb::WriteBatch;
using rocksdb::WriteOptions;
using std::string;
using std::unique_ptr;
namespace {
using namespace facebook::eden;
rocksdb::ColumnFamilyOptions makeColumnOptions(uint64_t LRUblockCacheSizeMB) {
rocksdb::ColumnFamilyOptions options;
// We'll never perform range scans on any of the keys that we store.
// This enables bloom filters and a hash policy that improves our
// get/put performance.
options.OptimizeForPointLookup(LRUblockCacheSizeMB);
options.OptimizeLevelStyleCompaction();
return options;
}
/**
* The different key spaces that we desire.
* The ordering is coupled with the values of the LocalStore::KeySpace enum.
*/
const std::vector<rocksdb::ColumnFamilyDescriptor>& columnFamilies() {
auto makeColumnFamilyDescriptors = [] {
// Most of the column families will share the same cache. We
// want the blob data to live in its own smaller cache; the assumption
// is that the vfs cache will compensate for that, together with the
// idea that we shouldn't need to materialize a great many files.
auto options = makeColumnOptions(64);
auto blobOptions = makeColumnOptions(8);
// Meyers singleton to avoid SIOF issues
std::vector<rocksdb::ColumnFamilyDescriptor> families;
for (size_t ks = 0; ks < kKeySpaceRecords.size(); ++ks) {
families.emplace_back(
kKeySpaceRecords[ks].name.str(),
(ks == LocalStore::BlobFamily) ? blobOptions : options);
}
// Put the default column family last.
// This way the KeySpace enum values can be used directly as indexes
// into our column family vectors.
families.emplace_back(rocksdb::kDefaultColumnFamilyName, options);
return families;
};
// Meyers singleton to avoid SIOF issues
static const std::vector<rocksdb::ColumnFamilyDescriptor> families =
makeColumnFamilyDescriptors();
return families;
}
/**
* Return a rocksdb::Range that contains all possible keys that we store.
*
* The input string will be used to store data for the Range slices.
* The caller must ensure that the rangeStorage parameter remains valid and
* unmodified until they are done using the returned Range.
*/
rocksdb::Range getFullRange(std::string& rangeStorage) {
// An empty slice is the lowest possible value.
Slice begin;
// All of our keys are currently 20 bytes.
// Use a longer key to ensure that this is greater than any valid key.
rangeStorage = std::string(
21, static_cast<char>(std::numeric_limits<unsigned char>::max()));
Slice end(rangeStorage);
return rocksdb::Range(begin, end);
}
rocksdb::Slice _createSlice(folly::ByteRange bytes) {
return Slice(reinterpret_cast<const char*>(bytes.data()), bytes.size());
}
class RocksDbWriteBatch : public LocalStore::WriteBatch {
public:
void put(
LocalStore::KeySpace keySpace,
folly::ByteRange key,
folly::ByteRange value) override;
void put(
LocalStore::KeySpace keySpace,
folly::ByteRange key,
std::vector<folly::ByteRange> valueSlices) override;
void flush() override;
~RocksDbWriteBatch() override;
// Use LocalStore::beginWrite() to create a write batch
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
RocksDbWriteBatch(
Synchronized<RocksHandles>::RLockedPtr&& dbHandles,
size_t bufferSize);
void flushIfNeeded();
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
folly::Synchronized<RocksHandles>::RLockedPtr lockedDB_;
rocksdb::WriteBatch writeBatch_;
size_t bufSize_;
};
void RocksDbWriteBatch::flush() {
auto pending = writeBatch_.Count();
if (pending == 0) {
return;
}
XLOG(DBG5) << "Flushing " << pending << " entries with data size of "
<< writeBatch_.GetDataSize();
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto status = lockedDB_->db->Write(WriteOptions(), &writeBatch_);
XLOG(DBG5) << "... Flushed";
if (!status.ok()) {
throw RocksException::build(
status, "error putting blob batch in local store");
}
writeBatch_.Clear();
}
void RocksDbWriteBatch::flushIfNeeded() {
auto needFlush = bufSize_ > 0 && writeBatch_.GetDataSize() >= bufSize_;
if (needFlush) {
flush();
}
}
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
RocksDbWriteBatch::RocksDbWriteBatch(
Synchronized<RocksHandles>::RLockedPtr&& dbHandles,
size_t bufSize)
: LocalStore::WriteBatch(),
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
lockedDB_(std::move(dbHandles)),
writeBatch_(bufSize),
bufSize_(bufSize) {}
RocksDbWriteBatch::~RocksDbWriteBatch() {
if (writeBatch_.Count() > 0) {
XLOG(ERR) << "WriteBatch being destroyed with " << writeBatch_.Count()
<< " items pending flush";
}
}
void RocksDbWriteBatch::put(
LocalStore::KeySpace keySpace,
folly::ByteRange key,
folly::ByteRange value) {
writeBatch_.Put(
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
lockedDB_->columns[keySpace].get(),
_createSlice(key),
_createSlice(value));
flushIfNeeded();
}
void RocksDbWriteBatch::put(
LocalStore::KeySpace keySpace,
folly::ByteRange key,
std::vector<folly::ByteRange> valueSlices) {
std::vector<Slice> slices;
for (auto& valueSlice : valueSlices) {
slices.emplace_back(_createSlice(valueSlice));
}
auto keySlice = _createSlice(key);
SliceParts keyParts(&keySlice, 1);
writeBatch_.Put(
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
lockedDB_->columns[keySpace].get(),
keyParts,
SliceParts(slices.data(), slices.size()));
flushIfNeeded();
}
rocksdb::Options getRocksdbOptions() {
rocksdb::Options options;
// Optimize RocksDB. This is the easiest way to get RocksDB to perform well.
options.IncreaseParallelism();
// Create the DB if it's not already present.
options.create_if_missing = true;
// Automatically create column families as we define new ones.
options.create_missing_column_families = true;
return options;
}
RocksHandles openDB(AbsolutePathPiece path, RocksDBOpenMode mode) {
auto options = getRocksdbOptions();
try {
return RocksHandles(path.stringPiece(), mode, options, columnFamilies());
} catch (const RocksException& ex) {
XLOG(ERR) << "Error opening RocksDB storage at " << path << ": "
<< ex.what();
if (mode == RocksDBOpenMode::ReadOnly) {
// In read-only mode fail rather than attempting to repair the DB.
throw;
}
// Fall through and attempt to repair the DB
}
RocksDbLocalStore::repairDB(path);
// Now try opening the DB again.
return RocksHandles(path.stringPiece(), mode, options, columnFamilies());
}
} // namespace
namespace facebook {
namespace eden {
RocksDbLocalStore::RocksDbLocalStore(
AbsolutePathPiece pathToRocksDb,
FaultInjector* faultInjector,
RocksDBOpenMode mode)
: faultInjector_(*faultInjector),
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
ioPool_(12, "RocksLocalStore"),
dbHandles_(folly::in_place, openDB(pathToRocksDb, mode)) {
// Publish fb303 stats once when we first open the DB.
// These will be kept up-to-date later by the periodicManagementTask() call.
publishStats();
}
RocksDbLocalStore::~RocksDbLocalStore() {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
close();
}
void RocksDbLocalStore::close() {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
// Acquire dbHandles_ in write-lock mode.
// Since any other access to the DB acquires a read lock this will block until
// all current DB operations are complete.
auto handles = dbHandles_.wlock();
handles->close();
}
void RocksDbLocalStore::repairDB(AbsolutePathPiece path) {
XLOG(ERR) << "Attempting to repair RocksDB " << path;
rocksdb::ColumnFamilyOptions unknownColumFamilyOptions;
unknownColumFamilyOptions.OptimizeForPointLookup(8);
unknownColumFamilyOptions.OptimizeLevelStyleCompaction();
const auto& columnDescriptors = columnFamilies();
auto dbPathStr = path.stringPiece().str();
rocksdb::DBOptions dbOptions(getRocksdbOptions());
auto status = RepairDB(
dbPathStr, dbOptions, columnDescriptors, unknownColumFamilyOptions);
if (!status.ok()) {
throw RocksException::build(status, "unable to repair RocksDB at ", path);
}
}
void RocksDbLocalStore::clearKeySpace(KeySpace keySpace) {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
auto columnFamily = handles->columns[keySpace].get();
std::unique_ptr<rocksdb::Iterator> it{
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
handles->db->NewIterator(ReadOptions(), columnFamily)};
XLOG(DBG2) << "clearing column family \"" << columnFamily->GetName() << "\"";
std::string rangeStorage;
const auto fullRange = getFullRange(rangeStorage);
// Delete all SST files that only contain keys in the specified range.
// Since we are deleting everything in this column family this should
// effectively delete everything.
auto status = DeleteFilesInRange(
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
handles->db.get(), columnFamily, &fullRange.start, &fullRange.limit);
if (!status.ok()) {
throw RocksException::build(
status,
"error deleting data in \"",
columnFamily->GetName(),
"\" column family");
}
// Call DeleteRange() as well. In theory DeleteFilesInRange may not delete
// everything in the range (but it probably will in our case since we are
// intending to delete everything).
const WriteOptions writeOptions;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
status = handles->db->DeleteRange(
writeOptions, columnFamily, fullRange.start, fullRange.limit);
if (!status.ok()) {
throw RocksException::build(
status,
"error deleting data in \"",
columnFamily->GetName(),
"\" column family");
}
}
void RocksDbLocalStore::compactKeySpace(KeySpace keySpace) {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
auto options = rocksdb::CompactRangeOptions{};
options.allow_write_stall = true;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto columnFamily = handles->columns[keySpace].get();
XLOG(DBG2) << "compacting column family \"" << columnFamily->GetName()
<< "\"";
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto status = handles->db->CompactRange(
options, columnFamily, /*begin=*/nullptr, /*end=*/nullptr);
if (!status.ok()) {
throw RocksException::build(
status,
"error compacting \"",
columnFamily->GetName(),
"\" column family");
}
}
StoreResult RocksDbLocalStore::get(LocalStore::KeySpace keySpace, ByteRange key)
const {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
string value;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto status = handles->db->Get(
ReadOptions(),
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
handles->columns[keySpace].get(),
_createSlice(key),
&value);
if (!status.ok()) {
if (status.IsNotFound()) {
// Return an empty StoreResult
return StoreResult();
}
// TODO: RocksDB can return a "TryAgain" error.
// Should we try again for the user, rather than re-throwing the error?
// We don't use RocksException::check(), since we don't want to waste our
// time computing the hex string of the key if we succeeded.
throw RocksException::build(
status, "failed to get ", folly::hexlify(key), " from local store");
}
return StoreResult(std::move(value));
}
FOLLY_NODISCARD folly::Future<StoreResult> RocksDbLocalStore::getFuture(
KeySpace keySpace,
folly::ByteRange key) const {
// We're really just passing key through to the get() method, but we need to
// make a copy of it on the way through. It will usually be an eden::Hash
// but can potentially be an arbitrary length so we can't just use Hash as
// the storage here. std::string is appropriate, but there's some noise
// with the conversion from unsigned/signed and back again.
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
return faultInjector_.checkAsync("local store get single", "")
.via(&ioPool_)
.thenValue([keySpace,
key = std::string(
reinterpret_cast<const char*>(key.data()), key.size()),
this](folly::Unit&&) {
return get(
keySpace,
folly::ByteRange(
reinterpret_cast<const unsigned char*>(key.data()),
key.size()));
});
}
FOLLY_NODISCARD folly::Future<std::vector<StoreResult>>
RocksDbLocalStore::getBatch(
KeySpace keySpace,
const std::vector<folly::ByteRange>& keys) const {
std::vector<folly::Future<std::vector<StoreResult>>> futures;
std::vector<std::shared_ptr<std::vector<std::string>>> batches;
batches.emplace_back(std::make_shared<std::vector<std::string>>());
for (auto& key : keys) {
if (batches.back()->size() >= 2048) {
batches.emplace_back(std::make_shared<std::vector<std::string>>());
}
batches.back()->emplace_back(
reinterpret_cast<const char*>(key.data()), key.size());
}
for (auto& batch : batches) {
futures.emplace_back(
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
faultInjector_.checkAsync("local store get batch", "")
.via(&ioPool_)
.thenValue(
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
[store = getSharedFromThis(),
keySpace,
keys = std::move(batch)](folly::Unit&&) {
XLOG(DBG3) << __func__ << " starting to actually do work";
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = store->getHandles();
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
std::vector<Slice> keySlices;
std::vector<std::string> values;
std::vector<rocksdb::ColumnFamilyHandle*> columns;
for (auto& key : *keys) {
keySlices.emplace_back(key);
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
columns.emplace_back(handles->columns[keySpace].get());
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
}
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto statuses = handles->db->MultiGet(
Fix deadlock when restarting during RocksDbLocalStore::get() Summary: If TreeInode::startLoadingInode() is in progress, and EdenServer::startTakeoverShutdown() is called, edenfs can deadlock: 1. Thread A: A FUSE request calls TreeInode::readdir() -> TreeInode::prefetch() -> TreeInode::startLoadingInode() on the children TreeInode-s -> RocksDbLocalStore::getFuture(). 2. Thread B: A takeover request calls EdenServer::performTakeoverShutdown() -> InodeMap::shutdown(). 3. Thread C: RocksDbLocalStore::getFuture() (called in step 1) completes -> TreeInode::inodeLoadComplete(). (The inodeLoadComplete continuation was registered by TreeInode::registerInodeLoadComplete().) 4. Thread C: After TreeInode::inodeLoadComplete() returns, the TreeInode's InodePtr is destructed, dropping the reference count to 0. 5. Thread C: InodeMap::onInodeUnreferenced() -> InodeMap::shutdownComplete() -> EdenMount::shutdown() (called in step 2) completes -> EdenServer::performTakeoverShutdown(). 6. Thread C: EdenServer::performTakeoverShutdown() -> localStore_.reset() -> RocksDbLocalStore::~RocksDbLocalStore(). 7. Thread C: RocksDbLocalStore::~RocksDbLocalStore() signals the thread pool to exit and waits for the pool's threads to exit. Because thread C is one of the threads managed by RocksDbLocalStore's thread pool, the signal is never handled and RocksDbLocalStore::~RocksDbLocalStore() never finishes. Fix this deadlock by executing EdenServer::shutdown()'s callback (in EdenServer::performTakeoverShutdown()) on a different thread. Reviewed By: simpkins Differential Revision: D14337058 fbshipit-source-id: 1d63b4e7d8f5103a2dde31e329150bf763be3db7
2019-03-13 05:25:54 +03:00
ReadOptions(), columns, keySlices, &values);
std::vector<StoreResult> results;
for (size_t i = 0; i < keys->size(); ++i) {
auto& status = statuses[i];
if (!status.ok()) {
if (status.IsNotFound()) {
// Return an empty StoreResult
results.emplace_back(); // StoreResult();
continue;
}
// TODO: RocksDB can return a "TryAgain" error.
// Should we try again for the user, rather than
// re-throwing the error?
// We don't use RocksException::check(), since we don't
// want to waste our time computing the hex string of the
// key if we succeeded.
throw RocksException::build(
status,
"failed to get ",
folly::hexlify(keys->at(i)),
" from local store");
}
results.emplace_back(std::move(values[i]));
}
return results;
}));
}
return folly::collect(futures).thenValue(
[](std::vector<std::vector<StoreResult>>&& tries) {
std::vector<StoreResult> results;
for (auto& batch : tries) {
results.insert(
results.end(),
make_move_iterator(batch.begin()),
make_move_iterator(batch.end()));
}
return results;
});
}
bool RocksDbLocalStore::hasKey(
LocalStore::KeySpace keySpace,
folly::ByteRange key) const {
string value;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
auto status = handles->db->Get(
ReadOptions(),
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
handles->columns[keySpace].get(),
_createSlice(key),
&value);
if (!status.ok()) {
if (status.IsNotFound()) {
return false;
}
// TODO: RocksDB can return a "TryAgain" error.
// Should we try again for the user, rather than re-throwing the error?
// We don't use RocksException::check(), since we don't want to waste our
// time computing the hex string of the key if we succeeded.
throw RocksException::build(
status, "failed to get ", folly::hexlify(key), " from local store");
}
return true;
}
std::unique_ptr<LocalStore::WriteBatch> RocksDbLocalStore::beginWrite(
size_t bufSize) {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
return std::make_unique<RocksDbWriteBatch>(getHandles(), bufSize);
}
void RocksDbLocalStore::put(
LocalStore::KeySpace keySpace,
folly::ByteRange key,
folly::ByteRange value) {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
handles->db->Put(
WriteOptions(),
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
handles->columns[keySpace].get(),
_createSlice(key),
_createSlice(value));
}
uint64_t RocksDbLocalStore::getApproximateSize(
LocalStore::KeySpace keySpace) const {
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto handles = getHandles();
uint64_t size = 0;
// kLiveSstFilesSize reports the size of all "live" sst files.
// This excludes sst files from older snapshot versions that RocksDB may
// still be holding onto. e.g., to provide a consistent view to iterators.
// kTotalSstFilesSize would report the size of all sst files if we wanted to
// report that.
uint64_t sstFilesSize;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
auto result = handles->db->GetIntProperty(
handles->columns[keySpace].get(),
rocksdb::DB::Properties::kLiveSstFilesSize,
&sstFilesSize);
if (result) {
size += sstFilesSize;
} else {
XLOG(WARN) << "unable to retrieve SST file size from RocksDB for key space "
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
<< handles->columns[keySpace]->GetName();
}
// kSizeAllMemTables reports the size of the memtables.
// This is the in-memory space for tracking the data in *.log files that have
// not yet been compacted into a .sst file.
//
// We use this as a something that will hopefully roughly approximate the size
// of the *.log files. In practice this generally seems to be a fair amount
// smaller than the on-disk *.log file size, except immediately after a
// compaction when there is still a couple MB of in-memory metadata despite
// having no uncompacted on-disk data.
uint64_t memtableSize;
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
result = handles->db->GetIntProperty(
handles->columns[keySpace].get(),
rocksdb::DB::Properties::kSizeAllMemTables,
&memtableSize);
if (result) {
size += memtableSize;
} else {
XLOG(WARN) << "unable to retrieve memtable size from RocksDB for key space "
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
<< handles->columns[keySpace]->GetName();
}
return size;
}
void RocksDbLocalStore::periodicManagementTask(const EdenConfig& config) {
// Compute and publish the stats
auto ephemeralSize = publishStats();
// If the ephemeral size is more than the configured limit,
// trigger garbage collection.
auto ephemeralLimit = config.getLocalStoreEphemeralSizeLimit();
if (ephemeralLimit > 0 && ephemeralSize > ephemeralLimit) {
XLOG(INFO) << "scheduling automatic local store garbage collection: "
<< "ephemeral data size " << ephemeralSize
<< " exceeds limit of " << ephemeralLimit;
triggerAutoGC();
}
}
size_t RocksDbLocalStore::publishStats() {
size_t ephemeralSize = 0;
size_t persistentSize = 0;
for (const auto& iter : folly::enumerate(kKeySpaceRecords)) {
auto size =
getApproximateSize(static_cast<LocalStore::KeySpace>(iter.index));
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, iter->name, ".size"), size);
if (iter->persistence == Persistence::Ephemeral) {
ephemeralSize += size;
} else {
persistentSize += size;
}
}
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "ephemeral.total_size"), ephemeralSize);
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "persistent.total_size"), persistentSize);
return ephemeralSize;
}
// In the future it would perhaps be nicer to move the triggerAutoGC()
// logic up into the LocalStore base class. However, for now it is more
// convenient to be able to use RocksDbLocalStore's ioPool_ to schedule the
// work. We could use the EdenServer's main thread pool from the LocalStore
// code, but the gc operation can take a significant amount of time, and it
// seems unfortunate to tie up one of the main pool threads for potentially
// multiple minutes.
void RocksDbLocalStore::triggerAutoGC() {
{
auto state = autoGCState_.wlock();
if (state->inProgress_) {
XLOG(WARN) << "skipping local store garbage collection: "
"another GC job is still running";
fb303::fbData->incrementCounter(
folly::to<string>(statsPrefix_, "auto_gc.schedule_failure"));
return;
}
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "auto_gc.running"), 1);
fb303::fbData->incrementCounter(
folly::to<string>(statsPrefix_, "auto_gc.schedule_count"));
state->startTime_ = std::chrono::steady_clock::now();
state->inProgress_ = true;
}
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
ioPool_.add([store = getSharedFromThis()] {
try {
store->clearCachesAndCompactAll();
} catch (const std::exception& ex) {
XLOG(ERR) << "error during automatic local store garbage collection: "
<< folly::exceptionStr(ex);
store->autoGCFinished(/*successful=*/false);
return;
}
store->autoGCFinished(/*successful=*/true);
});
}
void RocksDbLocalStore::autoGCFinished(bool successful) {
auto state = autoGCState_.wlock();
state->inProgress_ = false;
auto endTime = std::chrono::steady_clock::now();
auto duration = endTime - state->startTime_;
auto durationMS =
std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "auto_gc.running"), 0);
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "auto_gc.last_run"), time(nullptr));
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "auto_gc.last_run_succeeded"),
successful ? 1 : 0);
fb303::fbData->setCounter(
folly::to<string>(statsPrefix_, "auto_gc.last_duration_ms"), durationMS);
if (successful) {
fb303::fbData->incrementCounter(
folly::to<string>(statsPrefix_, "auto_gc.success"));
} else {
fb303::fbData->incrementCounter(
folly::to<string>(statsPrefix_, "auto_gc.failure"));
}
}
fix race conditions in RocksDbLocalStore access during shutdown Summary: This contains several fixes to LocalStore handling during shutdown. - Have EdenServer explicitly call localStore_->close() during shutdown. This ensures that the local store really gets close, just in case some other part of the code somehow still has an outstanding shared_ptr reference to it. - Add synchronization around internal access to the RocksDB object in RocksDbLocalStore. This ensures that calling `close()` is safe even if there happens to still be some outstanding I/O operations. In particular this helps ensure that if background GC operation is in progress that `close()` will wait until it completes before destroying the DB object. This also improves the code so that calling subsequent methods on a closed RocksDbLocalStore throws an exception, instead of simply crashing. I don't believe the additional synchronization in RocksDbLocalStore should have much impact on performance: the synchronization overhead should be very low compared to the cost of the RocksDB reads/writes. Ideally some of this synchronization logic should perhaps be moved into the base `LocalStore` class: all of the different `LocalStore` implementations should ideally ensure that `close()` is thread-safe and blocks until other pending I/O operations are complete. However, that requires a bigger refactoring. I may attempt that in a subsequent diff, but for now I mainly want to address this problem just for RocksDbLocalStore. Reviewed By: strager Differential Revision: D15948382 fbshipit-source-id: 96d633ac0879b3321f596224907fcfe72691b3f0
2019-06-25 04:26:34 +03:00
void RocksDbLocalStore::throwStoreClosedError() const {
// It might be nicer to throw an EdenError exception here.
// At the moment we don't simply due to library dependency ordering in the
// CMake-based build. We should ideally restructure the CMake-based build to
// more closely match our Buck-based library configuration.
throw std::runtime_error("the RocksDB local store is already closed");
}
} // namespace eden
} // namespace facebook