IF: Missing vote logging and Prometheus logging of votes #63

heifner · 2024-04-23T18:19:45Z

Adds info level logging of expected votes not incorporated into blocks. Example:

info  2024-04-23T20:27:31.840 nodeos    producer_plugin.cpp:658       log_missing_votes    ] Block 179:df4235f559ce7978 has no votes from finalizers: ["finalizer #4"]

Adds warn level logging of expected votes not incorporated into a configured finalizer block. Example:

warn  2024-04-23T20:27:31.840 nodeos    producer_plugin.cpp:651       log_missing_votes    ] Local finalizer finalizer #4 did not vote on block 179:df4235f559ce7978

Add Prometheus logging of votes incorporated into blocks. Example:

# HELP nodeos_block_num current block number
# TYPE nodeos_block_num gauge
nodeos_block_num 501
# HELP nodeos_block_votes votes incorporated into a block, -1 weak, 1 strong, 0 no vote
# TYPE nodeos_block_votes gauge
nodeos_block_votes{producer="finalizer #4"} 1
nodeos_block_votes{producer="finalizer #3"} 1
nodeos_block_votes{producer="finalizer #2"} 1
nodeos_block_votes{producer="finalizer #1"} 1

Resolves #8

…nfigured finalizer

libraries/chain/block.cpp

plugins/producer_plugin/producer_plugin.cpp

linh2931 · 2024-04-23T21:29:48Z

plugins/producer_plugin/producer_plugin.cpp

+   void log_missing_votes(const signed_block_ptr& block, const block_id_type& id,
+                          const finalizer_policy_ptr& active_finalizer_policy,
+                          const valid_quorum_certificate& qc) {
+      if (vote_logger.is_enabled(fc::log_level::info)) {


Instead of those two nested ifs, you can return is vote_logger is not enabled or minutes not passed, to reduce two nesting levels. But it is just a style. No need to change.

linh2931 · 2024-04-23T21:32:11Z

libraries/chain/block.cpp

@@ -63,7 +63,40 @@ namespace eosio { namespace chain {
      }

      return results;
+   }
+
+   // Does not validate ordering, assumes validate_and_extract_extensions() has been called in verify_qc_claim()


Is it possible to have an assert to make sure the assumption (pre-condition) is valid?

Yes, I thought about adding an assert, but did not want different behavior. Since assert is compiled out, might as well add it.

I think assert is worthy to add, since now we have one CICD enabled it.

linh2931 · 2024-04-24T11:58:30Z

libraries/chain/block.cpp

@@ -69,6 +69,8 @@ namespace eosio::chain {
   std::optional<block_extension> signed_block::extract_extension(uint16_t extension_id)const {
      using decompose_t = block_extension_types::decompose_t;

+      assert(std::ranges::is_sorted(block_extensions)); // currently all extensions are unique so default compare works


Great. Now it is clear the for loop is correct.

greg7mdp · 2024-04-24T17:42:04Z

plugins/producer_plugin/producer_plugin.cpp

+         if (qc._strong_votes) {
+            add_votes(*qc._strong_votes, m.strong_votes);
+         }
+         if (qc._weak_votes) {
+            add_votes(*qc._weak_votes, m.weak_votes);
+         }
+         if (m.strong_votes.size() + m.weak_votes.size() != active_finalizer_policy->finalizers.size()) {
+            fc::dynamic_bitset not_voted(active_finalizer_policy->finalizers.size());
+            if (qc._strong_votes) {
+               not_voted = *qc._strong_votes;
+            }
+            if (qc._weak_votes) {
+               assert(not_voted.size() == qc._weak_votes->size());
+               not_voted |= *qc._weak_votes;
+            }
+            not_voted.flip();
+            add_votes(not_voted, m.no_votes);
+         }


Suggested change

if (qc._strong_votes) {

add_votes(*qc._strong_votes, m.strong_votes);

}

if (qc._weak_votes) {

add_votes(*qc._weak_votes, m.weak_votes);

}

if (m.strong_votes.size() + m.weak_votes.size() != active_finalizer_policy->finalizers.size()) {

fc::dynamic_bitset not_voted(active_finalizer_policy->finalizers.size());

if (qc._strong_votes) {

not_voted = *qc._strong_votes;

}

if (qc._weak_votes) {

assert(not_voted.size() == qc._weak_votes->size());

not_voted |= *qc._weak_votes;

}

not_voted.flip();

add_votes(not_voted, m.no_votes);

}

fc::dynamic_bitset not_voted(active_finalizer_policy->finalizers.size());

if (qc._strong_votes) {

add_votes(*qc._strong_votes, m.strong_votes);

not_voted |= *qc._strong_votes;

}

if (qc._weak_votes) {

add_votes(*qc._weak_votes, m.weak_votes);

not_voted |= *qc._weak_votes;

}

not_voted.flip();

add_votes(not_voted, m.no_votes);

oh sorry, it is already merged!

The merged version is more efficient in that it doesn't do the not_voted in the common case of no weak votes.

It still does it in the likely case where all the finalizers did not vote, i.e. m.strong_votes.size() != active_finalizer_policy->finalizers.size()

There will be no QC if no votes at all.

If the policy size is 21 finalizers, and we have 16 strong votes, and no weak votes, you still compute the not_voted, right?

Yes. 16 strong + 0 week != 21, so will calculate the not_voted.

ericpassmore · 2024-04-29T23:23:47Z

Note:start
group: IF
category: CHORE
summary: Add info, warn, and Prometheus logging of votes that are not incorporated into block or configured finalizer block.
Note:end

heifner added 5 commits April 23, 2024 12:45

GH-8 Add similar helper functions as block_header for extensions

477f3ae

GH-8 Add accessor for head active finalizer policy

2a18889

GH-8 Info log missing votes in block and warn log missing vote for co…

077248b

…nfigured finalizer

GH-8 Do not log no vote messages if syncing.

38e4547

GH-8 Update help

484d40e

heifner added the OCI Work exclusive to OCI team label Apr 23, 2024

heifner requested review from linh2931 and greg7mdp April 23, 2024 18:20

This was linked to issues Apr 23, 2024

Warn finalizer in logs if they are not finalizing blocks #8

Closed

Test Failure: nodeos_read_terminate_at_block_if_lr_test #38

Closed

heifner removed a link to an issue Apr 23, 2024

Test Failure: nodeos_read_terminate_at_block_if_lr_test #38

Closed

greg7mdp requested changes Apr 23, 2024

View reviewed changes

heifner added 3 commits April 23, 2024 15:31

GH-8 Shortcut out on id > extension_id

914f117

GH-8 Update log messages

56ea22e

GH-8 Use dynamic_bitset for not_voted calculation

96e2f68

greg7mdp approved these changes Apr 23, 2024

View reviewed changes

linh2931 approved these changes Apr 23, 2024

View reviewed changes

GH-8 Add assert that extensions are sorted

e6b8d8b

linh2931 reviewed Apr 24, 2024

View reviewed changes

linh2931 approved these changes Apr 24, 2024

View reviewed changes

heifner merged commit 54812d4 into savanna Apr 24, 2024
36 checks passed

heifner deleted the GH-8-finalizer-log branch April 24, 2024 15:22

greg7mdp reviewed Apr 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IF: Missing vote logging and Prometheus logging of votes #63

IF: Missing vote logging and Prometheus logging of votes #63

heifner commented Apr 23, 2024 •

edited

Loading

linh2931 Apr 23, 2024

linh2931 Apr 23, 2024

heifner Apr 23, 2024

linh2931 Apr 23, 2024

linh2931 Apr 24, 2024

greg7mdp Apr 24, 2024

greg7mdp Apr 24, 2024

heifner Apr 24, 2024

greg7mdp Apr 24, 2024 •

edited

Loading

heifner Apr 24, 2024 •

edited

Loading

greg7mdp Apr 24, 2024 •

edited

Loading

heifner Apr 24, 2024

ericpassmore commented Apr 29, 2024 •

edited

Loading

IF: Missing vote logging and Prometheus logging of votes #63

IF: Missing vote logging and Prometheus logging of votes #63

Conversation

heifner commented Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg7mdp Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

heifner Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

greg7mdp Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericpassmore commented Apr 29, 2024 • edited Loading

heifner commented Apr 23, 2024 •

edited

Loading

greg7mdp Apr 24, 2024 •

edited

Loading

heifner Apr 24, 2024 •

edited

Loading

greg7mdp Apr 24, 2024 •

edited

Loading

ericpassmore commented Apr 29, 2024 •

edited

Loading