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

Lucene index amend improvements #14513

Merged
merged 16 commits into from
Oct 27, 2023
24 changes: 19 additions & 5 deletions src/DynamoCore/Utilities/LuceneSearchUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.IO;
using System.Linq;
using Dynamo.Configuration;
using Dynamo.Core;
using Dynamo.Events;
using Dynamo.Logging;
using Dynamo.Models;
Expand Down Expand Up @@ -95,6 +94,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)
{
Expand Down Expand Up @@ -132,12 +132,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
{
Expand All @@ -147,12 +148,12 @@ internal void CreateLuceneIndexWriter()
{

DisposeWriter();
(ExecutionEvents.ActiveSession.GetParameterValue(ParameterKeys.Logger) as DynamoLogger).LogError($"LuceneNET LockObtainFailedException {ex}");
dynamoModel.Logger.LogError($"LuceneNET LockObtainFailedException {ex}");
Copy link
Contributor Author

@QilongTang QilongTang Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to revert this because I found in edge case if the index is ever locked (although would rarely happen after all the improvements in this PR), the previous code will make Dynamo hang because of threading issues


}
catch (Exception ex)
{
(ExecutionEvents.ActiveSession.GetParameterValue(ParameterKeys.Logger) as DynamoLogger).LogError($"LuceneNET Exception {ex}");
dynamoModel.Logger.LogError($"LuceneNET Exception {ex}");
}
}

Expand Down Expand Up @@ -384,6 +385,9 @@ internal Analyzer CreateAnalyzerByLanguage(string language)
}
}

/// <summary>
/// Dispose Lucene index write objects and reuse other objects
/// </summary>
internal void DisposeWriter()
{
writer?.Dispose();
Expand Down Expand Up @@ -418,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))
reddyashish marked this conversation as resolved.
Show resolved Hide resolved
{
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);
Expand Down
3 changes: 2 additions & 1 deletion src/DynamoCoreWpf/Controls/StartPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is throwing a bunch of exceptions so filtering out the nulls from the list

{
try
{
Expand Down
22 changes: 11 additions & 11 deletions src/DynamoCoreWpf/ViewModels/Core/NodeViewModel.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1417,7 +1417,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)
{
Expand Down Expand Up @@ -1577,7 +1577,6 @@ internal void Close()
InitialResultsLoaded = false; // reset the loading screen settings
RequestShowFileDialog -= OnRequestShowFileDialog;
nonHostFilter.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
LuceneUtility.DisposeAll();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using Dynamo.Core;
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions src/DynamoUtilities/PathHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was throwing exception when folder path is null

// When network path is access denied, the Directory.Exits however still
// return true.
// EnumerateDirectories operation is additional check
Expand Down
Loading