Realizando el bloqueo en ASP.NET correctamente

Question

Tengo un sitio ASP.NET con una función de búsqueda bastante lenta, y quiero mejorar el rendimiento añadiendo los resultados al caché durante una hora usando la consulta como clave de caché:

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!"; 
    } 
} 

Digamos que el visitante A hace una búsqueda. La memoria caché está vacía, se establece el bloqueo y se solicita el resultado de la base de datos. Ahora el visitante B viene con una búsqueda diferente: ¿No tendrá que esperar el visitante B hasta que se complete la búsqueda del visitante A? Lo que realmente quería era que B llamara a la base de datos de inmediato, porque los resultados serán diferentes y la base de datos puede manejar múltiples solicitudes; simplemente no quiero repetir costosas consultas innecesarias.

¿Cuál sería el enfoque correcto para este escenario?

Answer 1

A menos que esté absolutamente seguro de que es fundamental no tener consultas redundantes entonces me gustaría evitar el bloqueo completo. La memoria caché de ASP.NET es inherentemente seguro para subprocesos, por lo que el único inconveniente para el siguiente código es que es posible que vea temporalmente algunas consultas redundantes compitiendo entre sí cuando su entrada de caché asociada expira:

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; 
} 

Si decide que que realmente debe evitar todas las consultas redundantes entonces se podría usar un juego de esclusas más granulares, una cerradura por consulta:

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

Tu código es correcto. También está usando double-if-sandwitching-lock que evitará condiciones de carrera que es una trampa común cuando no se utiliza. Esto no bloqueará el acceso a las cosas existentes en la memoria caché.

El único problema es cuando muchos clientes están insertando en la caché al mismo tiempo, y se pondrá en cola detrás de la cerradura, pero lo que me gustaría hacer es poner el results = GetResultsFromSlowDb(query); fuera de la cerradura:

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(); 
    } 

Si esto es lento, tu problema está en otra parte.

Answer 3

el código tiene una condición de carrera potencial:

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();   
} 

En general yo no usaría bloqueo, y lo haría de la siguiente manera para evitar la condición de carrera:

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

En lo anterior caso, varios subprocesos pueden intentar cargar datos si detectan una falta de caché al mismo tiempo. En la práctica, es probable que esto sea raro y, en la mayoría de los casos, no importante, ya que los datos que cargan serán equivalentes.

Pero si desea utilizar una cerradura para evitar que se lo puede hacer de la siguiente manera:

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;