Lucene index amend improvements (#14513)

QilongTang · web-flow · commit 4f279a91fe17 · 2023-10-27T14:27:42.000-04:00
* 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
diff --git a/src/DynamoCore/Search/NodeSearchModel.cs b/src/DynamoCore/Search/NodeSearchModel.cs
@@ -237,14 +237,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)
         {
@@ -132,12 +129,13 @@ 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
             {
@@ -147,12 +145,12 @@ internal void CreateLuceneIndexWriter()
             {
 
                 DisposeWriter();
-                (ExecutionEvents.ActiveSession.GetParameterValue(ParameterKeys.Logger) as DynamoLogger).LogError($"LuceneNET LockObtainFailedException {ex}");
+                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}");
             }
         }
 
@@ -384,6 +382,9 @@ internal Analyzer CreateAnalyzerByLanguage(string language)
             }
         }
 
+        /// <summary>
+        /// Dispose Lucene index write objects and reuse other objects
+        /// </summary>
         internal void DisposeWriter()
         {
             writer?.Dispose();
@@ -395,7 +396,7 @@ internal void DisposeWriter()
         /// </summary>
         internal void DisposeAll()
         {
-            writer?.Dispose();
+            DisposeWriter();
             dirReader?.Dispose();
             indexDir?.Dispose();
             Analyzer?.Dispose();
@@ -418,6 +419,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;
@@ -366,7 +367,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
@@ -1,14 +1,3 @@
-using Dynamo.Configuration;
-using Dynamo.Engine.CodeGeneration;
-using Dynamo.Graph;
-using Dynamo.Graph.Nodes;
-using Dynamo.Graph.Nodes.CustomNodes;
-using Dynamo.Graph.Workspaces;
-using Dynamo.Logging;
-using Dynamo.Models;
-using Dynamo.Selection;
-using Dynamo.Wpf.ViewModels.Core;
-using Newtonsoft.Json;
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
@@ -20,6 +9,17 @@
 using System.Windows;
 using System.Windows.Controls.Primitives;
 using System.Windows.Media;
+using Dynamo.Configuration;
+using Dynamo.Engine.CodeGeneration;
+using Dynamo.Graph;
+using Dynamo.Graph.Nodes;
+using Dynamo.Graph.Nodes.CustomNodes;
+using Dynamo.Graph.Workspaces;
+using Dynamo.Logging;
+using Dynamo.Models;
+using Dynamo.Selection;
+using Dynamo.Wpf.ViewModels.Core;
+using Newtonsoft.Json;
 using Point = System.Windows.Point;
 using Size = System.Windows.Size;
 
diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs
@@ -1026,6 +1026,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
@@ -1028,14 +1028,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();
@@ -1417,7 +1410,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)
                 {
@@ -1577,7 +1570,6 @@ internal void Close()
             InitialResultsLoaded = false;   // reset the loading screen settings
             RequestShowFileDialog -= OnRequestShowFileDialog;
             nonHostFilter.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
-            LuceneUtility.DisposeAll();
         }
     }
 }
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()

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`using System.Collections.Specialized;`
`5`	`5`	`using System.Diagnostics;`
`6`	`6`	`using System.IO;`
	`7`	`+using System.Linq;`
`7`	`8`	`using System.Windows;`
`8`	`9`	`using System.Windows.Controls;`
`9`	`10`	`using System.Windows.Input;`
`@@ -366,7 +367,7 @@ private void RefreshFileList(ObservableCollection<StartPageListItem> files,`
`366`	`367`	`IEnumerable<string> filePaths)`
`367`	`368`	`{`
`368`	`369`	`files.Clear();`
`369`		`- foreach (var filePath in filePaths)`
	`370`	`+ foreach (var filePath in filePaths.Where(x => x != null))`
`370`	`371`	`{`
`371`	`372`	`try`
`372`	`373`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1026,6 +1026,8 @@ internal virtual void InstallPackage(PackageDownloadHandle packageDownloadHandle`
`1026`	`1026`	`}`
`1027`	`1027`	`}`
`1028`	`1028`	`SetPackageState(packageDownloadHandle, installPath);`
	`1029`	`+ // Dispose Index writer to avoid file lock after new package is installed`
	`1030`	`+ Search.LuceneSearch.LuceneUtilityNodeSearch.DisposeWriter();`
`1029`	`1031`	`Analytics.TrackEvent(Actions.Installed, Categories.PackageManagerOperations, $"{packageDownloadHandle?.Name}");`
`1030`	`1032`	`}`
`1031`	`1033`	`catch (Exception e)`
Original file line number	Diff line number	Diff line change
`@@ -1028,14 +1028,7 @@ public void RefreshAndSearchAsync()`
`1028`	`1028`
`1029`	`1029`	`if (!DynamoModel.IsTestMode)`
`1030`	`1030`	`{`
`1031`		`- if (LuceneUtility.writer != null)`
`1032`		`- {`
`1033`		`- LuceneUtility.dirReader = LuceneUtility.writer.GetReader(applyAllDeletes: true);`
`1034`		`- }`
`1035`		`- else`
`1036`		`- {`
`1037`		`- LuceneUtility.dirReader = DirectoryReader.Open(LuceneUtility.indexDir);`
`1038`		`- }`
	`1031`	`+ LuceneUtility.dirReader = LuceneUtility.writer != null ? LuceneUtility.writer.GetReader(applyAllDeletes: true) : DirectoryReader.Open(LuceneUtility.indexDir);`
`1039`	`1032`	`LuceneUtility.Searcher = new IndexSearcher(LuceneUtility.dirReader);`
`1040`	`1033`
`1041`	`1034`	`LuceneUtility.CommitWriterChanges();`
`@@ -1417,7 +1410,7 @@ internal IEnumerable<PackageManagerSearchElementViewModel> Search(string searchT`
`1417`	`1410`	`var packages = new List<PackageManagerSearchElementViewModel>();`
`1418`	`1411`
`1419`	`1412`	`//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`
`1420`		`- LuceneUtility.dirReader = LuceneUtility.writer?.GetReader(applyAllDeletes: true);`
	`1413`	`+ LuceneUtility.dirReader = LuceneUtility.writer != null ? LuceneUtility.writer.GetReader(applyAllDeletes: true): DirectoryReader.Open(LuceneUtility.indexDir);`
`1421`	`1414`
`1422`	`1415`	`if (LuceneUtility.Searcher == null && LuceneUtility.dirReader != null)`
`1423`	`1416`	`{`
`@@ -1577,7 +1570,6 @@ internal void Close()`
`1577`	`1570`	`InitialResultsLoaded = false; // reset the loading screen settings`
`1578`	`1571`	`RequestShowFileDialog -= OnRequestShowFileDialog;`
`1579`	`1572`	`nonHostFilter.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);`
`1580`		`- LuceneUtility.DisposeAll();`
`1581`	`1573`	`}`
`1582`	`1574`	`}`
`1583`	`1575`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using System;`
	`1`	`+using System;`
`2`	`2`	`using System.Collections.Generic;`
`3`	`3`	`using System.Collections.ObjectModel;`
`4`	`4`	`using Dynamo.Core;`
`@@ -266,6 +266,8 @@ private void CommitChanges(object param)`
`266`	`266`	`packageLoader.LoadNewCustomNodesAndPackages(newPaths, customNodeManager);`
`267`	`267`	`}`
`268`	`268`	`packagePathsEnabled.Clear();`
	`269`	`+ // Dispose node Index writer to avoid file lock after new package path applied and packages loaded.`
	`270`	`+ Search.LuceneSearch.LuceneUtilityNodeSearch.DisposeWriter();`
`269`	`271`	`}`
`270`	`272`
`271`	`273`	`internal void SetPackagesScheduledState(string packagePath, bool packagePathDisabled)`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,9 @@ public static Exception CreateFolderIfNotExist(string folderPath)`
`37`	`37`	`{`
`38`	`38`	`try`
`39`	`39`	`{`
	`40`	`+ // Do not even try when folder path is null or empty.`
	`41`	`+ // This usually happens when system folder dialog is initialized with empty path`
	`42`	`+ if (string.IsNullOrEmpty(folderPath)) return null;`
`40`	`43`	`// When network path is access denied, the Directory.Exits however still`
`41`	`44`	`// return true.`
`42`	`45`	`// EnumerateDirectories operation is additional check`