-
Notifications
You must be signed in to change notification settings - Fork 714
Refactor/Allow metrics before block processing #10076
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.
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
Changes from 9 commits
07959e9
0d94338
b861520
b3f6ccd
44b960b
9ce2ade
f716a5b
4f64d23
b96fc7e
ed93611
762994b
f3cca46
7af3353
6fa07b6
0ec18ac
9735074
e6e65e1
04bc436
c8d866a
7b3934f
f511193
06aadd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // SPDX-FileCopyrightText: 2025 Demerzel Solutions Limited | ||
| // SPDX-License-Identifier: LGPL-3.0-only | ||
|
|
||
| using System; | ||
| using Nethermind.Core.Crypto; | ||
|
|
||
| namespace Nethermind.Consensus.Processing; | ||
|
|
||
| public class BlockAddedEventArgs(Hash256 blockHash) : EventArgs | ||
| { | ||
| public Hash256 BlockHash => blockHash; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,13 +3,31 @@ | |||||
|
|
||||||
| using System; | ||||||
| using System.Collections.Generic; | ||||||
| using Nethermind.Logging; | ||||||
| using Nethermind.Monitoring; | ||||||
| using Nethermind.Monitoring.Config; | ||||||
| using NonBlocking; | ||||||
|
|
||||||
| namespace Nethermind.Db; | ||||||
|
|
||||||
| public class DbTracker | ||||||
| { | ||||||
| private readonly ConcurrentDictionary<string, IDbMeta> _createdDbs = new ConcurrentDictionary<string, IDbMeta>(); | ||||||
| private readonly int _intervalSec; | ||||||
| private long _lastDbMetricsUpdate = 0; | ||||||
|
|
||||||
| private ILogger _logger; | ||||||
|
|
||||||
| public DbTracker(IMonitoringService monitoringService, IMetricsConfig metricsConfig, ILogManager logManager) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this component shouldn't be in DB project, this would break up dependency on metrics |
||||||
| { | ||||||
| _intervalSec = metricsConfig.DbMetricIntervalSeconds; | ||||||
| _logger = logManager.GetClassLogger<DbTracker>(); | ||||||
|
|
||||||
| if (metricsConfig.EnableDbSizeMetrics) | ||||||
| { | ||||||
| monitoringService.AddMetricsUpdateAction(UpdateDbMetrics); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public void AddDb(string name, IDbMeta dbMeta) | ||||||
| { | ||||||
|
|
@@ -21,6 +39,39 @@ public IEnumerable<KeyValuePair<string, IDbMeta>> GetAllDbMeta() | |||||
| return _createdDbs; | ||||||
| } | ||||||
|
|
||||||
| public bool Paused { get; set; } = false; | ||||||
|
|
||||||
| private void UpdateDbMetrics() | ||||||
| { | ||||||
| try | ||||||
| { | ||||||
| if (Paused) return; | ||||||
|
|
||||||
| if (Environment.TickCount64 - _lastDbMetricsUpdate < _intervalSec * 1000) | ||||||
| { | ||||||
| // Update max every minute | ||||||
|
||||||
| // Update max every minute | |
| // Update max based on configured interval |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\Nethermind.Config\Nethermind.Config.csproj" /> | ||
| <ProjectReference Include="..\Nethermind.Monitoring\Nethermind.Monitoring.csproj" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think DB shouldn't be dependent on monitoring.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine, I'll put it somewhere else. |
||
| <ProjectReference Include="..\Nethermind.Serialization.Rlp\Nethermind.Serialization.Rlp.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,13 +6,15 @@ | |||||
| using Nethermind.Api; | ||||||
| using Nethermind.Blockchain.Receipts; | ||||||
| using Nethermind.Blockchain.Synchronization; | ||||||
| using Nethermind.Consensus.Processing; | ||||||
| using Nethermind.Core; | ||||||
| using Nethermind.Db; | ||||||
| using Nethermind.Db.Rocks; | ||||||
| using Nethermind.Db.Rocks.Config; | ||||||
| using Nethermind.Db.Rpc; | ||||||
| using Nethermind.JsonRpc.Client; | ||||||
| using Nethermind.Logging; | ||||||
| using Nethermind.Monitoring.Config; | ||||||
| using Nethermind.Serialization.Json; | ||||||
|
|
||||||
| namespace Nethermind.Init.Modules; | ||||||
|
|
@@ -30,7 +32,8 @@ namespace Nethermind.Init.Modules; | |||||
| public class DbModule( | ||||||
| IInitConfig initConfig, | ||||||
| IReceiptConfig receiptConfig, | ||||||
| ISyncConfig syncConfig | ||||||
| ISyncConfig syncConfig, | ||||||
| bool dontConfigureMetric = false | ||||||
| ) : Module | ||||||
| { | ||||||
| protected override void Load(ContainerBuilder builder) | ||||||
|
|
@@ -57,11 +60,6 @@ protected override void Load(ContainerBuilder builder) | |||||
| return sortedKeyValue; | ||||||
| }) | ||||||
|
|
||||||
| // Monitoring use these to track active db. We intercept db factory to keep them lazy. Does not | ||||||
| // track db that is not created by db factory though... | ||||||
| .AddSingleton<DbTracker>() | ||||||
| .AddDecorator<IDbFactory, DbTracker.DbFactoryInterceptor>() | ||||||
|
|
||||||
| .AddDatabase(DbNames.State) | ||||||
| .AddDatabase(DbNames.Code) | ||||||
| .AddDatabase(DbNames.Metadata) | ||||||
|
|
@@ -77,6 +75,30 @@ protected override void Load(ContainerBuilder builder) | |||||
| .AddColumnDatabase<BlobTxsColumns>(DbNames.BlobTransactions) | ||||||
| ; | ||||||
|
|
||||||
| if (!dontConfigureMetric) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| { | ||||||
| // Intercept created db to publish metric. | ||||||
| // Dont use constructor injection to get all db because that would resolve all db | ||||||
| // making them not lazy. | ||||||
| builder | ||||||
| .AddSingleton<DbTracker>() | ||||||
| .AddDecorator<IDbFactory, DbTracker.DbFactoryInterceptor>() | ||||||
|
|
||||||
| // Intercept block processing by checking the queue and pausing the metrics when that happen. | ||||||
| // Dont use constructor injection because this would prevent the metric from being updated before | ||||||
| // the block processing chain is constructed, eg: verifytrie or import jobs. | ||||||
| .Intercept<IBlockProcessingQueue>((processingQueue, ctx) => | ||||||
| { | ||||||
| if (!ctx.Resolve<IMetricsConfig>().PauseDbMetricDuringBlockProcessing) return; | ||||||
|
|
||||||
| // Do not update db metrics while processing a block | ||||||
| DbTracker updater = ctx.Resolve<DbTracker>(); | ||||||
| processingQueue.BlockAdded += (sender, args) => updater.Paused = !processingQueue.IsEmpty; | ||||||
|
asdacap marked this conversation as resolved.
Outdated
|
||||||
| processingQueue.BlockRemoved += (sender, args) => updater.Paused = !processingQueue.IsEmpty; | ||||||
| }) | ||||||
| ; | ||||||
| } | ||||||
|
|
||||||
| switch (initConfig.DiagnosticMode) | ||||||
| { | ||||||
| case DiagnosticMode.MemDb: | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of
BlockAddedEventArgsI would recommend reusingBlockEventArgsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched