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.

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

Pull reading of last storage subsystem out of lock (part2) #73295

Merged
merged 19 commits into from
May 2, 2024
Merged
Prev Previous commit
Next Next commit
Simplify tests
  • Loading branch information
CyrusNajmabadi committed May 2, 2024
commit 2f2b044e5060cfe4c6a873e012c348ce81eca829
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ protected AbstractPersistentStorageTests()
ThreadPool.SetMinThreads(Math.Max(workerThreads, NumThreads), completionPortThreads);
}

internal abstract AbstractPersistentStorageService GetStorageService(
IMefHostExportProvider exportProvider,
IPersistentStorageConfiguration configuration,
IPersistentStorageFaultInjector? faultInjector,
string rootFolder);
//internal abstract AbstractPersistentStorageService GetStorageService(
// IMefHostExportProvider exportProvider,
// IPersistentStorageConfiguration configuration,
// IPersistentStorageFaultInjector? faultInjector,
// string rootFolder);

public void Dispose()
{
Expand Down Expand Up @@ -1016,8 +1016,9 @@ internal async Task<IChecksummedPersistentStorage> GetStorageAsync(
persistentFolder ??= _persistentFolder;
var configuration = new MockPersistentStorageConfiguration(solution.Id, persistentFolder.Path, throwOnFailure);

_storageService = GetStorageService(solution.Workspace.Services.SolutionServices.ExportProvider, configuration, faultInjector, _persistentFolder.Path);
var storage = await _storageService.GetStorageAsync(SolutionKey.ToSolutionKey(solution), CancellationToken.None);
_storageService = (AbstractPersistentStorageService)solution.Workspace.Services.SolutionServices.GetPersistentStorageService();
var storage = await _storageService.GetStorageAsync(
SolutionKey.ToSolutionKey(solution), configuration, faultInjector, CancellationToken.None);

// If we're injecting faults, we expect things to be strange
if (faultInjector == null)
Expand All @@ -1035,8 +1036,9 @@ internal async Task<IChecksummedPersistentStorage> GetStorageFromKeyAsync(
_storageService?.GetTestAccessor().Shutdown();
var configuration = new MockPersistentStorageConfiguration(solutionKey.Id, _persistentFolder.Path, throwOnFailure: true);

_storageService = GetStorageService(services.SolutionServices.ExportProvider, configuration, faultInjector, _persistentFolder.Path);
var storage = await _storageService.GetStorageAsync(solutionKey, CancellationToken.None);
_storageService = (AbstractPersistentStorageService)services.SolutionServices.GetPersistentStorageService();
var storage = await _storageService.GetStorageAsync(
solutionKey, configuration, faultInjector, CancellationToken.None);

// If we're injecting faults, we expect things to be strange
if (faultInjector == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ internal abstract partial class AbstractPersistentStorageService(IPersistentStor
/// to delete the database and retry opening one more time. If that fails again, the <see
/// cref="NoOpPersistentStorage"/> instance will be used.
/// </summary>
protected abstract ValueTask<IChecksummedPersistentStorage?> TryOpenDatabaseAsync(SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, CancellationToken cancellationToken);
protected abstract ValueTask<IChecksummedPersistentStorage?> TryOpenDatabaseAsync(SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken);
protected abstract bool ShouldDeleteDatabase(Exception exception);

public async ValueTask<IChecksummedPersistentStorage> GetStorageAsync(SolutionKey solutionKey, CancellationToken cancellationToken)
public ValueTask<IChecksummedPersistentStorage> GetStorageAsync(SolutionKey solutionKey, CancellationToken cancellationToken)
=> GetStorageAsync(solutionKey, this.Configuration, faultInjector: null, cancellationToken);

public async ValueTask<IChecksummedPersistentStorage> GetStorageAsync(
SolutionKey solutionKey,
IPersistentStorageConfiguration configuration,
IPersistentStorageFaultInjector? faultInjector,
CancellationToken cancellationToken)
{
if (solutionKey.FilePath == null)
return NoOpPersistentStorage.GetOrThrow(solutionKey, Configuration.ThrowOnFailure);
Expand All @@ -44,7 +51,7 @@ public async ValueTask<IChecksummedPersistentStorage> GetStorageAsync(SolutionKe
if (_solutionKeyToStorage.TryGetValue(solutionKey, out var storage))
return storage;

var workingFolder = Configuration.TryGetStorageLocation(solutionKey);
var workingFolder = configuration.TryGetStorageLocation(solutionKey);
if (workingFolder == null)
return NoOpPersistentStorage.GetOrThrow(solutionKey, Configuration.ThrowOnFailure);

Expand All @@ -53,7 +60,7 @@ public async ValueTask<IChecksummedPersistentStorage> GetStorageAsync(SolutionKe
// See if another thread set to the solution we care about while we were waiting on the lock.
if (!_solutionKeyToStorage.TryGetValue(solutionKey, out storage))
{
storage = await CreatePersistentStorageAsync(solutionKey, workingFolder, cancellationToken).ConfigureAwait(false);
storage = await CreatePersistentStorageAsync(solutionKey, workingFolder, faultInjector, cancellationToken).ConfigureAwait(false);
_solutionKeyToStorage.Add(solutionKey, storage);
}

Expand All @@ -62,14 +69,14 @@ public async ValueTask<IChecksummedPersistentStorage> GetStorageAsync(SolutionKe
}

private async ValueTask<IChecksummedPersistentStorage> CreatePersistentStorageAsync(
SolutionKey solutionKey, string workingFolderPath, CancellationToken cancellationToken)
SolutionKey solutionKey, string workingFolderPath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken)
{
// Attempt to create the database up to two times. The first time we may encounter
// some sort of issue (like DB corruption). We'll then try to delete the DB and can
// try to create it again. If we can't create it the second time, then there's nothing
// we can do and we have to store things in memory.
var result = await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, cancellationToken).ConfigureAwait(false) ??
await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, cancellationToken).ConfigureAwait(false);
var result = await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, faultInjector, cancellationToken).ConfigureAwait(false) ??
await TryCreatePersistentStorageAsync(solutionKey, workingFolderPath, faultInjector, cancellationToken).ConfigureAwait(false);

if (result != null)
return result;
Expand All @@ -80,12 +87,13 @@ private async ValueTask<IChecksummedPersistentStorage> CreatePersistentStorageAs
private async ValueTask<IChecksummedPersistentStorage?> TryCreatePersistentStorageAsync(
SolutionKey solutionKey,
string workingFolderPath,
IPersistentStorageFaultInjector? faultInjector,
CancellationToken cancellationToken)
{
var databaseFilePath = GetDatabaseFilePath(workingFolderPath);
try
{
return await TryOpenDatabaseAsync(solutionKey, workingFolderPath, databaseFilePath, cancellationToken).ConfigureAwait(false);
return await TryOpenDatabaseAsync(solutionKey, workingFolderPath, databaseFilePath, faultInjector, cancellationToken).ConfigureAwait(false);
}
catch (Exception e) when (Recover(e))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,14 @@ private static bool TryInitializeLibrariesLazy()
return true;
}

private readonly IPersistentStorageFaultInjector? _faultInjector;

public SQLitePersistentStorageService(
IPersistentStorageConfiguration configuration,
IAsynchronousOperationListener asyncListener,
IPersistentStorageFaultInjector? faultInjector)
: this(configuration, asyncListener)
{
_faultInjector = faultInjector;
}

protected override string GetDatabaseFilePath(string workingFolderPath)
{
Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(workingFolderPath));
return Path.Combine(workingFolderPath, StorageExtension, nameof(v2), PersistentStorageFileName);
}

protected override ValueTask<IChecksummedPersistentStorage?> TryOpenDatabaseAsync(
SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, CancellationToken cancellationToken)
SolutionKey solutionKey, string workingFolderPath, string databaseFilePath, IPersistentStorageFaultInjector? faultInjector, CancellationToken cancellationToken)
{
if (!TryInitializeLibraries())
{
Expand All @@ -93,7 +82,7 @@ protected override string GetDatabaseFilePath(string workingFolderPath)
workingFolderPath,
databaseFilePath,
asyncListener,
_faultInjector));
faultInjector));
}

// Error occurred when trying to open this DB. Try to remove it so we can create a good DB.
Expand Down