Etiquetas
Ayer a última hora me encontré con este código:
Quité algunas líneas para que no se pudiese deducir la empresa, que no es importante. Tampoco el programador que lo hizo, no es mi intención criticar a nadie en particular sino a una mala práctica de solucionar problemas de concurrencia con «ideas geniales» que no lo son tanta.
El código anterior recibe consultas B2B, graba los resultados en un fichero (por requerimientos de negocio) y luego lee desde esos mismos ficheros para enviar la respuesta a la agencia que hizo la consulta.
Es un software que factura millones de euros al año y pasó semanas de tests y QA. No se descubrió el problema hasta que una de los clientes optimizó sus consultas enviando varias en paralelo. Allí descubrieron que obtenían resultados erróneos: eran duplicados (con un riesgo financiero bastante importante).
Moraleja 1: Los errores de concurrencia son difíciles de encontrar y cuando aparecen suelen ser difíciles de debuguear, ten mucho cuidado antes de implementar soluciones ad-hoc.
El código intenta resolver el problema de concurrencia de varios procesos que podrían escribir en el mismo fichero verificando primero si el fichero existe, si es así incrementa el contador -que forma parte del nombre- hasta que encuentra uno que no coincida.
Además de detalles de eficiencia tiene un problema grave, las operaciones de verificación (file_exists) y creación del fichero (fopen) no son atómicas. Eso significa que varios procesos concurrentes (con una o varias CPUs) verifican, les da falso a todos y luego todos escriben en el mismo fichero. Una race condition de libro.
Moraleja 2: Si has tardado en encontrar el problema o no entiendes la importancia de operaciones atómicas ya deberías estar aprendiendo más de concurrencia.
Afortunadamente entre los tres que estábamos analizando el código en plan «emergencia» y por vídeoconferencia (la captura es del chat de Skype de Linux) desde países diferentes nos dimos cuenta enseguida del bug y de una solución simple: agregar el PID del proceso al nombre del fichero. Como el PID es único en el sistema y cada proceso individual es secuencial no hay riesgos de que sobreescriban en el mismo fichero.
No es la solución más eficiente (ya lo corregiremos estos días con más tranquilidad) pero es una solución segura y que podía implementarse en pocos minutos y sin efectos colaterales que podrían afectar a la lógica del programa.
Además, el bug podría haber sido menos perjudicial si en vez de escribir los resultados en el fichero y luego leerlos para responder se hubiese enviado directamente la respuesta ya almacenada en el proceso local.
Moraleja 3: Si no te has dado cuenta de la diferencia entre datos locales y compartidos y los problemas que pueden generar estos últimos, ponte ya a aprender de concurrencia.
Moraleja 4: Los problemas de concurrencia son muchos más habituales de los que crees y las soluciones no son tan obvias como parecen a primera vista.
Obviamente tengo que recomendar el libro que ya sabéis (y aparece a la izquierda 😉 ) pero ni aún así se puede asegurar que no cometerás errores, se requiere práctica y ser muy cuidadoso.
Ricardo Galli, de software 