Skip to content

Commit

Permalink
Remove default arguments of virtual function in DB::PageStorage and i…
Browse files Browse the repository at this point in the history 
…ts derived class (pingcap#4507)

close pingcap#4482
  • Loading branch information
@Lloyd-Pottiger
Lloyd-Pottiger authored Mar 31, 2022
1 parent 3f0667a commit 362ba87
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 66 deletions.
64 changes: 55 additions & 9 deletions dbms/src/Storages/Page/PageStorage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,26 +179,54 @@ class PageStorage : private boost::noncopyable
// Get some statistics of all living snapshots and the oldest living snapshot.
virtual SnapshotsStatistics getSnapshotsStat() const = 0;

virtual void write(WriteBatch && write_batch, const WriteLimiterPtr & write_limiter = nullptr) = 0;
void write(WriteBatch && write_batch, const WriteLimiterPtr & write_limiter = nullptr)
{
writeImpl(std::move(write_batch), write_limiter);
}

virtual PageEntry getEntry(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot = {}) = 0;
PageEntry getEntry(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot = {})
{
return getEntryImpl(ns_id, page_id, snapshot);
}

virtual Page read(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {}) = 0;
Page read(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {})
{
return readImpl(ns_id, page_id, read_limiter, snapshot);
}

virtual PageMap read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {}) = 0;
PageMap read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {})
{
return readImpl(ns_id, page_ids, read_limiter, snapshot);
}

virtual void read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {}) = 0;
void read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {})
{
readImpl(ns_id, page_ids, handler, read_limiter, snapshot);
}

using FieldIndices = std::vector<size_t>;
using PageReadFields = std::pair<PageId, FieldIndices>;
virtual PageMap read(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {}) = 0;

virtual void traverse(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot = {}) = 0;
PageMap read(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter = nullptr, SnapshotPtr snapshot = {})
{
return readImpl(ns_id, page_fields, read_limiter, snapshot);
}

void traverse(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot = {})
{
traverseImpl(acceptor, snapshot);
}

virtual PageId getNormalPageId(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot = {}) = 0;
PageId getNormalPageId(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot = {})
{
return getNormalPageIdImpl(ns_id, page_id, snapshot);
}

// We may skip the GC to reduce useless reading by default.
virtual bool gc(bool not_skip = false, const WriteLimiterPtr & write_limiter = nullptr, const ReadLimiterPtr & read_limiter = nullptr) = 0;
bool gc(bool not_skip = false, const WriteLimiterPtr & write_limiter = nullptr, const ReadLimiterPtr & read_limiter = nullptr)
{
return gcImpl(not_skip, write_limiter, read_limiter);
}

// Register and unregister external pages GC callbacks
virtual void registerExternalPagesCallbacks(const ExternalPageCallbacks & callbacks) = 0;
Expand All @@ -207,6 +235,24 @@ class PageStorage : private boost::noncopyable
#ifndef DBMS_PUBLIC_GTEST
protected:
#endif
virtual void writeImpl(WriteBatch && write_batch, const WriteLimiterPtr & write_limiter) = 0;

virtual PageEntry getEntryImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) = 0;

virtual Page readImpl(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) = 0;

virtual PageMap readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) = 0;

virtual void readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) = 0;

virtual PageMap readImpl(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) = 0;

virtual void traverseImpl(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot) = 0;

virtual PageId getNormalPageIdImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) = 0;

virtual bool gcImpl(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter) = 0;

String storage_name; // Identify between different Storage
PSDiskDelegatorPtr delegator; // Get paths for storing data
Config config;
Expand Down
20 changes: 10 additions & 10 deletions dbms/src/Storages/Page/V2/PageStorage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ PageId PageStorage::getMaxId(NamespaceId /*ns_id*/)
return versioned_page_entries.getSnapshot("")->version()->maxId();
}

PageId PageStorage::getNormalPageId(NamespaceId /*ns_id*/, PageId page_id, SnapshotPtr snapshot)
PageId PageStorage::getNormalPageIdImpl(NamespaceId /*ns_id*/, PageId page_id, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -372,7 +372,7 @@ PageId PageStorage::getNormalPageId(NamespaceId /*ns_id*/, PageId page_id, Snaps
return is_ref_id ? normal_page_id : page_id;
}

DB::PageEntry PageStorage::getEntry(NamespaceId /*ns_id*/, PageId page_id, SnapshotPtr snapshot)
DB::PageEntry PageStorage::getEntryImpl(NamespaceId /*ns_id*/, PageId page_id, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand Down Expand Up @@ -484,7 +484,7 @@ PageStorage::ReaderPtr PageStorage::getReader(const PageFileIdAndLevel & file_id
return pages_reader;
}

void PageStorage::write(DB::WriteBatch && wb, const WriteLimiterPtr & write_limiter)
void PageStorage::writeImpl(DB::WriteBatch && wb, const WriteLimiterPtr & write_limiter)
{
if (unlikely(wb.empty()))
return;
Expand Down Expand Up @@ -592,7 +592,7 @@ SnapshotsStatistics PageStorage::getSnapshotsStat() const
return versioned_page_entries.getSnapshotsStat();
}

DB::Page PageStorage::read(NamespaceId /*ns_id*/, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
DB::Page PageStorage::readImpl(NamespaceId /*ns_id*/, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -608,7 +608,7 @@ DB::Page PageStorage::read(NamespaceId /*ns_id*/, PageId page_id, const ReadLimi
return file_reader->read(to_read, read_limiter)[page_id];
}

PageMap PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
PageMap PageStorage::readImpl(NamespaceId /*ns_id*/, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand Down Expand Up @@ -651,7 +651,7 @@ PageMap PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageId> & pag
return page_map;
}

void PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
void PageStorage::readImpl(NamespaceId /*ns_id*/, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand Down Expand Up @@ -691,7 +691,7 @@ void PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageId> & page_i
}
}

PageMap PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
PageMap PageStorage::readImpl(NamespaceId /*ns_id*/, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand Down Expand Up @@ -734,7 +734,7 @@ PageMap PageStorage::read(NamespaceId /*ns_id*/, const std::vector<PageReadField
return page_map;
}

void PageStorage::traverse(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot)
void PageStorage::traverseImpl(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -757,7 +757,7 @@ void PageStorage::traverse(const std::function<void(const DB::Page & page)> & ac
for (const auto & p : file_and_pages)
{
// namespace id is not used in V2, so it's value is not important here
auto pages = read(MAX_NAMESPACE_ID, p.second, nullptr, snapshot);
auto pages = readImpl(MAX_NAMESPACE_ID, p.second, nullptr, snapshot);
for (const auto & id_page : pages)
{
acceptor(id_page.second);
Expand Down Expand Up @@ -897,7 +897,7 @@ WriteBatch::SequenceID PageStorage::WritingFilesSnapshot::minPersistedSequence()
return seq;
}

bool PageStorage::gc(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter)
bool PageStorage::gcImpl(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter)
{
// If another thread is running gc, just return;
bool v = false;
Expand Down
40 changes: 20 additions & 20 deletions dbms/src/Storages/Page/V2/PageStorage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class PageStorage : public DB::PageStorage

PageId getMaxId(NamespaceId ns_id) override;

PageId getNormalPageId(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) override;
PageId getNormalPageIdImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) override;

DB::PageStorage::SnapshotPtr getSnapshot(const String & tracing_id) override;

Expand All @@ -107,21 +107,21 @@ class PageStorage : public DB::PageStorage

SnapshotsStatistics getSnapshotsStat() const override;

void write(DB::WriteBatch && wb, const WriteLimiterPtr & write_limiter) override;
void writeImpl(DB::WriteBatch && wb, const WriteLimiterPtr & write_limiter) override;

DB::PageEntry getEntry(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) override;
DB::PageEntry getEntryImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot) override;

DB::Page read(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;
DB::Page readImpl(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;

PageMap read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;
PageMap readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;

void read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;
void readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;

PageMap read(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;
PageMap readImpl(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) override;

void traverse(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot) override;
void traverseImpl(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot) override;

bool gc(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter) override;
bool gcImpl(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter) override;

void registerExternalPagesCallbacks(const ExternalPageCallbacks & callbacks) override;

Expand Down Expand Up @@ -194,17 +194,17 @@ class PageStorage : public DB::PageStorage
#ifndef NDEBUG
// Just for tests, refactor them out later
DB::PageStorage::SnapshotPtr getSnapshot() { return getSnapshot(""); }
void write(DB::WriteBatch && wb) { return write(std::move(wb), nullptr); }
DB::PageEntry getEntry(PageId page_id) { return getEntry(TEST_NAMESPACE_ID, page_id, nullptr); }
DB::PageEntry getEntry(PageId page_id, SnapshotPtr snapshot) { return getEntry(TEST_NAMESPACE_ID, page_id, snapshot); };
DB::Page read(PageId page_id) { return read(TEST_NAMESPACE_ID, page_id, nullptr, nullptr); }
DB::Page read(PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) { return read(TEST_NAMESPACE_ID, page_id, read_limiter, snapshot); }
PageMap read(const std::vector<PageId> & page_ids) { return read(TEST_NAMESPACE_ID, page_ids, nullptr, nullptr); }
PageMap read(const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) { return read(TEST_NAMESPACE_ID, page_ids, read_limiter, snapshot); };
void read(const std::vector<PageId> & page_ids, const PageHandler & handler) { return read(TEST_NAMESPACE_ID, page_ids, handler, nullptr, nullptr); }
PageMap read(const std::vector<PageReadFields> & page_fields) { return read(TEST_NAMESPACE_ID, page_fields, nullptr, nullptr); }
void traverse(const std::function<void(const DB::Page & page)> & acceptor) { return traverse(acceptor, nullptr); }
bool gc() { return gc(false, nullptr, nullptr); }
void write(DB::WriteBatch && wb) { return writeImpl(std::move(wb), nullptr); }
DB::PageEntry getEntry(PageId page_id) { return getEntryImpl(TEST_NAMESPACE_ID, page_id, nullptr); }
DB::PageEntry getEntry(PageId page_id, SnapshotPtr snapshot) { return getEntryImpl(TEST_NAMESPACE_ID, page_id, snapshot); };
DB::Page read(PageId page_id) { return readImpl(TEST_NAMESPACE_ID, page_id, nullptr, nullptr); }
DB::Page read(PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) { return readImpl(TEST_NAMESPACE_ID, page_id, read_limiter, snapshot); }
PageMap read(const std::vector<PageId> & page_ids) { return readImpl(TEST_NAMESPACE_ID, page_ids, nullptr, nullptr); }
PageMap read(const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot) { return readImpl(TEST_NAMESPACE_ID, page_ids, read_limiter, snapshot); };
void read(const std::vector<PageId> & page_ids, const PageHandler & handler) { return readImpl(TEST_NAMESPACE_ID, page_ids, handler, nullptr, nullptr); }
PageMap read(const std::vector<PageReadFields> & page_fields) { return readImpl(TEST_NAMESPACE_ID, page_fields, nullptr, nullptr); }
void traverse(const std::function<void(const DB::Page & page)> & acceptor) { return traverseImpl(acceptor, nullptr); }
bool gc() { return gcImpl(false, nullptr, nullptr); }
#endif

#ifndef DBMS_PUBLIC_GTEST
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Storages/Page/V2/tests/page_storage_ctl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ try
for (Int64 idx = 0; num_gc == -1 || idx < num_gc; ++idx)
{
LOG_FMT_INFO(logger, "Running GC, [round={}] [num_gc={}]", (idx + 1), num_gc);
storage.gc(/*not_skip=*/true, nullptr, nullptr);
storage.gcImpl(/*not_skip=*/true, nullptr, nullptr);
LOG_FMT_INFO(logger, "Run GC done, [round={}] [num_gc={}]", (idx + 1), num_gc);
}
break;
Expand Down
18 changes: 9 additions & 9 deletions dbms/src/Storages/Page/V3/PageStorageImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ PageId PageStorageImpl::getMaxId(NamespaceId ns_id)
return page_directory->getMaxId(ns_id);
}

PageId PageStorageImpl::getNormalPageId(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot)
PageId PageStorageImpl::getNormalPageIdImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -84,7 +84,7 @@ SnapshotsStatistics PageStorageImpl::getSnapshotsStat() const
return page_directory->getSnapshotsStat();
}

void PageStorageImpl::write(DB::WriteBatch && write_batch, const WriteLimiterPtr & write_limiter)
void PageStorageImpl::writeImpl(DB::WriteBatch && write_batch, const WriteLimiterPtr & write_limiter)
{
if (unlikely(write_batch.empty()))
return;
Expand All @@ -94,7 +94,7 @@ void PageStorageImpl::write(DB::WriteBatch && write_batch, const WriteLimiterPtr
page_directory->apply(std::move(edit), write_limiter);
}

DB::PageEntry PageStorageImpl::getEntry(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot)
DB::PageEntry PageStorageImpl::getEntryImpl(NamespaceId ns_id, PageId page_id, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand Down Expand Up @@ -124,7 +124,7 @@ DB::PageEntry PageStorageImpl::getEntry(NamespaceId ns_id, PageId page_id, Snaps
}
}

DB::Page PageStorageImpl::read(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
DB::Page PageStorageImpl::readImpl(NamespaceId ns_id, PageId page_id, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -135,7 +135,7 @@ DB::Page PageStorageImpl::read(NamespaceId ns_id, PageId page_id, const ReadLimi
return blob_store.read(page_entry, read_limiter);
}

PageMap PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
PageMap PageStorageImpl::readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -149,7 +149,7 @@ PageMap PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageId> & pag
return blob_store.read(page_entries, read_limiter);
}

void PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
void PageStorageImpl::readImpl(NamespaceId ns_id, const std::vector<PageId> & page_ids, const PageHandler & handler, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -163,7 +163,7 @@ void PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageId> & page_i
blob_store.read(page_entries, handler, read_limiter);
}

PageMap PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
PageMap PageStorageImpl::readImpl(NamespaceId ns_id, const std::vector<PageReadFields> & page_fields, const ReadLimiterPtr & read_limiter, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -182,7 +182,7 @@ PageMap PageStorageImpl::read(NamespaceId ns_id, const std::vector<PageReadField
return blob_store.read(read_infos, read_limiter);
}

void PageStorageImpl::traverse(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot)
void PageStorageImpl::traverseImpl(const std::function<void(const DB::Page & page)> & acceptor, SnapshotPtr snapshot)
{
if (!snapshot)
{
Expand All @@ -198,7 +198,7 @@ void PageStorageImpl::traverse(const std::function<void(const DB::Page & page)>
}
}

bool PageStorageImpl::gc(bool /*not_skip*/, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter)
bool PageStorageImpl::gcImpl(bool /*not_skip*/, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter)
{
// If another thread is running gc, just return;
bool v = false;
Expand Down
Loading

0 comments on commit 362ba87

Please sign in to comment.