From fde8642cfd058434263f76d2756c0aa122f8501f Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Thu, 15 Jun 2023 10:50:35 -0700 Subject: [PATCH] Resolve Multi-Restore Deadlock (#6348) * amend so that a single queue for all assets.jsons are NOT shared. instead it is a single task queue PER assets.json being restored --- .../Store/GitStore.cs | 60 ++++++++++++------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs index e81b36d3272..030572103d1 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Store/GitStore.cs @@ -43,6 +43,21 @@ public class GitStore : IAssetsStore public GitStoreBreadcrumb BreadCrumb = new GitStoreBreadcrumb(); + /// + /// We need to lock repo inititialization behind a queue. + /// This is due to the fact that Restore() can be called from multiple parallel + /// requests, as multiple "startplayback" can be firing at the same time. + /// + /// While the Restore() action itself is idempotent, the Initialization of the assets repo + /// is NOT. We will use this queue to force ONE single initialization at a time. + /// + /// We don't want to gate ALL initializations behind the same gate though. We can restore + /// multiple DIFFERENT assets.jsons at the same time. It's specifically when two restores for the SAME + /// assets.json are fired that we run into problems. + /// + /// Everything else will still run in parallel. + /// + private ConcurrentDictionary InitTasks = new ConcurrentDictionary(); public ConcurrentDictionary Assets = new ConcurrentDictionary(); @@ -453,34 +468,39 @@ public bool IsAssetsRepoInitialized(GitAssetsConfiguration config) /// public bool InitializeAssetsRepo(GitAssetsConfiguration config, bool forceInit = false) { - var assetRepo = config.AssetsRepoLocation; - var initialized = IsAssetsRepoInitialized(config); var workCompleted = false; + var initQueue = InitTasks.GetOrAdd(config.AssetsRepoLocation, new TaskQueue()); - if (forceInit) + initQueue.Enqueue(() => { - DirectoryHelper.DeleteGitDirectory(assetRepo.ToString()); - Directory.CreateDirectory(assetRepo.ToString()); - initialized = false; - } + var assetRepo = config.AssetsRepoLocation; + var initialized = IsAssetsRepoInitialized(config); - if (!initialized) - { - try + if (forceInit) { - var cloneUrl = GetCloneUrl(config.AssetsRepo, config.RepoRoot); - // The -c core.longpaths=true is basically for Windows and is a noop for other platforms - GitHandler.Run($"clone -c core.longpaths=true --no-checkout --filter=tree:0 {cloneUrl} .", config); - GitHandler.Run($"sparse-checkout init", config); + DirectoryHelper.DeleteGitDirectory(assetRepo.ToString()); + Directory.CreateDirectory(assetRepo.ToString()); + initialized = false; } - catch(GitProcessException e) + + if (!initialized) { - throw GenerateInvokeException(e.Result); - } + try + { + var cloneUrl = GetCloneUrl(config.AssetsRepo, config.RepoRoot); + // The -c core.longpaths=true is basically for Windows and is a noop for other platforms + GitHandler.Run($"clone -c core.longpaths=true --no-checkout --filter=tree:0 {cloneUrl} .", config); + GitHandler.Run($"sparse-checkout init", config); + } + catch (GitProcessException e) + { + throw GenerateInvokeException(e.Result); + } - CheckoutRepoAtConfig(config, cleanEnabled: false); - workCompleted = true; - } + CheckoutRepoAtConfig(config, cleanEnabled: false); + workCompleted = true; + } + }); return workCompleted; }