mirror of
https://github.com/ckaczor/sqltoolsservice.git
synced 2026-01-14 01:25:40 -05:00
Remove extra level of tasks in binding queue (#79)
* Remove extra layer of tasks in binding queue * Change order of assigning result to avoid race condition * Add timeout log for the metadata lock event * Fix test cases
This commit is contained in:
@@ -4,7 +4,6 @@
|
||||
//
|
||||
|
||||
using System.Collections.Generic;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.SqlServer.Management.SqlParser.Binder;
|
||||
using Microsoft.SqlServer.Management.SqlParser.Intellisense;
|
||||
using Microsoft.SqlServer.Management.SqlParser.Parser;
|
||||
@@ -22,6 +21,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
/// </summary>
|
||||
public static class AutoCompleteHelper
|
||||
{
|
||||
private const int PrepopulateBindTimeout = 60000;
|
||||
|
||||
private static WorkspaceService<SqlToolsSettings> workspaceServiceInstance;
|
||||
|
||||
private static readonly string[] DefaultCompletionText = new string[]
|
||||
@@ -579,6 +580,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
|
||||
QueueItem queueItem = bindingQueue.QueueBindingOperation(
|
||||
key: scriptInfo.ConnectionKey,
|
||||
bindingTimeout: AutoCompleteHelper.PrepopulateBindTimeout,
|
||||
bindOperation: (bindingContext, cancelToken) =>
|
||||
{
|
||||
// parse a simple statement that returns common metadata
|
||||
@@ -619,7 +621,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
|
||||
// this forces lazy evaluation of the suggestion metadata
|
||||
AutoCompleteHelper.ConvertDeclarationsToCompletionItems(suggestions, 1, 6, 6);
|
||||
return Task.FromResult(null as object);
|
||||
return null;
|
||||
});
|
||||
|
||||
queueItem.ItemProcessed.WaitOne();
|
||||
|
||||
@@ -59,8 +59,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
/// </summary>
|
||||
public QueueItem QueueBindingOperation(
|
||||
string key,
|
||||
Func<IBindingContext, CancellationToken, Task<object>> bindOperation,
|
||||
Func<IBindingContext, Task<object>> timeoutOperation = null,
|
||||
Func<IBindingContext, CancellationToken, object> bindOperation,
|
||||
Func<IBindingContext, object> timeoutOperation = null,
|
||||
int? bindingTimeout = null)
|
||||
{
|
||||
// don't add null operations to the binding queue
|
||||
@@ -184,53 +184,48 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
continue;
|
||||
}
|
||||
|
||||
try
|
||||
IBindingContext bindingContext = GetOrCreateBindingContext(queueItem.Key);
|
||||
if (bindingContext == null)
|
||||
{
|
||||
IBindingContext bindingContext = GetOrCreateBindingContext(queueItem.Key);
|
||||
if (bindingContext == null)
|
||||
{
|
||||
queueItem.ItemProcessed.Set();
|
||||
continue;
|
||||
}
|
||||
|
||||
queueItem.ItemProcessed.Set();
|
||||
continue;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
// prefer the queue item binding item, otherwise use the context default timeout
|
||||
int bindTimeout = queueItem.BindingTimeout ?? bindingContext.BindingTimeout;
|
||||
|
||||
// handle the case a previous binding operation is still running
|
||||
|
||||
// handle the case a previous binding operation is still running
|
||||
if (!bindingContext.BindingLocked.WaitOne(bindTimeout))
|
||||
{
|
||||
queueItem.ResultsTask = Task.Run(() =>
|
||||
{
|
||||
var timeoutTask = queueItem.TimeoutOperation(bindingContext);
|
||||
timeoutTask.ContinueWith((obj) => queueItem.ItemProcessed.Set());
|
||||
return timeoutTask.Result;
|
||||
});
|
||||
|
||||
queueItem.Result = queueItem.TimeoutOperation(bindingContext);
|
||||
queueItem.ItemProcessed.Set();
|
||||
continue;
|
||||
}
|
||||
|
||||
// execute the binding operation
|
||||
CancellationTokenSource cancelToken = new CancellationTokenSource();
|
||||
queueItem.ResultsTask = queueItem.BindOperation(
|
||||
bindingContext,
|
||||
cancelToken.Token);
|
||||
|
||||
// set notification events once the binding operation task completes
|
||||
queueItem.ResultsTask.ContinueWith((obj) =>
|
||||
{
|
||||
queueItem.ItemProcessed.Set();
|
||||
bindingContext.BindingLocked.Set();
|
||||
});
|
||||
object result = null;
|
||||
CancellationTokenSource cancelToken = new CancellationTokenSource();
|
||||
var bindTask = Task.Run(() =>
|
||||
{
|
||||
result = queueItem.BindOperation(
|
||||
bindingContext,
|
||||
cancelToken.Token);
|
||||
});
|
||||
|
||||
// check if the binding tasks completed within the binding timeout
|
||||
if (!queueItem.ResultsTask.Wait(bindTimeout))
|
||||
if (bindTask.Wait(bindTimeout))
|
||||
{
|
||||
queueItem.Result = result;
|
||||
}
|
||||
else
|
||||
{
|
||||
// if the task didn't complete then call the timeout callback
|
||||
if (queueItem.TimeoutOperation != null)
|
||||
{
|
||||
cancelToken.Cancel();
|
||||
queueItem.ResultsTask = queueItem.TimeoutOperation(bindingContext);
|
||||
queueItem.ResultsTask.ContinueWith((obj) => queueItem.ItemProcessed.Set());
|
||||
queueItem.Result = queueItem.TimeoutOperation(bindingContext);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -238,7 +233,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
{
|
||||
// catch and log any exceptions raised in the binding calls
|
||||
// set item processed to avoid deadlocks
|
||||
Logger.Write(LogLevel.Error, "Binding queue threw exception " + ex.ToString());
|
||||
Logger.Write(LogLevel.Error, "Binding queue threw exception " + ex.ToString());
|
||||
}
|
||||
finally
|
||||
{
|
||||
bindingContext.BindingLocked.Set();
|
||||
queueItem.ItemProcessed.Set();
|
||||
}
|
||||
|
||||
|
||||
@@ -39,7 +39,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
|
||||
internal const int HoverTimeout = 3000;
|
||||
|
||||
internal const int FindCompletionsTimeout = 3000;
|
||||
internal const int BindingTimeout = 3000;
|
||||
|
||||
internal const int FindCompletionStartTimeout = 50;
|
||||
|
||||
@@ -452,7 +452,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
// get or create the current parse info object
|
||||
ScriptParseInfo parseInfo = GetScriptParseInfo(scriptFile.ClientFilePath, createIfNotExists: true);
|
||||
|
||||
if (parseInfo.BuildingMetadataEvent.WaitOne(LanguageService.FindCompletionsTimeout))
|
||||
if (parseInfo.BuildingMetadataEvent.WaitOne(LanguageService.BindingTimeout))
|
||||
{
|
||||
try
|
||||
{
|
||||
@@ -472,6 +472,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
{
|
||||
QueueItem queueItem = this.BindingQueue.QueueBindingOperation(
|
||||
key: parseInfo.ConnectionKey,
|
||||
bindingTimeout: LanguageService.BindingTimeout,
|
||||
bindOperation: (bindingContext, cancelToken) =>
|
||||
{
|
||||
try
|
||||
@@ -503,7 +504,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
Logger.Write(LogLevel.Error, "Unknown exception during parsing " + ex.ToString());
|
||||
}
|
||||
|
||||
return Task.FromResult(null as object);
|
||||
return null;
|
||||
});
|
||||
|
||||
queueItem.ItemProcessed.WaitOne();
|
||||
@@ -520,6 +521,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
parseInfo.BuildingMetadataEvent.Set();
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
Logger.Write(LogLevel.Warning, "Binding metadata lock timeout in ParseAndBind");
|
||||
}
|
||||
|
||||
return parseInfo.ParseResult;
|
||||
}
|
||||
@@ -644,13 +649,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
bindingContext.MetadataDisplayInfoProvider);
|
||||
|
||||
// convert from the parser format to the VS Code wire format
|
||||
return Task.FromResult(
|
||||
AutoCompleteHelper.ConvertQuickInfoToHover(
|
||||
return AutoCompleteHelper.ConvertQuickInfoToHover(
|
||||
quickInfo,
|
||||
startLine,
|
||||
startColumn,
|
||||
endColumn
|
||||
) as object);
|
||||
endColumn);
|
||||
});
|
||||
|
||||
queueItem.ItemProcessed.WaitOne();
|
||||
@@ -714,6 +717,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
|
||||
QueueItem queueItem = this.BindingQueue.QueueBindingOperation(
|
||||
key: scriptParseInfo.ConnectionKey,
|
||||
bindingTimeout: LanguageService.BindingTimeout,
|
||||
bindOperation: (bindingContext, cancelToken) =>
|
||||
{
|
||||
CompletionItem[] completions = null;
|
||||
@@ -741,12 +745,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
scriptParseInfo.BuildingMetadataEvent.Set();
|
||||
}
|
||||
|
||||
return Task.FromResult(completions as object);
|
||||
return completions;
|
||||
},
|
||||
timeoutOperation: (bindingContext) =>
|
||||
{
|
||||
return Task.FromResult(
|
||||
AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions) as object);
|
||||
return AutoCompleteHelper.GetDefaultCompletionItems(startLine, startColumn, endColumn, useLowerCaseSuggestions);
|
||||
});
|
||||
|
||||
queueItem.ItemProcessed.WaitOne();
|
||||
|
||||
@@ -30,12 +30,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
/// <summary>
|
||||
/// Gets or sets the bind operation callback method
|
||||
/// </summary>
|
||||
public Func<IBindingContext, CancellationToken, Task<object>> BindOperation { get; set; }
|
||||
public Func<IBindingContext, CancellationToken, object> BindOperation { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets the timeout operation to call if the bind operation doesn't finish within timeout period
|
||||
/// </summary>
|
||||
public Func<IBindingContext, Task<object>> TimeoutOperation { get; set; }
|
||||
public Func<IBindingContext, object> TimeoutOperation { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets an event to signal when this queue item has been processed
|
||||
@@ -43,10 +43,9 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
public ManualResetEvent ItemProcessed { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets the task that was used to execute this queue item.
|
||||
/// This allows the queuer to retrieve the execution result.
|
||||
/// Gets or sets the result of the queued task
|
||||
/// </summary>
|
||||
public Task<object> ResultsTask { get; set; }
|
||||
public object Result { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets or sets the binding operation timeout in milliseconds
|
||||
@@ -54,13 +53,13 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
|
||||
public int? BindingTimeout { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Converts the result of the execution task to type T
|
||||
/// Converts the result of the execution to type T
|
||||
/// </summary>
|
||||
public T GetResultAsT<T>() where T : class
|
||||
{
|
||||
var task = this.ResultsTask;
|
||||
return (task != null && task.IsCompleted && task.Result != null)
|
||||
? task.Result as T
|
||||
//var task = this.ResultsTask;
|
||||
return (this.Result != null)
|
||||
? this.Result as T
|
||||
: null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,45 +89,29 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices
|
||||
/// <summary>
|
||||
/// Test bind operation callback
|
||||
/// </summary>
|
||||
private Task<object> TestBindOperation(
|
||||
private object TestBindOperation(
|
||||
IBindingContext bindContext,
|
||||
CancellationToken cancelToken)
|
||||
{
|
||||
return Task.Run(() =>
|
||||
{
|
||||
cancelToken.WaitHandle.WaitOne(this.bindCallbackDelay);
|
||||
this.isCancelationRequested = cancelToken.IsCancellationRequested;
|
||||
if (!this.isCancelationRequested)
|
||||
{
|
||||
cancelToken.WaitHandle.WaitOne(this.bindCallbackDelay);
|
||||
this.isCancelationRequested = cancelToken.IsCancellationRequested;
|
||||
if (!this.isCancelationRequested)
|
||||
{
|
||||
++this.bindCallCount;
|
||||
}
|
||||
return new CompletionItem[0] as object;
|
||||
});
|
||||
++this.bindCallCount;
|
||||
}
|
||||
return new CompletionItem[0];
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Test callback for the bind timeout operation
|
||||
/// </summary>
|
||||
private Task<object> TestTimeoutOperation(
|
||||
private object TestTimeoutOperation(
|
||||
IBindingContext bindingContext)
|
||||
{
|
||||
++this.timeoutCallCount;
|
||||
return Task.FromResult(new CompletionItem[0] as object);
|
||||
return new CompletionItem[0];
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Runs for a few seconds to allow the queue to pump any requests
|
||||
/// </summary>
|
||||
private void WaitForQueue(int delay = 5000)
|
||||
{
|
||||
int step = 50;
|
||||
int steps = delay / step + 1;
|
||||
for (int i = 0; i < steps; ++i)
|
||||
{
|
||||
Thread.Sleep(step);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Queues a single task
|
||||
/// </summary>
|
||||
@@ -141,7 +125,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices
|
||||
bindOperation: TestBindOperation,
|
||||
timeoutOperation: TestTimeoutOperation);
|
||||
|
||||
WaitForQueue();
|
||||
Thread.Sleep(1000);
|
||||
|
||||
this.bindingQueue.StopQueueProcessor(15000);
|
||||
|
||||
@@ -166,7 +150,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices
|
||||
timeoutOperation: TestTimeoutOperation);
|
||||
}
|
||||
|
||||
WaitForQueue();
|
||||
Thread.Sleep(2000);
|
||||
|
||||
this.bindingQueue.StopQueueProcessor(15000);
|
||||
|
||||
@@ -183,14 +167,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.LanguageServices
|
||||
{
|
||||
InitializeTestSettings();
|
||||
|
||||
this.bindCallbackDelay = 10000;
|
||||
this.bindCallbackDelay = 1000;
|
||||
|
||||
this.bindingQueue.QueueBindingOperation(
|
||||
key: "testkey",
|
||||
bindingTimeout: bindCallbackDelay / 2,
|
||||
bindOperation: TestBindOperation,
|
||||
timeoutOperation: TestTimeoutOperation);
|
||||
|
||||
WaitForQueue(this.bindCallbackDelay + 2000);
|
||||
Thread.Sleep(this.bindCallbackDelay + 100);
|
||||
|
||||
this.bindingQueue.StopQueueProcessor(15000);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user