Effettuare il blocco in ASP.NET correttamente

Question

Ho un sito ASP.NET con una funzione di ricerca abbastanza lenta e voglio migliorare le prestazioni aggiungendo i risultati alla cache per un'ora utilizzando la query come chiave di cache:

using System; 
using System.Web; 
using System.Web.Caching; 

public class Search 
{ 
    private static object _cacheLock = new object(); 

    public static string DoSearch(string query) 
    { 
     string results = ""; 

     if (HttpContext.Current.Cache[query] == null) 
     { 
      lock (_cacheLock) 
      { 
       if (HttpContext.Current.Cache[query] == null) 
       { 
        results = GetResultsFromSlowDb(query); 

        HttpContext.Current.Cache.Add(query, results, null, DateTime.Now.AddHours(1), Cache.NoSlidingExpiration, CacheItemPriority.Normal, null); 
       } 
       else 
       { 
        results = HttpContext.Current.Cache[query].ToString(); 
       } 
      } 
     } 
     else 
     { 
      results = HttpContext.Current.Cache[query].ToString(); 
     } 

     return results; 
    } 

    private static string GetResultsFromSlowDb(string query) 
    { 
     return "Hello World!"; 
    } 
} 

Diciamo che il visitatore A esegue una ricerca. La cache è vuota, il blocco è impostato e il risultato è richiesto dal database. Ora il visitatore B arriva con una ricerca diversa: il visitatore B non dovrà aspettare per il blocco finché la ricerca del visitatore A non sarà completata? Quello che volevo era che B chiamasse immediatamente il database, perché i risultati sarebbero diversi e il database in grado di gestire più richieste - semplicemente non voglio ripetere costose query non necessarie.

Quale sarebbe l'approccio corretto per questo scenario?

Answer 1

A meno che siate assolutamente certi che è fondamentale per non avere domande ridondanti quindi vorrei evitare di bloccare del tutto. La cache di ASP.NET è intrinsecamente thread-safe, quindi l'unico inconveniente di questo codice è che si potrebbe vedere temporaneamente un paio di domande ridondanti correre a vicenda quando il loro ingresso cache associata scade:

public static string DoSearch(string query) 
{ 
    var results = (string)HttpContext.Current.Cache[query]; 
    if (results == null) 
    { 
     results = GetResultsFromSlowDb(query); 

     HttpContext.Current.Cache.Insert(query, results, null, 
      DateTime.Now.AddHours(1), Cache.NoSlidingExpiration); 
    } 
    return results; 
} 

Se si decide che è davvero necessario evitare di tutte le query ridondanti allora si potrebbe utilizzare una serie di serrature più granulari, una serratura per query:

public static string DoSearch(string query) 
{ 
    var results = (string)HttpContext.Current.Cache[query]; 
    if (results == null) 
    { 
     object miniLock = _miniLocks.GetOrAdd(query, k => new object()); 
     lock (miniLock) 
     { 
      results = (string)HttpContext.Current.Cache[query]; 
      if (results == null) 
      { 
       results = GetResultsFromSlowDb(query); 

       HttpContext.Current.Cache.Insert(query, results, null, 
        DateTime.Now.AddHours(1), Cache.NoSlidingExpiration); 
      } 

      object temp; 
      if (_miniLocks.TryGetValue(query, out temp) && (temp == miniLock)) 
       _miniLocks.TryRemove(query); 
     } 
    } 
    return results; 
} 

private static readonly ConcurrentDictionary<string, object> _miniLocks = 
            new ConcurrentDictionary<string, object>(); 

Answer 2

Il tuo codice è corretto. Stai anche usando double-if-sandwitching-lock che impedirà le condizioni di gara che è una trappola comune quando non utilizzato. Questo non bloccherà l'accesso alle cose esistenti nella cache.

L'unico problema è quando molti clienti stanno inserendo nella cache, allo stesso tempo, e saranno in coda dietro la serratura, ma quello che vorrei fare è di mettere il results = GetResultsFromSlowDb(query); al di fuori del blocco:

public static string DoSearch(string query) 
{ 
    string results = ""; 

    if (HttpContext.Current.Cache[query] == null) 
    { 
     results = GetResultsFromSlowDb(query); // HERE 
     lock (_cacheLock) 
     { 
      if (HttpContext.Current.Cache[query] == null) 
      { 


       HttpContext.Current.Cache.Add(query, results, null, DateTime.Now.AddHours(1), Cache.NoSlidingExpiration, CacheItemPriority.Normal, null); 
      } 
      else 
      { 
       results = HttpContext.Current.Cache[query].ToString(); 
      } 
     } 
    } 
    else 
    { 
     results = HttpContext.Current.Cache[query].ToString(); 
    } 

Se questo è lento, il tuo problema è altrove.

Answer 3

Il codice ha un potenziale condizione di gara:

if (HttpContext.Current.Cache[query] == null)   
{ 
    ... 
}   
else   
{ 
    // When you get here, another thread may have removed the item from the cache 
    // so this may still return null. 
    results = HttpContext.Current.Cache[query].ToString();   
} 

In generale non vorrei utilizzare il blocco, e lo avrebbe fatto nel seguente modo per evitare la condizione di competizione:

results = HttpContext.Current.Cache[query]; 
if (HttpContext.Current.Cache[query] == null)   
{ 
    results = GetResultsFromSomewhere(); 
    HttpContext.Current.Cache.Add(query, results,...); 
} 
return results; 

In precedenza caso, più thread potrebbero tentare di caricare i dati se rilevano un errore di cache all'incirca nello stesso momento. In pratica, è probabile che sia raro, e nella maggior parte dei casi non importante, perché i dati che caricano saranno equivalenti.

Ma se si desidera utilizzare un blocco per evitare che si può fare in modo come segue:

results = HttpContext.Current.Cache[query]; 
if (results == null)   
{ 
    lock(someLock) 
    { 
     results = HttpContext.Current.Cache[query]; 
     if (results == null) 
     { 
      results = GetResultsFromSomewhere(); 
      HttpContext.Current.Cache.Add(query, results,...); 
     }   
    } 
} 
return results;