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
      },
  "block_history_last_consensus_height": 18622777,

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

✅ Deploy Preview for oasisprotocol-oasis-core canceled.

Name	Link
🔨 Latest commit	`1368dfc`
🔍 Latest deploy log	https://app.netlify.com/sites/oasisprotocol-oasis-core/deploys/6798187d190ba600089c2036

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.

go/control/api/api.go

peternose · 2025-01-27T23:39:07Z

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

@@ -343,6 +343,17 @@ func (n *Node) getRuntimeStatus(ctx context.Context) (map[common.Namespace]contr
 			}
 		}

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


Suggested change

if err == nil {

if err != nil {

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)?

peternose · 2025-01-27T23:49:36Z

.changelog/5998.feature.md

@@ -0,0 +1 @@
+Show the progress of the history reindex in oasis control status


We usually start changelog and git messages with a prefix, in this case go/oasis_node/cmd or go/oasis_node would be fine.
After the title we could explain which field was added and what it shows. See previous logs for examples.

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?

peternose · 2025-01-27T23:57:23Z

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

+				"err", err,
+				"id", rt.ID(),
+			)
+		} else {


[nit] I'm not a big fan of if/else, as it makes code less readable and the reader normally expects only if part which handles the error. To be in-line with the code above, you could assign directly to the status field (even if there is an error, assuming that the returned value is zero value) or use switch like it is done above. I would use the former, just to be in-line with code above.

Maybe this code could be moved up, where we handle history.

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 commented Jan 27, 2025

View reviewed changes

go/control/api/api.go Show resolved Hide resolved

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

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

1368dfc

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

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

netlify bot commented Jan 27, 2025 •

edited

Loading

martintomazic Jan 27, 2025

peternose Jan 27, 2025

kostko Jan 28, 2025

peternose Jan 27, 2025

peternose Jan 27, 2025

kostko Jan 28, 2025

martintomazic Jan 31, 2025

peternose Jan 27, 2025

peternose Jan 27, 2025

peternose Jan 27, 2025

codecov bot commented Jan 28, 2025

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

		@@ -0,0 +1 @@
		Show the progress of the history reindex in oasis control status

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

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

codecov bot commented Jan 28, 2025

Codecov Report

netlify bot commented Jan 27, 2025 •

edited

Loading