Review fixes

This commit is contained in:
kssenii 2024-03-26 21:41:32 +01:00
parent 5100b0d52e
commit dc83eb1ad6
4 changed files with 24 additions and 25 deletions

View File

@ -39,7 +39,7 @@ EvictionCandidates::~EvictionCandidates()
// Reset the evicting state // Reset the evicting state
// (as the corresponding file segments were not yet removed). // (as the corresponding file segments were not yet removed).
for (const auto & candidate : key_candidates.candidates) for (const auto & candidate : key_candidates.candidates)
candidate->setEvicting(false, nullptr, nullptr); candidate->resetEvictingFlag();
} }
} }
@ -53,7 +53,7 @@ void EvictionCandidates::add(
it->second.key_metadata = locked_key.getKeyMetadata(); it->second.key_metadata = locked_key.getKeyMetadata();
it->second.candidates.push_back(candidate); it->second.candidates.push_back(candidate);
candidate->setEvicting(true, &locked_key, &lock); candidate->setEvictingFlag(locked_key, lock);
++candidates_size; ++candidates_size;
} }

View File

@ -13,7 +13,6 @@ namespace DB
namespace ErrorCodes namespace ErrorCodes
{ {
extern const int LOGICAL_ERROR; extern const int LOGICAL_ERROR;
extern const int NOT_IMPLEMENTED;
} }
IFileCachePriority::IFileCachePriority(size_t max_size_, size_t max_elements_) IFileCachePriority::IFileCachePriority(size_t max_size_, size_t max_elements_)
@ -52,13 +51,4 @@ void IFileCachePriority::check(const CachePriorityGuard::Lock & lock) const
} }
} }
void IFileCachePriority::holdImpl(size_t, size_t, const CachePriorityGuard::Lock &)
{
throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method holdImpl() is not implemented");
}
void IFileCachePriority::releaseImpl(size_t, size_t)
{
throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method holdImpl() is not implemented");
}
} }

View File

@ -52,14 +52,18 @@ public:
/// a separate lock `EntryGuard::Lock`, it will make this part of code more coherent, /// a separate lock `EntryGuard::Lock`, it will make this part of code more coherent,
/// but it will introduce one more mutex while it is avoidable. /// but it will introduce one more mutex while it is avoidable.
/// Introducing one more mutex just for coherency does not win the trade-off (isn't it?). /// Introducing one more mutex just for coherency does not win the trade-off (isn't it?).
void setEvicting(bool evicting_, const LockedKey * locked_key, const CachePriorityGuard::Lock * lock) const void setEvictingFlag(const LockedKey &, const CachePriorityGuard::Lock &) const
{ {
if (evicting_ && (!locked_key || !lock)) auto prev = evicting.exchange(true, std::memory_order_relaxed);
throw Exception(ErrorCodes::LOGICAL_ERROR, chassert(!prev);
"Setting evicting state to `true` can be done only under lock"); UNUSED(prev);
}
chassert(evicting.load() != evicting_); void resetEvictingFlag() const
evicting.store(evicting_); {
auto prev = evicting.exchange(false, std::memory_order_relaxed);
chassert(prev);
UNUSED(prev);
} }
private: private:
@ -189,12 +193,9 @@ public:
protected: protected:
IFileCachePriority(size_t max_size_, size_t max_elements_); IFileCachePriority(size_t max_size_, size_t max_elements_);
virtual void holdImpl( virtual void holdImpl(size_t /* size */, size_t /* elements */, const CachePriorityGuard::Lock &) {}
size_t size,
size_t elements,
const CachePriorityGuard::Lock & lock);
virtual void releaseImpl(size_t size, size_t elements); virtual void releaseImpl(size_t /* size */, size_t /* elements */) {}
size_t max_size = 0; size_t max_size = 0;
size_t max_elements = 0; size_t max_elements = 0;

View File

@ -50,12 +50,20 @@ struct FileSegmentMetadata : private boost::noncopyable
return iterator->getEntry()->isEvicting(lock); return iterator->getEntry()->isEvicting(lock);
} }
void setEvicting(bool evicting, const LockedKey * locked_key, const CachePriorityGuard::Lock * lock) const void setEvictingFlag(const LockedKey & locked_key, const CachePriorityGuard::Lock & lock) const
{ {
auto iterator = getQueueIterator(); auto iterator = getQueueIterator();
if (!iterator) if (!iterator)
throw Exception(ErrorCodes::LOGICAL_ERROR, "Iterator is not set"); throw Exception(ErrorCodes::LOGICAL_ERROR, "Iterator is not set");
iterator->getEntry()->setEvicting(evicting, locked_key, lock); iterator->getEntry()->setEvictingFlag(locked_key, lock);
}
void resetEvictingFlag() const
{
auto iterator = getQueueIterator();
if (!iterator)
throw Exception(ErrorCodes::LOGICAL_ERROR, "Iterator is not set");
iterator->getEntry()->resetEvictingFlag();
} }
Priority::IteratorPtr getQueueIterator() const { return file_segment->getQueueIterator(); } Priority::IteratorPtr getQueueIterator() const { return file_segment->getQueueIterator(); }