Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ public async ValueTask Enqueue(Block block, ProcessingOptions processingOptions)
if (!_recoveryComplete)
{
Interlocked.Increment(ref _queueCount);
BlockAdded?.Invoke(this, new BlockEventArgs(block));

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

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

The BlockAdded event is fired immediately after incrementing the queue count, but before checking if the block can actually be resolved or enqueued. If blockRef.Resolve returns false or an exception occurs during enqueuing, the block is never actually added to the queue, yet BlockAdded was already invoked. This creates an inconsistency where BlockAdded fires but the block is not actually in the queue. The DbTracker uses this event to determine when to pause metrics, which could lead to incorrect pausing behavior. Consider moving the BlockAdded event invocation to after successful enqueuing, similar to how it's paired with BlockRemoved in the error paths.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The pair, block removed is also called, so its fine. We dont want a block removed without a block added.


_lastProcessedBlock = DateTime.UtcNow;
try
{
Expand Down Expand Up @@ -424,6 +426,7 @@ private void FireProcessingQueueEmpty()

public event EventHandler? ProcessingQueueEmpty;
public event EventHandler<BlockRemovedEventArgs>? BlockRemoved;
public event EventHandler<BlockEventArgs>? BlockAdded;
public bool IsEmpty => Volatile.Read(ref _queueCount) == 0;
public int Count => Volatile.Read(ref _queueCount);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public interface IBlockProcessingQueue
/// </summary>
event EventHandler ProcessingQueueEmpty;

event EventHandler<BlockEventArgs> BlockAdded;
event EventHandler<BlockRemovedEventArgs> BlockRemoved;

/// <summary>
Expand Down
7 changes: 6 additions & 1 deletion src/Nethermind/Nethermind.Core.Test/Db/TestMemDbProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Nethermind.Blockchain.Synchronization;
using Nethermind.Db;
using Nethermind.Init.Modules;
using Nethermind.Monitoring;

namespace Nethermind.Core.Test.Db
{
Expand All @@ -23,7 +24,11 @@ public static Task<IDbProvider> InitAsync()
public static IDbProvider Init()
{
return new ContainerBuilder()
.AddModule(new DbModule(new InitConfig() { DiagnosticMode = DiagnosticMode.MemDb }, new ReceiptConfig(), new SyncConfig()))
.AddModule(new DbModule(
new InitConfig() { DiagnosticMode = DiagnosticMode.MemDb },
new ReceiptConfig(),
new SyncConfig()
))
.AddSingleton<IDbProvider, ContainerOwningDbProvider>()
.Build()
.Resolve<IDbProvider>();
Comment thread
asdacap marked this conversation as resolved.
Expand Down
9 changes: 9 additions & 0 deletions src/Nethermind/Nethermind.Core/ContainerBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,15 @@ public static ContainerBuilder Intercept<T>(this ContainerBuilder builder, Actio
});
}

public static ContainerBuilder Intercept<T>(this ContainerBuilder builder, Action<T, IComponentContext> interceptor) where T : class
{
return builder.AddDecorator<T>((ctx, service) =>
{
interceptor(service, ctx);
return service;
});
}

/// <summary>
/// A convenient way of creating a service whose members can be configured independent of other instances of the same
/// type (assuming the type is of lifetime scope). This is useful for same type with multiple configuration
Expand Down
120 changes: 120 additions & 0 deletions src/Nethermind/Nethermind.Db.Test/DbTrackerTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only
Comment thread
rubo marked this conversation as resolved.

using System;
using System.Linq;
using Autofac;
using FluentAssertions;
using Nethermind.Api;
using Nethermind.Blockchain.Receipts;
using Nethermind.Blockchain.Synchronization;
using Nethermind.Consensus.Processing;
using Nethermind.Core;
using Nethermind.Core.Crypto;
using Nethermind.Core.Test;
using Nethermind.Core.Test.Builders;
using Nethermind.Init.Modules;
using Nethermind.Logging;
using Nethermind.Monitoring;
using Nethermind.Monitoring.Config;
using NSubstitute;
using NUnit.Framework;

namespace Nethermind.Db.Test;
Expand All @@ -15,6 +29,9 @@ public void TestTrackOnlyCreatedDb()
{
using IContainer container = new ContainerBuilder()
.AddSingleton<DbTracker>()
.AddSingleton<IMetricsConfig>(new MetricsConfig())
.AddSingleton<ILogManager>(LimboLogs.Instance)
.AddSingleton<IMonitoringService>(NoopMonitoringService.Instance)
.AddDecorator<IDbFactory, DbTracker.DbFactoryInterceptor>()
.AddSingleton<IDbFactory, MemDbFactory>()
.Build();
Expand All @@ -30,4 +47,107 @@ public void TestTrackOnlyCreatedDb()
var firstEntry = tracker.GetAllDbMeta().First();
firstEntry.Key.Should().Be("TestDb");
}

[Parallelizable(ParallelScope.None)]
[TestCase(true)]
[TestCase(false)]
public void TestUpdateDbMetric(bool isProcessing)
{
IBlockProcessingQueue queue = Substitute.For<IBlockProcessingQueue>();
(IContainer container, Action updateAction, FakeDb fakeDb) = ConfigureMetricUpdater((builder) => builder.AddSingleton<IBlockProcessingQueue>(queue));
using var _ = container;

// Reset
Metrics.DbReads["TestDb"] = 0;

if (isProcessing)
{
container.Resolve<IBlockProcessingQueue>(); // Only setup is something requested the block processing queue.
queue.IsEmpty.Returns(false);
queue.BlockAdded += Raise.EventWith<BlockEventArgs>(new BlockEventArgs(Build.A.Block.TestObject));
}

updateAction!();

// Assert
Assert.That(Metrics.DbReads["TestDb"], isProcessing ? Is.EqualTo(0) : Is.EqualTo(10));
}

[Parallelizable(ParallelScope.None)]
[Test]
public void DoesNotUpdateIfIntervalHasNotPassed()
{
(IContainer container, Action updateAction, FakeDb fakeDb) = ConfigureMetricUpdater();
using var _ = container;

container.Resolve<IDbFactory>().CreateDb(new DbSettings("TestDb", "TestDb"));

// Reset
Metrics.DbReads["TestDb"] = 0;

updateAction!();
Assert.That(Metrics.DbReads["TestDb"], Is.EqualTo(10));

fakeDb.SetMetric(new IDbMeta.DbMetric()
{
TotalReads = 11
});

updateAction!();
Assert.That(Metrics.DbReads["TestDb"], Is.EqualTo(10));
}

private (IContainer, Action, FakeDb) ConfigureMetricUpdater(Action<ContainerBuilder>? configurer = null)
{
IDbFactory fakeDbFactory = Substitute.For<IDbFactory>();

IMonitoringService monitoringService = Substitute.For<IMonitoringService>();
ContainerBuilder builder = new ContainerBuilder()
.AddModule(new DbModule(new InitConfig(), new ReceiptConfig(), new SyncConfig()))
.AddModule(new DbMonitoringModule())
.AddSingleton<IMetricsConfig>(new MetricsConfig())
.AddSingleton<ILogManager>(LimboLogs.Instance)
.AddSingleton<IMonitoringService>(monitoringService)
.AddDecorator<IDbFactory, DbTracker.DbFactoryInterceptor>()
.AddSingleton<IDbFactory>(fakeDbFactory);

configurer?.Invoke(builder);

IContainer container = builder
.Build();

IDbMeta.DbMetric metric = new IDbMeta.DbMetric()
{
TotalReads = 10
};
FakeDb fakeDb = new FakeDb(metric);
fakeDbFactory.CreateDb(Arg.Any<DbSettings>()).Returns(fakeDb);

Action updateAction = null;
monitoringService
.When((m) => m.AddMetricsUpdateAction(Arg.Any<Action>()))
.Do((c) =>
{
updateAction = (Action)c[0];
});

container.Resolve<IDbFactory>().CreateDb(new DbSettings("TestDb", "TestDb"));

return (container, updateAction, fakeDb);
}

private class FakeDb(IDbMeta.DbMetric metric) : TestMemDb, IDbMeta
{
private IDbMeta.DbMetric _metric = metric;

public override IDbMeta.DbMetric GatherMetric(bool includeSharedCache = false)
{
return _metric;
}

internal void SetMetric(IDbMeta.DbMetric metric)
{
_metric = metric;
}
}
}
51 changes: 51 additions & 0 deletions src/Nethermind/Nethermind.Db/DbTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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)
{
Expand All @@ -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 based on configured interval
return;
}

foreach (KeyValuePair<string, IDbMeta> kv in GetAllDbMeta())
{
// Note: At the moment, the metric for a columns db is combined across column.
IDbMeta.DbMetric dbMetric = kv.Value.GatherMetric(includeSharedCache: kv.Key == DbNames.State); // Only include shared cache if state db
Db.Metrics.DbSize[kv.Key] = dbMetric.Size;
Db.Metrics.DbBlockCacheSize[kv.Key] = dbMetric.CacheSize;
Db.Metrics.DbMemtableSize[kv.Key] = dbMetric.MemtableSize;
Db.Metrics.DbIndexFilterSize[kv.Key] = dbMetric.IndexSize;
Db.Metrics.DbReads[kv.Key] = dbMetric.TotalReads;
Db.Metrics.DbWrites[kv.Key] = dbMetric.TotalWrites;
}
_lastDbMetricsUpdate = Environment.TickCount64;
}
catch (Exception e)
{
if (_logger.IsError) _logger.Error("Error during updating db metrics", e);
}
}

public class DbFactoryInterceptor(DbTracker tracker, IDbFactory baseFactory) : IDbFactory
{
public IDb CreateDb(DbSettings dbSettings)
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Db/MemDb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public virtual void Set(ReadOnlySpan<byte> key, byte[]? value, WriteFlags flags
_spanDb[key] = value;
}

public IDbMeta.DbMetric GatherMetric(bool includeSharedCache = false) => new() { Size = Count };
public virtual IDbMeta.DbMetric GatherMetric(bool includeSharedCache = false) => new() { Size = Count };

private IEnumerable<KeyValuePair<byte[], byte[]?>> OrderedDb => _db.OrderBy(kvp => kvp.Key, Bytes.Comparer);
}
Expand Down
1 change: 1 addition & 0 deletions src/Nethermind/Nethermind.Db/Nethermind.Db.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

<ItemGroup>
<ProjectReference Include="..\Nethermind.Config\Nethermind.Config.csproj" />
<ProjectReference Include="..\Nethermind.Monitoring\Nethermind.Monitoring.csproj" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still think DB shouldn't be dependent on monitoring.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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>

Expand Down
36 changes: 31 additions & 5 deletions src/Nethermind/Nethermind.Init/Modules/DbModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -57,11 +59,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)
Expand Down Expand Up @@ -130,3 +127,32 @@ public IColumnsDb<T> CreateColumnsDb<T>(DbSettings dbSettings) where T : struct,
}
}
}

public class DbMonitoringModule : Module
{
protected override void Load(ContainerBuilder builder)
{
base.Load(builder);

// 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;
processingQueue.BlockRemoved += (sender, args) => updater.Paused = !processingQueue.IsEmpty;

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

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

The event handlers are subscribed to IBlockProcessingQueue events but are never unsubscribed. This can cause a memory leak because the DbTracker instance will be kept alive by these event subscriptions even if it should be garbage collected. The IBlockProcessingQueue is likely long-lived, while the interception happens for each resolved instance. Consider storing the event subscription in a disposable wrapper and ensuring cleanup happens when the DbTracker is disposed, or ensure that the lifetimes are correctly aligned so this doesn't cause issues.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Its fine. There is only one instance.

})
;
}
}
Loading