Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize merge tracker and add merge metrics #4350

Merged
merged 18 commits into from
Aug 2, 2022
Merged

Optimize merge tracker and add merge metrics #4350

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a383d7c
Optimize merge tracker and add merge metrics
g11tech Jul 29, 2022
b92f6f9
early return if genesis is merge block
g11tech Jul 29, 2022
9b5e0e7
add metric for latest block time
g11tech Jul 29, 2022
51a74fc
grafana dashboard
g11tech Jul 29, 2022
37f82e5
cleanup merge update
g11tech Jul 29, 2022
3277894
expanded/compressed merge logging
g11tech Jul 29, 2022
239f22d
fix type check in tests
g11tech Jul 29, 2022
7e84d92
improve logging
g11tech Jul 30, 2022
2f8204a
expanded merge at 12hrs
g11tech Jul 30, 2022
59a4e21
Review PR
dapplion Aug 1, 2022
a153c63
Fix tests
dapplion Aug 1, 2022
97bc0a1
Update prettyTimeDiffSec test
dapplion Aug 1, 2022
57a8fbf
merge time and dashboard fixes
g11tech Aug 1, 2022
fbcf640
fix the comment
g11tech Aug 1, 2022
c5ea976
change timeseries input to seconds from ms
g11tech Aug 1, 2022
e0c642e
merge found fix
g11tech Aug 2, 2022
6c5cd12
change last block timestamp collection to seconds
g11tech Aug 2, 2022
ba902cc
Simplify Eth1MergeBlockTracker statuses
dapplion Aug 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 25 additions & 48 deletions dashboards/lodestar_general.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4866,23 +4866,19 @@
"options": {
"0": {
"index": 0,
"text": "NOT_SET"
"text": "STOPPED"
},
"1": {
"index": 1,
"text": "PRE_MERGE"
"text": "SEARCHING"
},
"2": {
"index": 2,
"text": "SEARCHING"
"text": "FOUND"
},
"3": {
"index": 3,
"text": "FOUND"
},
"4": {
"index": 4,
"text": "POST_MERGE"
"text": "MERGE_COMPLETE"
}
},
"type": "value"
Expand Down Expand Up @@ -4981,17 +4977,17 @@
"values": false
},
"text": {},
"textMode": "name"
"textMode": "auto"
},
"pluginVersion": "8.0.6",
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_merge_status",
"expr": "lodestar_eth1_merge_ttd",
"format": "time_series",
"instant": true,
"instant": false,
"interval": "",
"legendFormat": "{{terminalTotalDifficulty}}",
"legendFormat": "",
"refId": "A"
}
],
Expand Down Expand Up @@ -5055,25 +5051,7 @@
"datasource": null,
"fieldConfig": {
"defaults": {
"mappings": [
{
"options": {
"0": {
"index": 0,
"text": "NOT_STARTED"
},
"1": {
"index": 1,
"text": "SEARCHING"
},
"2": {
"index": 2,
"text": "FOUND"
}
},
"type": "value"
}
],
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
Expand Down Expand Up @@ -5113,14 +5091,14 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_merge_block_status",
"expr": "lodestar_eth1_parent_blocks_fetched_total",
"hide": false,
"interval": "",
"legendFormat": "",
"refId": "A"
}
],
"title": "Terminal Block",
"title": "Parents fetched",
"type": "stat"
},
{
Expand Down Expand Up @@ -5258,8 +5236,7 @@
"value": null
}
]
},
"unit": "s"
}
},
"overrides": []
},
Expand Down Expand Up @@ -5289,14 +5266,14 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_time_left_for_merge",
"expr": "lodestar_eth1_merge_td_factor",
"hide": false,
"interval": "",
"legendFormat": "latest block ttd",
"legendFormat": "td factor",
"refId": "A"
}
],
"title": "Time to merge",
"title": "TD factor",
"type": "stat"
},
{
Expand Down Expand Up @@ -5396,7 +5373,7 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_latest_block_time",
"expr": "lodestar_eth1_latest_block_timestamp",
"hide": false,
"interval": "",
"legendFormat": "latest block ttd",
Expand Down Expand Up @@ -5449,14 +5426,14 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_merge_last_searched_block_number",
"expr": "lodestar_eth1_get_terminal_pow_block_promise_cache_hit_total",
"hide": false,
"interval": "",
"legendFormat": "latest block ttd",
"legendFormat": "cache hits",
"refId": "A"
}
],
"title": "Last Searched Num",
"title": "Promise cache hits",
"type": "stat"
},
{
Expand Down Expand Up @@ -5678,14 +5655,14 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_merge_last_searched_block_number",
"expr": "rate(lodestar_eth1_poll_merge_block_errors_total[$__rate_interval])",
"hide": false,
"interval": "",
"legendFormat": "last searched block",
"legendFormat": "polling errors",
"refId": "A"
}
],
"title": "Last Searched block Number",
"title": "Error rate",
"type": "timeseries"
},
{
Expand Down Expand Up @@ -5764,10 +5741,10 @@
"targets": [
{
"exemplar": false,
"expr": "lodestar_eth1_merge_last_searched_block_td",
"expr": "lodestar_eth1_latest_block_ttd",
"hide": false,
"interval": "",
"legendFormat": "last searched block td",
"legendFormat": "lastest block td",
"refId": "A"
},
{
Expand All @@ -5779,7 +5756,7 @@
"refId": "B"
}
],
"title": "Last Searched block",
"title": "Latest TD vs TTD",
"type": "timeseries"
}
],
Expand Down
4 changes: 2 additions & 2 deletions packages/beacon-node/src/eth1/eth1MergeBlockTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class Eth1MergeBlockTracker {
// Set latestBlock stats
metrics.eth1.eth1LatestBlockNumber.set(this.latestEth1Block.number);
metrics.eth1.eth1LatestBlockTD.set(Number(this.latestEth1Block.totalDifficulty / this.safeTDFactor));
metrics.eth1.eth1LatestBlockTimestamp.set(this.latestEth1Block.timestamp);
metrics.eth1.eth1LatestBlockTimestamp.set(this.latestEth1Block.timestamp * 1000);
Copy link
Contributor

@dapplion dapplion Aug 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Prometheus metrics always deal with IS units, so in case of time it must be seconds. See https://prometheus.io/docs/practices/naming/#metric-names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved collection in seconds, and update the dashboard expression (*1000) which displays with unit "From now" which seems to be using ms.

}
});
}
Expand Down Expand Up @@ -135,7 +135,7 @@ export class Eth1MergeBlockTracker {
return {
ttdHit: false,
tdFactor: this.safeTDFactor,
tdDiffScaled: Number((this.latestEth1Block.totalDifficulty / this.safeTDFactor) as bigint),
tdDiffScaled: Number((tdDiff / this.safeTDFactor) as bigint),
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! 🙏

ttd: this.config.TERMINAL_TOTAL_DIFFICULTY,
td: this.latestEth1Block.totalDifficulty,
timestamp: this.latestEth1Block.timestamp,
Expand Down
6 changes: 4 additions & 2 deletions packages/beacon-node/src/node/notifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ export async function runNodeNotifier(modules: NodeNotifierModules): Promise<voi
// Notifier log lines must be kept at a reasonable max width otherwise it's very hard to read
const tdProgress = chain.eth1.getTDProgress();
if (tdProgress !== null && !tdProgress.ttdHit) {
tdTimeSeries.addPoint(tdProgress.tdDiffScaled, tdProgress.timestamp);
// TimeSeries accept time in Ms while timestamp is in Sec
tdTimeSeries.addPoint(tdProgress.tdDiffScaled, tdProgress.timestamp * 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Or TimeSeries.addPoint can take seconds instead and simplify the consumers since all want sec right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right! we can shift to seconds


const timestampTDD = tdTimeSeries.computeY0Point();
const secToTTD = Math.floor(Date.now() / 1000) - timestampTDD;
// It is possible to get ttd estimate with an error at imminent merge
const secToTTD = Math.max(Math.floor(timestampTDD - Date.now() / 1000), 0);
const timeLeft = isFinite(secToTTD) ? prettyTimeDiffSec(secToTTD) : "?";

logger.info(`TTD in ${timeLeft} current TD ${tdProgress.td} / ${tdProgress.ttd}`);
Expand Down
4 changes: 3 additions & 1 deletion packages/beacon-node/src/util/timeSeries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ export class TimeSeries {
*/
computeY0Point(): number {
const {m, b} = linearRegression(this.points);
return -b / m;
// The X cordinate system has been shifted right by startTimeSec, so return the
// projection in original coordinated system
return -b / m + this.startTimeSec;
}

/** Remove all entries */
Expand Down