microsoft / microsoft/garnet

Multi-database + storage tier: databases share one log device, causing cross-database data corruption

Open
#2,152 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
12k
Forks
703
Avg merge
2d 19h
Merged PRs (30d)
36

Description

### Describe the bug

When more than one logical database is used together with tiered storage (`--storage-tier`), all databases write to the **same** main-log and object-log files. Each database gets its own `TsavoriteKV` with its own `IDevice`, but every device is built from a fixed file descriptor that does not include the database id:

```csharp
// libs/server/Servers/GarnetServerOptions.cs — GetSettings()
kvSettings.LogDevice = logFactory.Get(new FileDescriptor("Store", "hlog"));
kvSettings.ObjectLogDevice = logFactory.Get(new FileDescriptor("Store", "hlog_objs"));
```

`CreateDatabase(dbId)` → `CreateStore(dbId, …)` → `opts.GetSettings(…)`, so `dbId` never reaches the log device names. Each store numbers pages from logical address 0, so they write overlapping regions of one file and overwrite each other.

Checkpoint and AOF directories *are* per-database; the log devices are the outlier:

```
Store\checkpoints\ Store\checkpoints_1\ Store\checkpoints_2\ <- per-DB
Store\hlog.0 Store\hlog_objs.0 <- ONE, shared by all
```

**Effect:** a read on database 0 silently returns **database 1's value**. This is not data loss — it is cross-database data leakage. It happens at runtime with no restart, no checkpoint and no recovery involved.

Measured with 2 databases x 300 hash keys:

```
[lowMemory=False] db0: correct=300 foreign=0 <- fully resident, no corruption
[lowMemory=False] db1: correct=300 foreign=0
[lowMemory=True] db0: correct=80 foreign=220 <- db0 returns db1's values
[lowMemory=True] db1: correct=300 foreign=0
```

`foreign` means db0's key `h:42` returned `db1-v00042`.

The `lowMemory` contrast pins the mechanism: corruption appears **only when records are evicted to the log device and read back from it**. Fully-resident data is unaffected, which is likely why this has not been noticed — it needs memory pressure plus tiered storage plus multiple databases at once.

Ruled out as an alternative explanation: `SingleDatabaseManager.TryGetOrAddDatabase` throws for `dbId != 0`, and `StoreWrapper.CheckMultiDatabaseCompatibility()` hot-swaps in a real `MultiDatabaseManager`, so database 1 is genuinely a separate store and not database 0 under an alias.

Cluster mode is unaffected: it forces `MaxDatabases = 1` (`AllowMultiDb => !EnableCluster && MaxDatabases > 1`, `maxDatabases = opts.EnableCluster ? 1 : opts.MaxDatabases`, and `DatabaseManagerFactory` returning `SingleDatabaseManager`).

### Steps to reproduce the bug

Add this test to `test/standalone/Garnet.test/`. It uses only existing test helpers and compiles against `main` unmodified.

```csharp
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using NUnit.Framework;
using NUnit.Framework.Legacy;
using StackExchange.Redis;

namespace Garnet.test
{
[TestFixture]
public class MultiDbSharedLogDeviceTests : TestBase
{
[SetUp]
public void Setup() => TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);

[TearDown]
public void TearDown() => TestUtils.OnTearDown(suppressFailure: true);

static string Val(int dbId, int key) => $"db{dbId}-v{key:D5}";

// lowMemory=true evicts records to the log device; lowMemory=false keeps them resident.
// Only the evicting case corrupts, which is the shared-file signature.
[Test]
public void DatabasesMustNotShareLogDevice([Values(false, true)] bool lowMemory)
{
const int numKeys = 300, numDbs = 2;

using var server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, lowMemory: lowMemory);
server.Start();
using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());

for (var dbId = 0; dbId < numDbs; dbId++)
{
var db = redis.GetDatabase(dbId);
for (var key = 0; key < numKeys; key++)
db.HashSet($"h:{key}", [new HashEntry("f", Val(dbId, key))]);
}

for (var dbId = 0; dbId < numDbs; dbId++)
{
var db = redis.GetDatabase(dbId);
int correct = 0, foreign = 0;
for (var key = 0; key < numKeys; key++)
{
var got = (string)db.HashGet($"h:{key}", "f");
if (got == Val(dbId, key)) ++correct;
else if (got == Val(1 - dbId, key)) ++foreign;
}
TestContext.Progress.WriteLine($"db{dbId}: correct={correct} foreign={foreign}");
ClassicAssert.AreEqual(0, foreign, $"db{dbId} returned another database's values");
ClassicAssert.AreEqual(numKeys, correct, $"db{dbId} lost records");
}
}
}
}
```

Run:

```
dotnet test test/standalone/Garnet.test -f net10.0 -c Debug --filter "FullyQualifiedName~DatabasesMustNotShareLogDevice"
```

Observed:

```
db0: correct=300 foreign=0
db1: correct=300 foreign=0
Passed DatabasesMustNotShareLogDevice(False)

db0: correct=80 foreign=220
Failed DatabasesMustNotShareLogDevice(True)
db0 returned another database's values
```

Equivalent reproduction without any test code:

```
garnet --storage-tier --logdir /tmp/gx --memory 64k --page 4k

redis-cli -n 0 # write ~300 hash keys h:0..h:299, value db0-
redis-cli -n 1 # write the same keys, value db1-
redis-cli -n 0 HGET h:200 f # returns db1-200
ls /tmp/gx/Store/ # one hlog.0 and hlog_objs.0, but checkpoints and checkpoints_1
```

The directory listing on its own shows the problem: N databases, one log file.

### Expected behavior

Each logical database should have its own main-log and object-log files, as each already has its own checkpoint and AOF directory. A read on database 0 must never return a value written to database 1.

A fix presumably makes the log device names database-aware, mirroring `GarnetServerOptions.GetCheckpointDirectoryName(int dbId)` / `GetAppendOnlyFileDirectoryName(int dbId)`, for example `hlog_{dbId}` or a `Store_{dbId}` directory. Database 0 should keep today's names so existing single-database stores on disk continue to be recovered unchanged.

### Screenshots

N/A

### Release version

Reproduced against `main` @ `277ea6c34`.

The offending device descriptors are byte-identical on `main`; the reproduction above was executed on a branch off `93bdfaad6` whose changes are confined to the recovery path, and the corruption reproduces with no recovery, checkpoint or restart involved.

### IDE

N/A — reproduced from the `dotnet` CLI.

### OS version

Windows 11. The defect is in file naming and is expected to be platform independent.

### Additional context

Requires all three conditions together: tiered storage enabled, more than one database in use, and enough memory pressure to evict records to the log device. In-memory-only multi-database use and all cluster configurations are unaffected.

Found while testing the `--upgrade` downlevel object-log conversion in #2081. That PR does not cause this and does not fix it; it refuses to up-convert a multi-database store for the same underlying reason (a single shared object log cannot hold each database's converted records).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in libs/server/Servers/GarnetServerOptions.cs at GetSettings(), then trace CreateDatabase(dbId) through CreateStore and the checkpoint/AOF naming helpers. Add the supplied regression test under test/standalone/Garnet.test/ and run the documented dotnet test command with both low-memory cases. Done means each database has isolated log devices, the test reports no foreign values, and existing single-database names remain recoverable.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.