Java Smells: SQL programático

Os voy a enseñar código irritante:

private void writeHtml(Writer writer) throws IOException{
    writer.write("<html>");
    writer.write("<head>");
    // ...
    writer.write("</head>");
    writer.write("<body>");
    writer.write("some html output");
    writer.write("</body>");
    writer.write("</html>");
}

La mitad de vosotros estáis pensando ¿WTF? ¿Qué hace todo ese código html ahí en medio? ¿Por qué no está usando Velocity? ¿Por qué no está usando Mustache? ¿Por qué no está usando [inserte aquí un motor de plantillas]?. La otra mitad de vosotros que ve en esto algo normal, por favor, circulen.

Encontrarnos esto nos irrita porque mezclando diferentes lenguajes dentro de un mismo fuente complica el actualizar una parte (o la parte java , o la parte html), y asume que el desarrollador de uno de los lenguajes es el responsable del desarrollo del otro, cosa que aunque a veces es correcto, es una mala suposición que solo nos puede traer desgracias.

Sin embargo, por alguna razón que se me escapa, a la gente le produce menos miedo ver esto:

ResultSet res=stm.executeQuery("SELECT * FROM USERS");

como si el mezclar SQL con Java fuera algo más aceptable, como si dijéramos que un programador Java es un DBA con todo el conocimiento de cómo y porqué se hace una determinada consulta.

Huyendo del hecho de escribir consultas directamente en código, hemos pasado a usar frameworks ORM, Object-Relational-Mapping, que dan una visión de la base de datos como si fueran clases, aportando métodos de acceso a las operaciones de inserción / modificación / borrado / consulta de una forma más cómoda para el desarrollador genérico Java, así podemos encontrar como con Spring-Data usando repositorios podemos encontrar cosas como:

List<User> users=userRepository.findAll();

Con lo que ya no vemos de ninguna forma el SQL que nos podía irritar, ¿dónde nos lleva esto? Nos lleva a que delegamos la comunicación con la base de datos a una librería (insertar aquí JPA/Hibernate) de propósito general. ¿Qué pasaría entonces si por ejemplo, queremos cambiar un atributo de una lista de usuarios que cumplen una condición?

List<User> users=userRepository.findAll();
for(User user:users){
   if(user.getBirthDate()==null){
     user.setPendingData(true);
     userRepository.save(user);
   }
}

Y aquí, pese a no ver por ninguna parte código SQL, un desarrollador poco experimentado no vería nada raro, pero un DBA podría leer entre lineas que hay varios problemas realmente grave en este código fuente:

– ¿Porqué hemos traido todos los registros de la base de datos? Podría haber millones de registros, y la tabla de usuarios podría tener decenas de columnas, hemos traído todos, los necesitáramos o no para actualizar únicamente algunos de ellos

– ¿Porqué hemos traido las filas completas? Si sólo vamos a comprobar la fecha de nacimiento, ¿porqué hemos traído toda la fila íntegra? podríamos haber traido sin darnos cuenta un CLOB, o un BLOB ocupando la memoria innecesariamente

– Y finalmente, ¿porqué no hemos hecho TODO esto directamente en la base de datos con una única consulta SQL?

UPDATE USERS SET PENDING_DATA=true WHERE BIRTH_DATE IS NULL

El problema reside en la comodidad a la hora de desarrollar usando frameworks que nos abstraen la base de datos, esto provoca que olvidemos realmente con lo que estamos tratando más allá del propio código fuente que tenemos bajo nuestra nariz. Esta comodidad va acompañada de malos vicios, que desgraciadamente son más difíciles de detectar que cuando eran sentencias SQL directamente insertadas en el código. Estos vicios suelen ser los causantes de fallos de rendimiento tanto en memoria (tratamos más datos de los que necesitamos) como de tráfico de red (movemos por la red más datos de los que necesitamos) y provocan el ya bien conocido «eso se resuelve reiniciando el servidor».

Lógicamente, los responsables de estos problemas no son JPA / Hibernate / Spring-Data (todos las soluciones ORM permiten hacer las cosas bien a costa de programar un poco más) , son los desarrolladores que por tomar el camino más cómodo sacrifican el rendimiento de la aplicación y le pasan el problema al equipo de mantenimiento ( que normalmente son los propios desarrolladores en cuanto pase el día de puesta en producción ).

Una posible solución a este problema es optar por soluciones mixtas, como por ejemplo Spring JDBC Template o Jdbi, que en ambos casos nos permiten tener separados SQL de código fuente, pero dándonos acceso directo a ambos, permitiendo que desarrolladores y DBAs puedan cada uno trabajar en su marco de conocimiento.

Optimización: Llamadas a toByteArray

Hoy analizando código legacy he encontrado decenas de llamadas al método toByteArray de la clase ByteArrayOutputStream, todas en más o menos la siguiente estructura:

ByteArrayOutputStream baos=new ByteArrayOutputStream();
// ... código escribiendo en el byteArrayOutputStream
byte data[]=baos.toByteArray();
// ... código usando esos bytes

Este código es totalmente correcto, pero presenta un fallo de rendimiento interesante, y reside en el hecho de que ByteArrayOutputStream, al obtener los bytes interiores realiza una copia y la devuelve, aquí tenéis el código original de ByteArrayOutputStream.toArray  : 

 public synchronized byte toByteArray()[] {
        return Arrays.copyOf(buf, count);
    }

Es decir, que si el ByteArrayOutputStream estaba ocupando 1Mb (porque contuviese un fichero de cierto tamaño, por ejemplo) duplicaremos el espacio ocupado en cuanto hagamos un toByteArray, ¿Qué podemos hacer?

Bueno, como siempre ya hay gente a la que se le ha ocurrido una solución, y es realizar una implementación propia de ByteArrayOutputStream que permita el acceso directo al array de bytes internos, teniendo siempre en cuenta que cualquier cambio que se realice sobre el mismo se reflejará en el contenido del ByteArrayOutputStream (así que mucho ojo). Aquí tenéis un enlace a la implementación existente en Spring en la clase FastByteArrayOutputStream, y podéis incluirla en vuestros proyectos sin necesidad de usar Spring en el mismo.

Java Bugfixing: InputStream & OutputStream

 

Cuando nos encontramos una base de código legacy, uno de los errores latentes que más aparece en el código de desarrolladores menos experimentados es dejarse streams (input/output streams) abiertos, o a falta de realizar flush.

Veamos un ejemplo clásico de lectura de un fichero en el que el desarrollador olvida cerrar un stream y las consecuencias de dicho error:

import java.io.FileOutputStream;

public class UnclosedStream {
	public static final void main(String[] args) throws Exception{
		for(int i=0;i!=100000;i++) {
			FileOutputStream fos=new FileOutputStream("/tmp/file_"+i+".txt");
			// ... operaciones con el stream
			// Aqui olvidamos cerrar el stream
		}
	}
}

Resultado de esta ejecución:

Exception in thread "main" java.io.FileNotFoundException: /tmp/file_10230.txt (Too many open files)
	at java.io.FileOutputStream.open0(Native Method)
	at java.io.FileOutputStream.open(FileOutputStream.java:270)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:213)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:101)
	at com.chocodev.blog.io.streams.UnclosedStream.main(UnclosedStream.java:8)

El error que vemos es «Too many open files», indicando que la aplicación ha conseguido llegar al número máximo de ficheros que el sistema operativo está dispuesto a permitir (búsquese ulimit). Éste error desprende un olor espantoso cuando un proyecto es de cierto tamaño, lo trágico es que en nuestro ejemplo está claro dónde se produce el error y dónde solucionarlo, pero en el caso de un proyecto de algunas decenas/centenas de miles de líneas de código donde desarrollan varias personas, este error se presenta normalmente en algún lugar inverosímil no relacionado con el bloque de código en el que realmente se comete el error. Es fácil ver los síntomas pero puede ser difícil encontrar el culpable.

Ante este tipo de errores, cabe plantearse cualquiera de las siguientes soluciones:

– Java < 1.7

– Opción 1: Correcto cierre de stream, usando clases estándar

 

public class ClosedStream {
	private static final Logger LOG=Logger.getLogger(ClosedStream.class.getName());
	public static final void main(String[] args) throws Exception {
		for (int i = 0; i != 100000; i++) {
			FileOutputStream fos = null;
			try {
				fos = new FileOutputStream("/tmp/file_" + i + ".txt");
				// ... operar con el stream				
			} catch (IOException ex) {
				LOG.log(Level.SEVERE, ex.getMessage(), ex);
			} finally {
				if (fos != null) {
					try {
						fos.close();
					} catch (Exception ex) {
						LOG.log(Level.SEVERE, ex.getMessage(), ex);
					}
				}
			}
		}
	}
}

Esta forma de operar con stream posteriormente cerrándolo siempre en un bloque finally, libera el recurso pase lo que pase y no requiere de otras librerías, aunque hay que reconocer que no es un código excesivamente conciso (en parte por el propio funcionamiento del lenguaje) ya que nos obliga a hacer otro try dentro del finally para capturar alguna posible excepción derivada de la llamada a close.

– Opcion 2: Correcto cierre de stream, usando librerías

Hay librerías bastante prácticas para el cierre de streams, por ejemplo, si utilizamos commons-io:

import java.io.FileOutputStream;
import java.io.IOException;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.apache.commons.io.IOUtils;

public class ClosedStreamCommonsIO {
	private static final Logger LOG=Logger.getLogger(ClosedStreamCommonsIO.class.getName());
	public static final void main(String[] args) throws Exception {
		for (int i = 0; i != 100000; i++) {
			FileOutputStream fos = null;
			try {
				fos = new FileOutputStream("/tmp/file_" + i + ".txt");
				// ... operar con el stream				
			} catch (IOException ex) {
				LOG.log(Level.SEVERE, ex.getMessage(), ex);
			} finally {
				IOUtils.closeQuietly(fos);				
			}
		}
	}
}

En este caso, IOUtils.closeQuietly se encarga del cierre del stream ignorando cualquier posible error producido en el close

– Java >= 1.7

– Uso de try-with-resources

A partir de Java 1.7, apareció el bloque try-with-resources, que simplifica de forma estándar el cierre de streams en un amplio porcentaje de los casos de la siguiente forma:

import java.io.FileOutputStream;

public class ClosedStreamJava7 {
	public static final void main(String[] args) throws Exception {
		for (int i = 0; i != 100000; i++) {
			try(FileOutputStream fos =  
					new FileOutputStream("/tmp/file_" + i + ".txt"))
			{
				// ... operar con el stream				
			}
		}
	}
}

En este caso, los streams declarados dentro del try(…) son cerrados una vez terminado el bloque, incluso es posible declarar varios dentro del mismo try:

try(FileOutputStream f1=new FileOutputStream("f1");
	FileOutputStream f2=new FileOutputStream("f2")){
	// ... operar con los streams
}

Con el bloque try-with-resources se puede utilizar cualquier clase de la API que implemente el interfaz java.io.Closeable

En este post os he mostrado posiblemente el culpable de la mayoría de los problemas que terminan con un «esta aplicación necesita reiniciarse cada X días / cada noche para mantener el rendimiento». Esta frase es la peor que nosotros como desarrolladores podemos darle a un administrador de sistemas, y la peor que un administrador de sistemas puede darle a un cliente.