Skip to content

Rep crawler overhaul#4449

Merged
dsiganos merged 28 commits intonanocurrency:developfrom
pwojcikdev:repcrawler-overhaul-2
Mar 8, 2024
Merged

Rep crawler overhaul#4449
dsiganos merged 28 commits intonanocurrency:developfrom
pwojcikdev:repcrawler-overhaul-2

Conversation

@pwojcikdev
Copy link
Copy Markdown
Contributor

@pwojcikdev pwojcikdev commented Feb 25, 2024

Initial motivation for this PR was providing better control over rep crawler component lifetime and running its internal logic on a separate dedicated thread. In the process of working on it I made many smaller improvements that should make the logic clearer and less error prone.

Testing results provided by @gr0vity-dev show that performance is on par with current develop.
image

@pwojcikdev pwojcikdev force-pushed the repcrawler-overhaul-2 branch from 9c10108 to 5cc8256 Compare February 28, 2024 12:42
@pwojcikdev
Copy link
Copy Markdown
Contributor Author

I made a few adjustments based on recent network stress test. It should make repcrawler a bit better at finding representatives when vote requests are unreliable.

develop
Screenshot 2024-02-28 at 13 48 37
rep-crawler-overhaul
Screenshot 2024-02-28 at 13 48 30

@pwojcikdev pwojcikdev force-pushed the repcrawler-overhaul-2 branch from b2db447 to 5cc8256 Compare February 28, 2024 22:04
@pwojcikdev
Copy link
Copy Markdown
Contributor Author

With the latest commit running on live network, rep crawler averages about ~6 requests per second with response rate of about 25%. It is able to keep a stable peered stake.
Screenshot 2024-03-05 at 14 23 09

@dsiganos
Copy link
Copy Markdown
Contributor

dsiganos commented Mar 6, 2024

I am trying to understand how multiple reps inside one node would be dealt.
So I created this test case and it fails:

// Test that nodes can track PRs when multiple PRs are inside one node
TEST (rep_crawler, two_reps_one_node)
{
	nano::test::system system;
	auto & node1 = *system.add_node ();
	auto & node2 = *system.add_node ();

	// create a second PR account
	nano::keypair pr1 = nano::test::setup_rep (system, node1, node1.balance (nano::dev::genesis_key.pub) / 10);

	ASSERT_EQ (0, node2.rep_crawler.representative_count ());

	// enable the two PRs in node1
	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
	system.wallet (0)->insert_adhoc (pr1.prv);

	ASSERT_TIMELY_EQ (5s, node2.rep_crawler.representative_count (), 2);
	auto reps = node2.rep_crawler.representatives ();
	ASSERT_EQ (2, reps.size ());
}

@pwojcikdev
Copy link
Copy Markdown
Contributor Author

@dsiganos Good catch, feel free to commit this testcase. I'll look into solving it.

@qwahzi qwahzi added this to the V27 milestone Mar 6, 2024
dsiganos added 6 commits March 7, 2024 13:47
Do not delete the query when a confirm_ack is received with the right vote.
We might get more replies and we have no way of knowing how many will come.
Just let the query timeout.
# Conflicts:
#	nano/lib/stats_enums.hpp
#	nano/lib/thread_roles.cpp
#	nano/lib/thread_roles.hpp
@dsiganos dsiganos merged commit d5bf9a2 into nanocurrency:develop Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged / V27.0

Development

Successfully merging this pull request may close these issues.

3 participants