Skip to content

Election age statistics#4537

Merged
clemahieu merged 3 commits intonanocurrency:developfrom
RickiNano:election-age-statistics
Apr 9, 2024
Merged

Election age statistics#4537
clemahieu merged 3 commits intonanocurrency:developfrom
RickiNano:election-age-statistics

Conversation

@RickiNano
Copy link
Copy Markdown
Contributor

This PR adds information to the election_statistics RPC call about the maximum and average age of elections in AEC .
This will help identify long running elections.
I have also updated the unit test to create an election

Request:

{
  "action": "election_statistics"
}

Response example:

{
    "normal": "200",
    "hinted": "22",
    "optimistic": "425",
    "total": "647",
    "aec_utilization_percentage": "8.09",
    "max_election_age": "6",
    "average_election_age": "2.46"
}

@qwahzi qwahzi added documentation This item indicates the need for or supplies updated or expanded documentation rpc Changes related to Remote Procedure Calls labels Apr 7, 2024
@qwahzi qwahzi added this to the V27 milestone Apr 7, 2024
auto election_start = election->get_election_start ();
auto age = now - election_start;
total_age += age;
if (election_start < oldest_election_start)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can oldest_election_start = std::min(oldest_election_start, election->election_start) be used instead of branching?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think I can do that if using the getter function. That would probably require election_start to be public which I think is a bad idea. It shouldn't be modified outside the election class

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be compatible with the getter:

oldest_election_start = std::min(oldest_election_start, election->get_election_start())

@clemahieu clemahieu merged commit 1fb3e9f into nanocurrency:develop Apr 9, 2024
node_config.ipc_config.transport_tcp.enabled = true;
node_config.ipc_config.transport_tcp.port = system.get_available_port ();
nano::node_flags node_flags;
node_flags.disable_request_loop = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be nice to explain why you are disabling the request loop.

.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
node1->process_active (send1);
ASSERT_TIMELY (5s, !node1->active.empty ());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line looks strange to me. Maybe there is a race condition between process_active() and start_elections(). The usual way to use start_elections() is:

	ASSERT_TRUE (nano::test::process (*node1, {send1}));
	ASSERT_TRUE (nano::test::start_elections (system, *node1, {send1}));

ASSERT_EQ ("0", response.get<std::string> ("optimistic"));
ASSERT_EQ ("1", response.get<std::string> ("total"));
ASSERT_NE ("0.00", response.get<std::string> ("aec_utilization_percentage"));
ASSERT_NO_THROW (response.get<std::string> ("max_election_age"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These 2 checks could check that the values are within an expected range.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation This item indicates the need for or supplies updated or expanded documentation rpc Changes related to Remote Procedure Calls

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants