Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Trivial] Remove dead code #3753

Closed
wants to merge 9 commits into from
+13 −196
Closed

[Trivial] Remove dead code #3753

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5de5164
Remove dead code
thejohnfreeman Dec 21, 2020
02a615e
[fold] Revert some changes
thejohnfreeman Feb 4, 2021
1effa04
[fold] Remove unused override
thejohnfreeman Feb 5, 2021
3a69aac
[fold] Document unused method
thejohnfreeman Feb 5, 2021
53b81a1
[fold] Remove dead code
thejohnfreeman Feb 5, 2021
8587358
[fold] Merge 1.7.0-rc2
thejohnfreeman Feb 5, 2021
0edb207
Merge 1.8.0-b1
thejohnfreeman Mar 15, 2021
fe39326
Merge 1.8.0-b2
thejohnfreeman Apr 1, 2021
499d1f8
Fix compilation of krb5 dependency with GCC 10
thejohnfreeman Mar 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/ripple/app/main/Application.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ class ApplicationImp : public Application, public RootStoppable, public BasicApp
std::unique_ptr<DatabaseCon> mLedgerDB;
std::unique_ptr<DatabaseCon> mWalletDB;
std::unique_ptr<Overlay> overlay_;
std::vector<std::unique_ptr<Stoppable>> websocketServers_;

boost::asio::signal_set m_signals;

Expand Down
15 changes: 0 additions & 15 deletions src/ripple/nodestore/Backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,6 @@ class Backend
virtual Status
fetch(void const* key, std::shared_ptr<NodeObject>* pObject) = 0;

/** Return `true` if batch fetches are optimized. */
virtual bool
canFetchBatch() = 0;

/** Fetch a batch synchronously. */
virtual std::pair<std::vector<std::shared_ptr<NodeObject>>, Status>
fetchBatch(std::vector<uint256 const*> const& hashes) = 0;
Expand Down Expand Up @@ -135,23 +131,12 @@ class Backend
virtual void
setDeletePath() = 0;

/** Perform consistency checks on database. */
virtual void
verify() = 0;

/** Returns the number of file descriptors the backend expects to need. */
virtual int
fdRequired() const = 0;

virtual Counters const&
counters() const = 0;

/** Returns true if the backend uses permanent storage. */
bool
backed() const
{
return fdRequired();
}
};

} // namespace NodeStore
Expand Down
10 changes: 0 additions & 10 deletions src/ripple/nodestore/Manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,6 @@ class Manager
beast::Journal journal) = 0;
};

//------------------------------------------------------------------------------

/** Create a Backend. */
std::unique_ptr<Backend>
make_Backend(
Section const& config,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal);

} // namespace NodeStore
} // namespace ripple

Expand Down
11 changes: 0 additions & 11 deletions src/ripple/nodestore/backend/CassandraFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,12 +578,6 @@ class CassandraBackend : public Backend
return ok;
}

bool
canFetchBatch() override
{
return true;
}

struct ReadCallbackData
{
CassandraBackend& backend;
Expand Down Expand Up @@ -815,11 +809,6 @@ class CassandraBackend : public Backend
{
}

void
verify() override
{
}

int
fdRequired() const override
{
Expand Down
15 changes: 1 addition & 14 deletions src/ripple/nodestore/backend/MemoryFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ class MemoryBackend : public Backend
MemoryBackend(
size_t keyBytes,
Section const& keyValues,
Scheduler& scheduler,
beast::Journal journal)
: name_(get<std::string>(keyValues, "path")), journal_(journal)
{
Expand Down Expand Up @@ -147,12 +146,6 @@ class MemoryBackend : public Backend
return ok;
}

bool
canFetchBatch() override
{
return false;
}

std::pair<std::vector<std::shared_ptr<NodeObject>>, Status>
fetchBatch(std::vector<uint256 const*> const& hashes) override
{
Expand Down Expand Up @@ -210,11 +203,6 @@ class MemoryBackend : public Backend
{
}

void
verify() override
{
}

int
fdRequired() const override
{
Expand Down Expand Up @@ -255,8 +243,7 @@ MemoryFactory::createInstance(
Scheduler& scheduler,
beast::Journal journal)
{
return std::make_unique<MemoryBackend>(
keyBytes, keyValues, scheduler, journal);
return std::make_unique<MemoryBackend>(keyBytes, keyValues, journal);
}

} // namespace NodeStore
Expand Down
25 changes: 0 additions & 25 deletions src/ripple/nodestore/backend/NuDBFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,6 @@ class NuDBBackend : public Backend
return status;
}

bool
canFetchBatch() override
{
return true;
}

std::pair<std::vector<std::shared_ptr<NodeObject>>, Status>
fetchBatch(std::vector<uint256 const*> const& hashes) override
{
Expand Down Expand Up @@ -304,25 +298,6 @@ class NuDBBackend : public Backend
deletePath_ = true;
}

void
verify() override
{
auto const dp = db_.dat_path();
auto const kp = db_.key_path();
auto const lp = db_.log_path();
nudb::error_code ec;
db_.close(ec);
if (ec)
Throw<nudb::system_error>(ec);
nudb::verify_info vi;
nudb::verify<nudb::xxhasher>(vi, dp, kp, 0, nudb::no_progress{}, ec);
if (ec)
Throw<nudb::system_error>(ec);
db_.open(dp, kp, lp, ec);
if (ec)
Throw<nudb::system_error>(ec);
}

int
fdRequired() const override
{
Expand Down
11 changes: 0 additions & 11 deletions src/ripple/nodestore/backend/NullFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ class NullBackend : public Backend
return notFound;
}

bool
canFetchBatch() override
{
return false;
}

std::pair<std::vector<std::shared_ptr<NodeObject>>, Status>
fetchBatch(std::vector<uint256 const*> const& hashes) override
{
Expand Down Expand Up @@ -103,11 +97,6 @@ class NullBackend : public Backend
{
}

void
verify() override
{
}

/** Returns the number of file descriptors the backend expects to need */
int
fdRequired() const override
Expand Down
13 changes: 0 additions & 13 deletions src/ripple/nodestore/backend/RocksDBFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ class RocksDBBackend : public Backend, public BatchWriter::Callback
public:
beast::Journal m_journal;
size_t const m_keyBytes;
Scheduler& m_scheduler;
BatchWriter m_batch;
std::string m_name;
std::unique_ptr<rocksdb::DB> m_db;
Expand All @@ -104,7 +103,6 @@ class RocksDBBackend : public Backend, public BatchWriter::Callback
: m_deletePath(false)
, m_journal(journal)
, m_keyBytes(keyBytes)
, m_scheduler(scheduler)
, m_batch(*this, scheduler)
{
if (!get_if_exists(keyValues, "path", m_name))
Expand Down Expand Up @@ -305,12 +303,6 @@ class RocksDBBackend : public Backend, public BatchWriter::Callback
return status;
}

bool
canFetchBatch() override
{
return false;
}

std::pair<std::vector<std::shared_ptr<NodeObject>>, Status>
fetchBatch(std::vector<uint256 const*> const& hashes) override
{
Expand Down Expand Up @@ -425,11 +417,6 @@ class RocksDBBackend : public Backend, public BatchWriter::Callback
storeBatch(batch);
}

void
verify() override
{
}

/** Returns the number of file descriptors the backend expects to need */
int
fdRequired() const override
Expand Down
13 changes: 0 additions & 13 deletions src/ripple/nodestore/impl/DatabaseRotatingImp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,6 @@ class DatabaseRotatingImp : public DatabaseRotating
std::shared_ptr<Backend> archiveBackend_;
mutable std::mutex mutex_;

struct Backends
{
std::shared_ptr<Backend> const& writableBackend;
std::shared_ptr<Backend> const& archiveBackend;
};

Backends
getBackends() const
{
std::lock_guard lock(mutex_);
return Backends{writableBackend_, archiveBackend_};
}

std::shared_ptr<NodeObject>
fetchNodeObject(
uint256 const& hash,
Expand Down
11 changes: 0 additions & 11 deletions src/ripple/nodestore/impl/Shard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,17 +583,6 @@ Shard::finalize(
{
state_ = finalizing;

/*
TODO MP
A lock is required when calling the NuDB verify function. Because
this can be a time consuming process, the server may desync.
Until this function is modified to work on an open database, we
are unable to use it from rippled.

// Verify backend integrity
backend_->verify();
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check with @miguelportilla before removing this and the verify implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am okay removing this commented out code. Although the backend verify function isn't used, it could be useful at some point. For instance, the above situation can be addressed by calling verify on the new deterministic backend. I could also see a server startup switch to have the backend be verified at startup.


// Check if a final key has been stored
if (std::shared_ptr<NodeObject> nodeObject;
backend_->fetch(finalKey.data(), &nodeObject) == Status::ok)
Expand Down
5 changes: 1 addition & 4 deletions src/ripple/overlay/impl/OverlayImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ OverlayImpl::Timer::on_timer(error_code ec)
overlay_.m_peerFinder->once_per_second();
overlay_.sendEndpoints();
overlay_.autoConnect();

if ((overlay_.timer_count_ % Tuning::checkIdlePeers) == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this deleted? This was added so that we don't check for idle peers too frequently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timer_count_ is initialized to 0 and never updated. It is always 0, and this condition is always true.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, right! it's a bug then! it should be incremented.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see what happened. it used to be this:

if ((++overlay_.timer_count_ % Tuning::checkSeconds) == 0)
    overlay_.check();

if ((overlay_.timer_count_ % Tuning::checkIdlePeers) == 0)
    overlay_.deleteIdlePeers();

but then two top lines were deleted in one of the unrelated commits.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like the right PR to fix that bug, but I wouldn't remove the code either. Let's just keep the code for now and open an issue so we don't lose track of this. @gregtatcam can you open an issue?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened the issue #3754.

overlay_.deleteIdlePeers();
overlay_.deleteIdlePeers();

timer_.expires_from_now(std::chrono::seconds(1));
timer_.async_wait(overlay_.strand_.wrap(std::bind(
Expand Down Expand Up @@ -138,7 +136,6 @@ OverlayImpl::OverlayImpl(
collector))
, m_resolver(resolver)
, next_id_(1)
, timer_count_(0)
, slots_(app, *this)
, m_stats(
std::bind(&OverlayImpl::collect_metrics, this),
Expand Down
1 change: 0 additions & 1 deletion src/ripple/overlay/impl/OverlayImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ class OverlayImpl : public Overlay, public reduce_relay::SquelchHandler
hash_map<Peer::id_t, std::weak_ptr<PeerImp>> ids_;
Resolver& m_resolver;
std::atomic<Peer::id_t> next_id_;
int timer_count_;
std::atomic<uint64_t> jqTransOverflow_{0};
std::atomic<uint64_t> peerDisconnects_{0};
std::atomic<uint64_t> peerDisconnectsCharges_{0};
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/peerfinder/impl/StoreSqdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

#include <ripple/basics/contract.h>
#include <ripple/core/SociDB.h>
#include <ripple/peerfinder/impl/Store.h>

#include <boost/optional.hpp>

namespace ripple {
Expand Down