Merge pull request #18367 from CyrusNajmabadi/sqliteWork6

Small changes to storage subsystem to make it easier to slot in SQLite

src/Compilers/Core/Portable/InternalUtilities/MultiDictionary.cs

@@ -220,5 +220,10 @@ internal void Clear()
         {
             _dictionary.Clear();
         }
+
+        public void Remove(K key)
+        {
+            _dictionary.Remove(key);
+        }
     }
 }
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspace/Esent/EsentPersistentStorage.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.Storage;
 using Microsoft.Isam.Esent;
 using Microsoft.Isam.Esent.Interop;
 using Roslyn.Utilities;
@@ -16,9 +17,6 @@ namespace Microsoft.CodeAnalysis.Esent
 {
     internal partial class EsentPersistentStorage : AbstractPersistentStorage
     {
-        private const string StorageExtension = "vbcs.cache";
-        private const string PersistentStorageFileName = "storage.ide";
-
         // cache delegates so that we don't re-create it every times
         private readonly Func<int, object, object, object, CancellationToken, Stream> _readStreamSolution;
         private readonly Func<EsentStorage.Key, int, object, object, CancellationToken, Stream> _readStream;
@@ -29,8 +27,12 @@ internal partial class EsentPersistentStorage : AbstractPersistentStorage
         private readonly EsentStorage _esentStorage;
 
         public EsentPersistentStorage(
-            IOptionService optionService, string workingFolderPath, string solutionFilePath, Action<AbstractPersistentStorage> disposer) 
-            : base(optionService, workingFolderPath, solutionFilePath, disposer)
+            IOptionService optionService,
+            string workingFolderPath,
+            string solutionFilePath, 
+            string databaseFile,
+            Action<AbstractPersistentStorage> disposer) 
+            : base(optionService, workingFolderPath, solutionFilePath, databaseFile, disposer)
         {
             // cache delegates
             _readStreamSolution = ReadStreamSolution;
@@ -41,35 +43,17 @@ internal partial class EsentPersistentStorage : AbstractPersistentStorage
             // solution must exist in disk. otherwise, we shouldn't be here at all.
             Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(solutionFilePath));
 
-            var databaseFile = GetDatabaseFile(workingFolderPath);
-
-            this.EsentDirectory = Path.GetDirectoryName(databaseFile);
-
-            if (!Directory.Exists(this.EsentDirectory))
-            {
-                Directory.CreateDirectory(this.EsentDirectory);
-            }
-
             _nameTableCache = new ConcurrentDictionary<string, int>(StringComparer.OrdinalIgnoreCase);
 
             var enablePerformanceMonitor = optionService.GetOption(PersistentStorageOptions.EsentPerformanceMonitor);
-            _esentStorage = new EsentStorage(databaseFile, enablePerformanceMonitor);
+            _esentStorage = new EsentStorage(DatabaseFile, enablePerformanceMonitor);
         }
 
-        public static string GetDatabaseFile(string workingFolderPath)
-        {
-            Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(workingFolderPath));
-
-            return Path.Combine(workingFolderPath, StorageExtension, PersistentStorageFileName);
-        }
-
-        public void Initialize()
+        public override void Initialize(Solution solution)
         {
             _esentStorage.Initialize();
         }
 
-        public string EsentDirectory { get; }
-
         public override Task<Stream> ReadStreamAsync(Document document, string name, CancellationToken cancellationToken = default(CancellationToken))
         {
             Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(name));
@@ -283,7 +267,7 @@ private bool TryGetUniqueId(string value, bool fileCheck, out int id)
             {
                 // if we get fatal errors from esent such as disk out of space or log file corrupted by other process and etc
                 // don't crash VS, but let VS know it can't use esent. we will gracefully recover issue by using memory.
-                EsentLogger.LogException(ex);
+                StorageDatabaseLogger.LogException(ex);
 
                 return false;
             }

src/Workspaces/Core/Desktop/Workspace/Esent/EsentPersistentStorageService.cs

@@ -7,11 +7,15 @@
 using Microsoft.CodeAnalysis.SolutionSize;
 using Microsoft.CodeAnalysis.Storage;
 using Microsoft.Isam.Esent.Interop;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Esent
 {
-    internal partial class EsentPersistentStorageService : AbstractPersistentStorageService, IPersistentStorageService
+    internal partial class EsentPersistentStorageService : AbstractPersistentStorageService
     {
+        private const string StorageExtension = "vbcs.cache";
+        private const string PersistentStorageFileName = "storage.ide";
+
         public EsentPersistentStorageService(
             IOptionService optionService,
             SolutionSizeTracker solutionSizeTracker)
@@ -24,64 +28,21 @@ public EsentPersistentStorageService(IOptionService optionService, bool testing)
         {
         }
 
-        public EsentPersistentStorageService(IOptionService optionService) 
-            : base(optionService)
+        protected override string GetDatabaseFilePath(string workingFolderPath)
         {
+            Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(workingFolderPath));
+            return Path.Combine(workingFolderPath, StorageExtension, PersistentStorageFileName);
         }
 
-        protected override string GetDatabaseFilePath(string workingFolderPath)
-            => EsentPersistentStorage.GetDatabaseFile(workingFolderPath);
+        protected override AbstractPersistentStorage OpenDatabase(Solution solution, string workingFolderPath)
+            => new EsentPersistentStorage(OptionService,
+                workingFolderPath, solution.FilePath, GetDatabaseFilePath(workingFolderPath), this.Release);
 
-        protected override bool TryCreatePersistentStorage(
-            string workingFolderPath, string solutionPath,
-            out AbstractPersistentStorage persistentStorage)
+        protected override bool ShouldDeleteDatabase(Exception exception)
         {
-            persistentStorage = null;
-            EsentPersistentStorage esent = null;
-
-            try
-            {
-                esent = new EsentPersistentStorage(OptionService, workingFolderPath, solutionPath, this.Release);
-                esent.Initialize();
-
-                persistentStorage = esent;
-                return true;
-            }
-            catch (EsentAccessDeniedException ex)
-            {
-                // esent db is already in use by someone.
-                if (esent != null)
-                {
-                    esent.Close();
-                }
-
-                EsentLogger.LogException(ex);
-
-                return false;
-            }
-            catch (Exception ex)
-            {
-                if (esent != null)
-                {
-                    esent.Close();
-                }
-
-                EsentLogger.LogException(ex);
-            }
-
-            try
-            {
-                if (esent != null)
-                {
-                    Directory.Delete(esent.EsentDirectory, recursive: true);
-                }
-            }
-            catch
-            {
-                // somehow, we couldn't delete the directory.
-            }
-
-            return false;
+            // Access denied can happen when some other process is holding onto the DB.
+            // Don't want to delete it in that case.  For all other cases, delete the db.
+            return !(exception is EsentAccessDeniedException);
         }
     }
 }
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspace/Storage/PersistenceStorageServiceFactory.cs

@@ -14,6 +14,7 @@ namespace Microsoft.CodeAnalysis.Storage
     [ExportWorkspaceServiceFactory(typeof(IPersistentStorageService), ServiceLayer.Desktop), Shared]
     internal class PersistenceStorageServiceFactory : IWorkspaceServiceFactory
     {
+        private readonly object _gate = new object();
         private readonly SolutionSizeTracker _solutionSizeTracker;
 
         private IPersistentStorageService _singleton;
@@ -26,22 +27,29 @@ public PersistenceStorageServiceFactory(SolutionSizeTracker solutionSizeTracker)
 
         public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
         {
-            if (_singleton == null)
+            lock (_gate)
             {
-                var optionService = workspaceServices.GetService<IOptionService>();
-                var database = optionService.GetOption(StorageOptions.Database);
-                switch (database)
+                if (_singleton == null)
                 {
-                    case StorageDatabase.Esent:
-                        Interlocked.CompareExchange(ref _singleton, new EsentPersistentStorageService(optionService, _solutionSizeTracker), null);
-                        break;
-                    default:
-                        Interlocked.CompareExchange(ref _singleton, NoOpPersistentStorageService.Instance, null);
-                        break;
+                    _singleton = GetPersistentStorageService(workspaceServices);
                 }
+
+                return _singleton;
             }
+        }
 
-            return _singleton;
+        private IPersistentStorageService GetPersistentStorageService(HostWorkspaceServices workspaceServices)
+        {
+            var optionService = workspaceServices.GetService<IOptionService>();
+            var database = optionService.GetOption(StorageOptions.Database);
+            switch (database)
+            {
+                case StorageDatabase.Esent:
+                    return new EsentPersistentStorageService(optionService, _solutionSizeTracker);
+                case StorageDatabase.None:
+                default:
+                    return NoOpPersistentStorageService.Instance;
+            }
         }
     }
 }
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspace/Storage/PersistentStorageService.cs

@@ -4,8 +4,10 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.Shared.Utilities;
 using Microsoft.CodeAnalysis.SolutionSize;
 using Roslyn.Utilities;
 
@@ -17,11 +19,6 @@ namespace Microsoft.CodeAnalysis.Storage
     /// </summary>
     internal abstract partial class AbstractPersistentStorageService : IPersistentStorageService
     {
-        /// <summary>
-        /// threshold to start to use a DB (50MB)
-        /// </summary>
-        private const int SolutionSizeThreshold = 50 * 1024 * 1024;
-
         protected readonly IOptionService OptionService;
         private readonly SolutionSizeTracker _solutionSizeTracker;
 
@@ -51,21 +48,14 @@ internal abstract partial class AbstractPersistentStorageService : IPersistentSt
         }
 
         protected AbstractPersistentStorageService(IOptionService optionService, bool testing) 
-            : this(optionService)
-        {
-            _testing = true;
-        }
-
-        protected AbstractPersistentStorageService(IOptionService optionService) 
             : this(optionService, solutionSizeTracker: null)
         {
+            _testing = true;
         }
 
         protected abstract string GetDatabaseFilePath(string workingFolderPath);
-
-        protected abstract bool TryCreatePersistentStorage(
-            string workingFolderPath, string solutionPath,
-            out AbstractPersistentStorage persistentStorage);
+        protected abstract AbstractPersistentStorage OpenDatabase(Solution solution, string workingFolderPath);
+        protected abstract bool ShouldDeleteDatabase(Exception exception);
 
         public IPersistentStorage GetStorage(Solution solution)
         {
@@ -131,7 +121,7 @@ private IPersistentStorage GetStorage(Solution solution, string workingFolderPat
                 }
 
                 // try create new one
-                storage = TryCreatePersistentStorage(workingFolderPath, solution.FilePath);
+                storage = TryCreatePersistentStorage(solution, workingFolderPath);
                 _lookup.Add(solution.FilePath, storage);
 
                 if (storage != null)
@@ -175,7 +165,8 @@ private bool SolutionSizeAboveThreshold(Solution solution)
             }
 
             var size = _solutionSizeTracker.GetSolutionSize(solution.Workspace, solution.Id);
-            return size > SolutionSizeThreshold;
+            var threshold = this.OptionService.GetOption(StorageOptions.SolutionSizeThreshold);
+            return size >= threshold;
         }
 
         private void RegisterPrimarySolutionStorageIfNeeded(Solution solution, AbstractPersistentStorage storage)
@@ -201,16 +192,14 @@ private string GetWorkingFolderPath(Solution solution)
             return locationService?.GetStorageLocation(solution);
         }
 
-        private AbstractPersistentStorage TryCreatePersistentStorage(string workingFolderPath, string solutionPath)
+        private AbstractPersistentStorage TryCreatePersistentStorage(Solution solution, string workingFolderPath)
         {
-            if (TryCreatePersistentStorage(workingFolderPath, solutionPath, out var persistentStorage))
-            {
-                return persistentStorage;
-            }
-
-            // first attempt could fail if there was something wrong with existing db.
-            // try one more time in case the first attempt fixed the problem.
-            if (TryCreatePersistentStorage(workingFolderPath, solutionPath, out persistentStorage))
+            // 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.
+            if (TryCreatePersistentStorage(solution, workingFolderPath, out var persistentStorage) ||
+                TryCreatePersistentStorage(solution, workingFolderPath, out persistentStorage))
             {
                 return persistentStorage;
             }
@@ -220,6 +209,42 @@ private AbstractPersistentStorage TryCreatePersistentStorage(string workingFolde
             return null;
         }
 
+        private bool TryCreatePersistentStorage(
+            Solution solution, string workingFolderPath,
+            out AbstractPersistentStorage persistentStorage)
+        {
+            persistentStorage = null;
+            AbstractPersistentStorage database = null;
+
+            try
+            {
+                database = OpenDatabase(solution, workingFolderPath);
+                database.Initialize(solution);
+
+                persistentStorage = database;
+                return true;
+            }
+            catch (Exception ex)
+            {
+                StorageDatabaseLogger.LogException(ex);
+
+                if (database != null)
+                {
+                    database.Close();
+                }
+
+                if (ShouldDeleteDatabase(ex))
+                {
+                    // this was not a normal exception that we expected during DB open.
+                    // Report this so we can try to address whatever is causing this.
+                    FatalError.ReportWithoutCrash(ex);
+                    IOUtilities.PerformIO(() => Directory.Delete(database.DatabaseDirectory, recursive: true));
+                }
+
+                return false;
+            }
+        }
+
         protected void Release(AbstractPersistentStorage storage)
         {
             lock (_lookupAccessLock)
@@ -285,4 +310,4 @@ public void UnregisterPrimarySolution(SolutionId solutionId, bool synchronousShu
             }
         }
     }
-}
+}
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspace/Storage/StorageDatabase.cs

@@ -4,7 +4,7 @@ namespace Microsoft.CodeAnalysis.Storage
 {
     internal enum StorageDatabase
     {
-        SQLite = 0,
+        None = 0,
         Esent = 1,
     }
 }
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspace/Esent/EsentLogger.cs → src/Workspaces/Core/Desktop/Workspace/Storage/StorageDatabaseLogger.cs

@@ -4,9 +4,9 @@
 using System.Collections.Concurrent;
 using Microsoft.CodeAnalysis.Internal.Log;
 
-namespace Microsoft.CodeAnalysis.Esent
+namespace Microsoft.CodeAnalysis.Storage
 {
-    internal class EsentLogger
+    internal class StorageDatabaseLogger
     {
         private const string Kind = nameof(Kind);
         private const string Reason = nameof(Reason);
@@ -21,7 +21,7 @@ internal static void LogException(Exception ex)
                 return;
             }
 
-            Logger.Log(FunctionId.Esent_Exceptions, KeyValueLogMessage.Create(m =>
+            Logger.Log(FunctionId.StorageDatabase_Exceptions, KeyValueLogMessage.Create(m =>
             {
                 // this is okay since it is our exception
                 m[Kind] = ex.GetType().ToString();

src/Workspaces/Core/Desktop/Workspace/Storage/StorageOptions.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Options;
 
 namespace Microsoft.CodeAnalysis.Storage
@@ -15,5 +10,11 @@ internal static class StorageOptions
 
         public static readonly Option<StorageDatabase> Database = new Option<StorageDatabase>(
             OptionName, nameof(Database), defaultValue: StorageDatabase.Esent);
+
+        /// <summary>
+        /// Solution size threshold to start to use a DB (Default: 50MB)
+        /// </summary>
+        public static readonly Option<int> SolutionSizeThreshold = new Option<int>(
+            OptionName, nameof(SolutionSizeThreshold), defaultValue: 50 * 1024 * 1024);
     }
-}
+}
\ No newline at end of file

src/Workspaces/Core/Desktop/Workspaces.Desktop.csproj

@@ -58,7 +58,6 @@
       <DependentUpon>WorkspaceDesktopResources.resx</DependentUpon>
     </Compile>
     <Compile Include="Workspace\CommandLineProject.cs" />
-    <Compile Include="Workspace\Esent\EsentLogger.cs" />
     <Compile Include="Workspace\Esent\EsentPersistentStorage.cs" />
     <Compile Include="Workspace\Esent\EsentStorage.AbstractTable.cs" />
     <Compile Include="Workspace\Esent\EsentStorage.AbstractTableAccessor.cs" />
@@ -112,6 +111,7 @@
     <Compile Include="Workspace\Storage\PersistenceStorageServiceFactory.cs" />
     <Compile Include="Workspace\Storage\PersistentStorageService.cs" />
     <Compile Include="Workspace\Storage\StorageDatabase.cs" />
+    <Compile Include="Workspace\Storage\StorageDatabaseLogger.cs" />
     <Compile Include="Workspace\Storage\StorageOptions.cs" />
   </ItemGroup>
   <ItemGroup>

src/Workspaces/Core/Portable/Log/FunctionId.cs

@@ -274,7 +274,7 @@ internal enum FunctionId
         DiagnosticAnalyzerDriver_AnalyzerCrash,
         DiagnosticAnalyzerDriver_AnalyzerTypeCount,
         PersistedSemanticVersion_Info,
-        Esent_Exceptions,
+        StorageDatabase_Exceptions,
         WorkCoordinator_ShutdownTimeout,
         Diagnostics_HyperLink,
 

src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/AbstractPersistentStorage.cs

@@ -16,24 +16,48 @@ internal abstract class AbstractPersistentStorage : IPersistentStorage
 
         private int _refCounter;
 
+        public string WorkingFolderPath { get; }
+        public string SolutionFilePath { get; }
+
+        public string DatabaseFile { get; }
+        public string DatabaseDirectory => Path.GetDirectoryName(DatabaseFile);
+
+        protected bool PersistenceEnabled
+            => _optionService.GetOption(PersistentStorageOptions.Enabled);
+
         protected AbstractPersistentStorage(
-            IOptionService optionService, string workingFolderPath, string solutionFilePath, Action<AbstractPersistentStorage> disposer)
+            IOptionService optionService, 
+            string workingFolderPath, 
+            string solutionFilePath,
+            string databaseFile,
+            Action<AbstractPersistentStorage> disposer)
         {
             Contract.ThrowIfNull(disposer);
 
             this.WorkingFolderPath = workingFolderPath;
             this.SolutionFilePath = solutionFilePath;
+            this.DatabaseFile = databaseFile;
 
             _refCounter = 0;
             _optionService = optionService;
             _disposer = disposer;
+
+            if (!Directory.Exists(this.DatabaseDirectory))
+            {
+                Directory.CreateDirectory(this.DatabaseDirectory);
+            }
         }
 
-        public string WorkingFolderPath { get; }
-        public string SolutionFilePath { get; }
+        public abstract void Initialize(Solution solution);
+        public abstract void Close();
 
-        protected bool PersistenceEnabled
-            => _optionService.GetOption(PersistentStorageOptions.Enabled);
+        public abstract Task<Stream> ReadStreamAsync(string name, CancellationToken cancellationToken = default(CancellationToken));
+        public abstract Task<Stream> ReadStreamAsync(Project project, string name, CancellationToken cancellationToken = default(CancellationToken));
+        public abstract Task<Stream> ReadStreamAsync(Document document, string name, CancellationToken cancellationToken = default(CancellationToken));
+
+        public abstract Task<bool> WriteStreamAsync(string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
+        public abstract Task<bool> WriteStreamAsync(Project project, string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
+        public abstract Task<bool> WriteStreamAsync(Document document, string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
 
         public void Dispose()
         {
@@ -59,17 +83,5 @@ public bool ReleaseRefUnsafe()
             Contract.Requires(changedValue >= 0);
             return changedValue == 0;
         }
-
-        public virtual void Close()
-        {
-        }
-
-        public abstract Task<Stream> ReadStreamAsync(string name, CancellationToken cancellationToken = default(CancellationToken));
-        public abstract Task<Stream> ReadStreamAsync(Project project, string name, CancellationToken cancellationToken = default(CancellationToken));
-        public abstract Task<Stream> ReadStreamAsync(Document document, string name, CancellationToken cancellationToken = default(CancellationToken));
-
-        public abstract Task<bool> WriteStreamAsync(string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
-        public abstract Task<bool> WriteStreamAsync(Project project, string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
-        public abstract Task<bool> WriteStreamAsync(Document document, string name, Stream stream, CancellationToken cancellationToken = default(CancellationToken));
     }
 }
\ No newline at end of file