Lucene index amend improvements (#14513)

* Use index writer to amend index after Dynamo Launch * Update sorting * Update * Update * Update * Make sure package loading end will already release index lock * Update Comments * Clean Up * Code clean up * clean up * Code Clean Up * regressions * revert code clean up because it affects running tests in parallel * update
DynamoDS · Mar 4, 2024 · 10b7a64 · 10b7a64
1 parent b55fe15
commit 10b7a64
Show file tree

Hide file tree

Showing 10 changed files with 49 additions and 47 deletions.
diff --git a/src/AssemblySharedInfoGenerator/AssemblySharedInfo.cs b/src/AssemblySharedInfoGenerator/AssemblySharedInfo.cs
@@ -45,7 +45,7 @@
 // to distinguish one build from another. AssemblyFileVersion is specified
 // in AssemblyVersionInfo.cs so that it can be easily incremented by the
 // automated build process.
-[assembly: AssemblyVersion("2.19.5.7897")]
+[assembly: AssemblyVersion("2.19.5.7936")]
 
 
 // By default, the "Product version" shown in the file properties window is
@@ -64,4 +64,4 @@
 // You can specify all the values or you can default the Build and Revision Numbers
 // by using the '*' as shown below:
 // [assembly: AssemblyVersion("1.0.*")]
-[assembly: AssemblyFileVersion("2.19.5.7897")]
+[assembly: AssemblyFileVersion("2.19.5.7936")]
diff --git a/src/DynamoCore/Search/NodeSearchModel.cs b/src/DynamoCore/Search/NodeSearchModel.cs
@@ -247,14 +247,7 @@ internal IEnumerable<NodeSearchElement> Search(string search, LuceneSearchUtilit
             if (luceneSearchUtility != null)
             {
                 //The DirectoryReader and IndexSearcher have to be assigned after commiting indexing changes and before executing the Searcher.Search() method, otherwise new indexed info won't be reflected
-                if (luceneSearchUtility.writer != null)
-                {
-                    luceneSearchUtility.dirReader = luceneSearchUtility.writer.GetReader(applyAllDeletes: true);
-                }
-                else
-                {
-                    luceneSearchUtility.dirReader = DirectoryReader.Open(luceneSearchUtility.indexDir);
-                }
+                luceneSearchUtility.dirReader = luceneSearchUtility.writer != null ? luceneSearchUtility.writer.GetReader(applyAllDeletes: true) : DirectoryReader.Open(luceneSearchUtility.indexDir);
                 luceneSearchUtility.Searcher = new IndexSearcher(luceneSearchUtility.dirReader);
 
                 string searchTerm = search.Trim();

diff --git a/src/DynamoCore/Utilities/LuceneSearchUtility.cs b/src/DynamoCore/Utilities/LuceneSearchUtility.cs
@@ -3,12 +3,8 @@
 using System.IO;
 using System.Linq;
 using Dynamo.Configuration;
-using Dynamo.Core;
-using Dynamo.Events;
-using Dynamo.Logging;
 using Dynamo.Models;
 using Dynamo.Search.SearchElements;
-using Dynamo.Session;
 using Lucene.Net.Analysis;
 using Lucene.Net.Analysis.Br;
 using Lucene.Net.Analysis.Cjk;
@@ -95,6 +91,7 @@ public enum LuceneStorage
         /// <summary>
         /// Constructor for LuceneSearchUtility, it will use the storage type passed as parameter
         /// </summary>
+        /// <param name="model"></param>
         /// <param name="config"></param>
         internal LuceneSearchUtility(DynamoModel model, LuceneStartConfig config)
         {
@@ -134,32 +131,28 @@ internal void InitializeLuceneConfig()
         /// <summary>
         /// Create index writer for followup doc indexing
         /// </summary>
-        internal void CreateLuceneIndexWriter()
+        /// <param name="mode">Index open mode for Lucene index writer</param>
+        internal void CreateLuceneIndexWriter(OpenMode mode = OpenMode.CREATE)
         {
             // Create an index writer
             IndexWriterConfig indexConfig = new IndexWriterConfig(LuceneConfig.LuceneNetVersion, Analyzer)
             {
-                OpenMode = OpenMode.CREATE
+                OpenMode = mode
             };
             try
             {
                 writer = new IndexWriter(indexDir, indexConfig);
             }
             catch (LockObtainFailedException ex)
             {
-                try
-                {
-                    writer = new IndexWriter(new RAMDirectory(), indexConfig);
-                    (ExecutionEvents.ActiveSession.GetParameterValue(ParameterKeys.Logger) as DynamoLogger).LogError($"LuceneNET LockObtainFailedException {ex}, switching to RAM mode.");
-                }
-                catch(Exception)
-                {
-                    DisposeWriter();
-                }
+
+                DisposeWriter();
+                dynamoModel.Logger.LogError($"LuceneNET LockObtainFailedException {ex}");
+
             }
             catch (Exception ex)
             {
-                (ExecutionEvents.ActiveSession.GetParameterValue(ParameterKeys.Logger) as DynamoLogger).LogError($"LuceneNET Exception {ex}");
+                dynamoModel.Logger.LogError($"LuceneNET Exception {ex}");
             }
         }
 
@@ -392,6 +385,9 @@ internal Analyzer CreateAnalyzerByLanguage(string language)
             }
         }
 
+        /// <summary>
+        /// Dispose Lucene index write objects and reuse other objects
+        /// </summary>
         internal void DisposeWriter()
         {
             writer?.Dispose();
@@ -403,7 +399,7 @@ internal void DisposeWriter()
         /// </summary>
         internal void DisposeAll()
         {
-            writer?.Dispose();
+            DisposeWriter();
             dirReader?.Dispose();
             indexDir?.Dispose();
             Analyzer?.Dispose();
@@ -426,6 +422,16 @@ internal void CommitWriterChanges()
         internal void AddNodeTypeToSearchIndex(NodeSearchElement node, Document doc)
         {
             if (addedFields == null) return;
+            // During DynamoModel initialization, the index writer should still be valid here
+            // If the index writer is null and index not locked, it means the index writer has been disposed, e.g. DynamoModel finished initialization
+            // If the index writer is null and index locked, it means another Dynamo session is currently updating the search index
+            // Try to create a new index writer to amend the index
+            if (writer == null && !IndexWriter.IsLocked(this.indexDir))
+            {
+                CreateLuceneIndexWriter(OpenMode.CREATE_OR_APPEND);
+            }
+            // If the index writer is still null, skip the indexing
+            if (writer == null) return;
 
             SetDocumentFieldValue(doc, nameof(LuceneConfig.NodeFieldsEnum.FullCategoryName), node.FullCategoryName);
             SetDocumentFieldValue(doc, nameof(LuceneConfig.NodeFieldsEnum.Name), node.Name);

diff --git a/src/DynamoCoreWpf/Controls/StartPage.xaml.cs b/src/DynamoCoreWpf/Controls/StartPage.xaml.cs
@@ -4,6 +4,7 @@
 using System.Collections.Specialized;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Windows;
 using System.Windows.Controls;
 using System.Windows.Input;
@@ -370,7 +371,7 @@ private void RefreshFileList(ObservableCollection<StartPageListItem> files,
             IEnumerable<string> filePaths)
         {
             files.Clear();
-            foreach (var filePath in filePaths)
+            foreach (var filePath in filePaths.Where(x => x != null))
             {
                 try
                 {

diff --git a/src/DynamoCoreWpf/ViewModels/Core/NodeViewModel.cs b/src/DynamoCoreWpf/ViewModels/Core/NodeViewModel.cs
@@ -18,7 +18,6 @@
 using Dynamo.Logging;
 using Dynamo.Models;
 using Dynamo.Selection;
-using Dynamo.UI;
 using Dynamo.Wpf.ViewModels.Core;
 using Newtonsoft.Json;
 using Point = System.Windows.Point;

diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs
@@ -1014,6 +1014,8 @@ internal virtual void InstallPackage(PackageDownloadHandle packageDownloadHandle
                         }
                     }
                     SetPackageState(packageDownloadHandle, installPath);
+                    // Dispose Index writer to avoid file lock after new package is installed
+                    Search.LuceneSearch.LuceneUtilityNodeSearch.DisposeWriter();
                     Analytics.TrackEvent(Actions.Installed, Categories.PackageManagerOperations, $"{packageDownloadHandle?.Name}");
                 }
                 catch (Exception e)

diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
@@ -796,14 +796,7 @@ public void RefreshAndSearchAsync()
 
                     if (!DynamoModel.IsTestMode)
                     {
-                        if (LuceneUtility.writer != null)
-                        {
-                            LuceneUtility.dirReader = LuceneUtility.writer.GetReader(applyAllDeletes: true);
-                        }
-                        else
-                        {
-                            LuceneUtility.dirReader = DirectoryReader.Open(LuceneUtility.indexDir);
-                        }
+                        LuceneUtility.dirReader = LuceneUtility.writer != null ? LuceneUtility.writer.GetReader(applyAllDeletes: true) : DirectoryReader.Open(LuceneUtility.indexDir);
                         LuceneUtility.Searcher = new IndexSearcher(LuceneUtility.dirReader);
 
                         LuceneUtility.CommitWriterChanges();
@@ -1140,7 +1133,7 @@ internal IEnumerable<PackageManagerSearchElementViewModel> Search(string searchT
                 var packages = new List<PackageManagerSearchElementViewModel>();
 
                 //The DirectoryReader and IndexSearcher have to be assigned after commiting indexing changes and before executing the Searcher.Search() method,otherwise new indexed info won't be reflected
-                LuceneUtility.dirReader = LuceneUtility.writer?.GetReader(applyAllDeletes: true);
+                LuceneUtility.dirReader = LuceneUtility.writer != null ? LuceneUtility.writer.GetReader(applyAllDeletes: true): DirectoryReader.Open(LuceneUtility.indexDir);
 
                 if (LuceneUtility.Searcher == null && LuceneUtility.dirReader != null)
                 {

diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PackagePathViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PackagePathViewModel.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using Dynamo.Core;
@@ -266,6 +266,8 @@ private void CommitChanges(object param)
                 packageLoader.LoadNewCustomNodesAndPackages(newPaths, customNodeManager);
             }
             packagePathsEnabled.Clear();
+            // Dispose node Index writer to avoid file lock after new package path applied and packages loaded.
+            Search.LuceneSearch.LuceneUtilityNodeSearch.DisposeWriter();
         }
 
         internal void SetPackagesScheduledState(string packagePath, bool packagePathDisabled)

diff --git a/src/DynamoUtilities/PathHelper.cs b/src/DynamoUtilities/PathHelper.cs
@@ -37,6 +37,9 @@ public static Exception CreateFolderIfNotExist(string folderPath)
         {
             try
             {
+                // Do not even try when folder path is null or empty.
+                // This usually happens when system folder dialog is initialized with empty path
+                if (string.IsNullOrEmpty(folderPath)) return null;
                 // When network path is access denied, the Directory.Exits however still 
                 // return true.
                 // EnumerateDirectories operation is additional check

diff --git a/src/PythonMigrationViewExtension/PythonMigrationViewExtension.cs b/src/PythonMigrationViewExtension/PythonMigrationViewExtension.cs
@@ -187,14 +187,17 @@ private void UnSubscribePythonNodeEvents(PythonNodeBase node)
 
         private void UnSubscribeWorkspaceEvents()
         {
-            CurrentWorkspace.RequestPackageDependencies -= PythonDependencies.AddPythonPackageDependency;
-            CurrentWorkspace.NodeAdded -= OnNodeAdded;
-            CurrentWorkspace.NodeRemoved -= OnNodeRemoved;
-            CurrentWorkspace.Nodes
-                .Where(n => n is PythonNode)
-                .Cast<PythonNode>()
-                .ToList()
-                .ForEach(n => UnSubscribePythonNodeEvents(n));
+            if (CurrentWorkspace != null)
+            {
+                CurrentWorkspace.RequestPackageDependencies -= PythonDependencies.AddPythonPackageDependency;
+                CurrentWorkspace.NodeAdded -= OnNodeAdded;
+                CurrentWorkspace.NodeRemoved -= OnNodeRemoved;
+                CurrentWorkspace.Nodes
+                    .Where(n => n is PythonNode)
+                    .Cast<PythonNode>()
+                    .ToList()
+                    .ForEach(n => UnSubscribePythonNodeEvents(n));
+            }
         }
 
         private void UnsubscribeEvents()