go/control: Show last consensus height seen by block history #6013

martintomazic · 2025-01-27T23:28:43Z

What & Why

How to test

Delete data/runtime/[runtimeID] (this way we trigger reindex)
oasis-node control status -a unix:path/to/data/internal.sock and observe whilst history reindex is happening, consensus height should be increasing e.g.:

      "storage": {
        "status": "initializing",
        "last_finalized_round": 18446744073709551615
      },
      "history": {
        "last_round": 8911898,
        "last_height": 25095662
      },

netlify · 2025-01-27T23:29:12Z

✅ Deploy Preview for oasisprotocol-oasis-core canceled.

Name	Link
🔨 Latest commit	`ff6fd84`
🔍 Latest deploy log	https://app.netlify.com/sites/oasisprotocol-oasis-core/deploys/679ff518ed68a20008936982

martintomazic · 2025-01-27T23:29:31Z

go/oasis-node/cmd/node/node_control.go

+		// Fetch last consensus height seen by block history.
+		height, err := rt.History().LastConsensusHeight()


it would be useful for oasis-node to report the current reindex round

Notice I report consensus height instead of runtime round. I would argue this is also more appropriate, since reindex iterates over the block height...

Can we show the last height and the last round?
Would percent of indexed blocks help the node operators?

I agree, all of these would be useful.

Can we show the last height and the last round?

I have added those. Have a bit of a mixed feelings about percentage...

Do we want to add it to consensus sync and storage syncing as well (to be consistent)?

Should we only show it if during reindex/syncing?

The value is not deterministic so we cannot expose it as status, [reindex/syncing]_progress: 90.1% looks good?

Do we want to add it to consensus sync and storage syncing as well (to be consistent)?

I said it would be nice to have percentages, so that you know how far are you.

Should we only show it if during reindex/syncing?

Probably no. If we could show 100%, that would mean that the syncing is not in progress.

The value is not deterministic so we cannot expose it as status

Not sure what you are trying to say here.

martintomazic · 2025-01-27T23:32:53Z

go/control/api/api.go

+	// BlockHistoryLastConsensusHeight is last consensus height seen by block history.
+	BlockHistoryLastConsensusHeight int64 `json:"block_history_last_consensus_height"`
+


so that the node operator could estimate when the reindex will finish.

I am not sure current node control status is helpful unless node operator is very familiar with oasis?

Hackish alternative may be to only show it if during "history reindex" status right below it, giving more context to the operator:

diff --git a/go/worker/common/api/api.go b/go/worker/common/api/api.go index 41e0a46ff..0e050b421 100644 --- a/go/worker/common/api/api.go +++ b/go/worker/common/api/api.go @@ -107,6 +107,9 @@ type Status struct { // Status is a concise status of the committee node. Status StatusState `json:"status"` + // HistoryReindexHeight is last consensus height seen by block history. + HistoryReindexHeight *int64 `json:"history_reindex_height,omitempty"` + // ActiveVersion is the currently active version. ActiveVersion *version.Version `json:"active_version"` diff --git a/go/worker/common/committee/node.go b/go/worker/common/committee/node.go index 9f06bcd15..668e43365 100644 --- a/go/worker/common/committee/node.go +++ b/go/worker/common/committee/node.go @@ -275,6 +275,20 @@ func (n *Node) GetStatus() (*api.Status, error) { var status api.Status status.Status = n.getStatusStateLocked() + // Include last consensus height if during history reindex. + if status.Status == api.StatusStateWaitingHistoryReindex { + height, err := n.Runtime.History().LastConsensusHeight() + if err != nil { + n.logger.Warn("failed to retrieve last consensus height seen by block history", + "err", err, + "id", n.Runtime.ID(), + ) + } else { + status.HistoryReindexHeight = &height + + } + }

I am not sure current node control status is helpful unless node operator is very familiar with oasis?

The CLI can provide a nicer computed overlay from this information. But it would be good if the node could estimate the average reindex speed and then an ETA as the range of blocks to reindex is known.

But it would be good if the node could estimate the average reindex speed and then an ETA as the range of blocks to reindex is known.

I would prefer to tackle ETA in another PR here is why:

consensus reindex >= storage sync >> history reindex (time needed for my testnet node to sync from start)

ETA also depends if you are doing reindex during consensus sync or if you start with already synced node. Need to do some more experiments just to be sure...

go/oasis-node/cmd/node/node_control.go

peternose · 2025-01-27T23:46:00Z

go/control/api/api.go

@@ -189,6 +189,9 @@ type RuntimeStatus struct {
 	// Storage contains the storage worker status in case this node is a storage node.
 	Storage *storageWorker.Status `json:"storage,omitempty"`

+	// BlockHistoryLastConsensusHeight is last consensus height seen by block history.
+	BlockHistoryLastConsensusHeight int64 `json:"block_history_last_consensus_height"`


Quite long field name. I would try to find a shorter name or move it under history.
Should this field be grouped with LastRetainedRound?

Yes, we should shorten it.

Now that I think the cleanest way would probably be to show last round and height as seen by the block history right below storage field. I am thinking about exposing this status from the roothash/api/BlockHistory interface (see):

"latest_round": 10160924, "latest_hash": "85867c19bd6ad43521432e187962e3bb19a9731c79cd64f195821bfd90bd7b24", "latest_time": "2025-01-31T11:08:05+01:00", "latest_state_root": { "ns": "000000000000000000000000000000000000000000000000a6d1e3ebf60dff6c", "version": 10160924, "root_type": 1, "hash": "a914f8c8baf05a47757c1d7f7caecebdf614c8b3d1e20aa33d6855ec96a50ba1" }, "genesis_round": 2995927, "genesis_hash": "c9f3ca654531b775d944c85e1f00e76944aaf5de1902acfa082ff76e852dba5e", "last_retained_round": 10145927, "last_retained_hash": "fad686c7eb077ec1a1a5a1ba1c27027a3788b003c94130e78c7dfb25a1d0e8cd", // other fields "storage": { "status": "syncing rounds", "last_finalized_round": 10160923 }, "history": { "last_round": 10160923, "last_height": 25058099 },

Should this field be grouped with LastRetainedRound?

LastRetainedRound is actually earliest (not latest) runtime block that is also synced to the storage.
i. You may still get an error and show the latest retained round from the block history even if you don't have it synced to the storage... See. Is this a bug?

History reindex is actually happening and finishes before storage round syncing so ideally this should be separate grouping.

I am thinking could we move last_retained_round and last_retained_hash to storage? This is breaking obviously. More importantly the storage status is received from the storage committee worker, whilst LastRetainedRound and hash are technically overwritten, directly from the runtime storage (see, described in 1.i). As from the docstring the former is synced + finalized, whilst if reading from the storage directly it is only guaranteed to be synced? Not sure what is the difference (if any)?

History reindex is actually happening and finishes before storage round syncing so ideally this should be separate grouping.

Maybe we show status of both. One part should probably be under storage, other under history. Although I'm not sure if users will understand the difference.

Should this field be grouped with LastRetainedRound?

The code which fetches these fields should also be together.

This is breaking

Breaking for who? I think it is not a problem if we change the status. If node operators depend on it, they can check the changelog and modify their code.

.changelog/5998.feature.md

peternose · 2025-01-27T23:50:25Z

go/oasis-node/cmd/node/node_control.go

+		// Fetch last consensus height seen by block history.
+		height, err := rt.History().LastConsensusHeight()


Can we show the last height and the last round?
Would percent of indexed blocks help the node operators?

go/oasis-node/cmd/node/node_control.go

codecov · 2025-01-28T00:06:33Z

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 64.64%. Comparing base (87c6072) to head (1368dfc).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
go/oasis-node/cmd/node/node_control.go	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6013      +/-   ##
==========================================
- Coverage   65.12%   64.64%   -0.49%     
==========================================
  Files         631      631              
  Lines       64419    64427       +8     
==========================================
- Hits        41956    41647     -309     
- Misses      17549    17867     +318     
+ Partials     4914     4913       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

martintomazic · 2025-02-02T22:53:52Z

go/control/api/api.go

+// HistoryStatus is the status as observed by the block history.
+type HistoryStatus struct {
+	// LastRound is the round of the latest block.
+	LastRound uint64 `json:"last_round"`
+	// LastHeight is the consensus height of the latest block.
+	LastHeight int64 `json:"last_height"`
+}
+


Do we also want a status like we have for storage? e.g. initializing, reindexing blocks, syncing blocks?

If so this logic would have to be moved to consensus roothash, or we add a method to query it. Albeit, feels weird adding this functionality to roothash/api/Backend...?

Do we also want a status like we have for storage? e.g. initializing, reindexing blocks, syncing blocks?

Yes, I would like to have percentages (and maybe optional status).

From you example, I would like to have at least 3 fields, two that say which range is reindexBlocks working on (800 - 1000), and percentages of reindexed blocks (30/200 = 15%). When this is done, the node will see that too many blocks are still to be indexed, so it will update status by updating range (1000 - 1100) and percentage (0/100 = 0%).

Having field LastRound might be a problem when you start doing things in parallel, as some lower rounds might not be processed yet. Range might be better.

peternose · 2025-02-03T12:59:21Z

.changelog/5998.feature.md

@@ -0,0 +1,8 @@
+go/oasis_node: Add runtime block history status message


Suggested change

go/oasis_node: Add runtime block history status message

go/oasis-node: Add runtime block history status message

peternose · 2025-02-03T13:00:22Z

.changelog/5998.feature.md

+go/oasis_node: Add runtime block history status message
+
+A new field `history` has been added to the `oasis-node control status`
+output under the runtime status section. This field displays the latest round


Suggested change

output under the runtime status section. This field displays the latest round

output under the runtime status section. This field displays the round

peternose · 2025-02-03T13:24:34Z

go/roothash/api/history.go

@@ -48,6 +48,8 @@ type BlockHistory interface {

 	// LastConsensusHeight returns the last consensus height which was seen
 	// by block history.
+	//
+	// This method can return height for block not yet synced to storage.


This whole comment is a bit confusing, as the first part says seen while the second says that the block (which one, consensus or runtime?) doesn't need to be synced.

Can we change to

Suggested change

// This method can return height for block not yet synced to storage.

// LastConsensusHeight returns the consensus height of the last committed block.

peternose · 2025-02-03T13:25:31Z

go/roothash/api/history.go

@@ -57,6 +59,7 @@ type BlockHistory interface {
 	GetCommittedBlock(ctx context.Context, round uint64) (*block.Block, error)

 	// GetBlock returns the block at a specific round.
+	//


You should be consistent and add new line in all places or nowhere.

peternose · 2025-02-03T15:54:58Z

go/oasis-node/cmd/node/node_control.go

+		// Fetch last consensus height seen by block history.
+		height, err := rt.History().LastConsensusHeight()


Do we want to add it to consensus sync and storage syncing as well (to be consistent)?

I said it would be nice to have percentages, so that you know how far are you.

Should we only show it if during reindex/syncing?

Probably no. If we could show 100%, that would mean that the syncing is not in progress.

The value is not deterministic so we cannot expose it as status

Not sure what you are trying to say here.

peternose · 2025-02-03T15:56:14Z

go/control/api/api.go

@@ -189,6 +189,9 @@ type RuntimeStatus struct {
 	// Storage contains the storage worker status in case this node is a storage node.
 	Storage *storageWorker.Status `json:"storage,omitempty"`

+	// BlockHistoryLastConsensusHeight is last consensus height seen by block history.
+	BlockHistoryLastConsensusHeight int64 `json:"block_history_last_consensus_height"`


History reindex is actually happening and finishes before storage round syncing so ideally this should be separate grouping.

Maybe we show status of both. One part should probably be under storage, other under history. Although I'm not sure if users will understand the difference.

Should this field be grouped with LastRetainedRound?

The code which fetches these fields should also be together.

This is breaking

Breaking for who? I think it is not a problem if we change the status. If node operators depend on it, they can check the changelog and modify their code.

peternose · 2025-02-03T15:57:18Z

go/control/api/api.go

+// HistoryStatus is the status as observed by the block history.
+type HistoryStatus struct {
+	// LastRound is the round of the latest block.
+	LastRound uint64 `json:"last_round"`
+	// LastHeight is the consensus height of the latest block.
+	LastHeight int64 `json:"last_height"`
+}
+


Do we also want a status like we have for storage? e.g. initializing, reindexing blocks, syncing blocks?

Yes, I would like to have percentages (and maybe optional status).

From you example, I would like to have at least 3 fields, two that say which range is reindexBlocks working on (800 - 1000), and percentages of reindexed blocks (30/200 = 15%). When this is done, the node will see that too many blocks are still to be indexed, so it will update status by updating range (1000 - 1100) and percentage (0/100 = 0%).

Having field LastRound might be a problem when you start doing things in parallel, as some lower rounds might not be processed yet. Range might be better.

martintomazic commented Jan 27, 2025

View reviewed changes

martintomazic marked this pull request as ready for review January 27, 2025 23:34

martintomazic requested review from kostko, peterjgilbert, pro-wh, ptrus and peternose as code owners January 27, 2025 23:34

martintomazic force-pushed the martin/feature/show-history-reindex-round branch from 47f3130 to 1368dfc Compare January 27, 2025 23:36

peternose reviewed Jan 27, 2025

View reviewed changes

kostko linked an issue Jan 28, 2025 that may be closed by this pull request

control status: Show current history reindex round number #5998

Open

martintomazic added 2 commits February 2, 2025 23:03

go/roothash/api: Improve BlockHistory docstring

dc7fed0

go/control: Show last consensus height seen by block history

ff6fd84

martintomazic force-pushed the martin/feature/show-history-reindex-round branch from 1368dfc to ff6fd84 Compare February 2, 2025 22:43

martintomazic commented Feb 2, 2025

View reviewed changes

martintomazic requested a review from peternose February 3, 2025 10:08

peternose reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go/control: Show last consensus height seen by block history #6013

go/control: Show last consensus height seen by block history #6013

martintomazic commented Jan 27, 2025 •

edited

Loading

netlify bot commented Jan 27, 2025 •

edited

Loading

martintomazic Jan 27, 2025

peternose Jan 27, 2025

kostko Jan 28, 2025

martintomazic Feb 2, 2025

peternose Feb 3, 2025

martintomazic Jan 27, 2025

kostko Jan 28, 2025

martintomazic Feb 2, 2025

peternose Jan 27, 2025

kostko Jan 28, 2025

martintomazic Jan 31, 2025

peternose Feb 3, 2025

peternose Jan 27, 2025

codecov bot commented Jan 28, 2025

martintomazic Feb 2, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

peternose Feb 3, 2025

		// Fetch last consensus height seen by block history.
		height, err := rt.History().LastConsensusHeight()

		// BlockHistoryLastConsensusHeight is last consensus height seen by block history.
		BlockHistoryLastConsensusHeight int64 `json:"block_history_last_consensus_height"`

		@@ -0,0 +1,8 @@
		go/oasis_node: Add runtime block history status message

	go/oasis_node: Add runtime block history status message
	go/oasis-node: Add runtime block history status message

	output under the runtime status section. This field displays the latest round
	output under the runtime status section. This field displays the round

	// This method can return height for block not yet synced to storage.
	// LastConsensusHeight returns the consensus height of the last committed block.

go/control: Show last consensus height seen by block history #6013

Are you sure you want to change the base?

go/control: Show last consensus height seen by block history #6013

Conversation

martintomazic commented Jan 27, 2025 • edited Loading

What & Why

How to test

netlify bot commented Jan 27, 2025 • edited Loading

✅ Deploy Preview for oasisprotocol-oasis-core canceled.

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

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

codecov bot commented Jan 28, 2025

Codecov Report

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

Choose a reason for hiding this comment

martintomazic commented Jan 27, 2025 •

edited

Loading

netlify bot commented Jan 27, 2025 •

edited

Loading