From 9fae716515ed8b92caf51fa0d5e5d047c6010a12 Mon Sep 17 00:00:00 2001 From: Emilio Date: Wed, 13 Nov 2024 14:58:42 -0500 Subject: [PATCH] GEOMESA-3416 Log more details when type conversion fails (#3234) * Add reloading cache for type conversions to mitigate transitory classpath issues --- docs/user/datastores/runtime_config.rst | 6 + .../geotools/converters/FastConverter.scala | 146 +++++++++--------- 2 files changed, 78 insertions(+), 74 deletions(-) diff --git a/docs/user/datastores/runtime_config.rst b/docs/user/datastores/runtime_config.rst index 2b8ed07b0cb3..561fc6ec3eb1 100644 --- a/docs/user/datastores/runtime_config.rst +++ b/docs/user/datastores/runtime_config.rst @@ -341,3 +341,9 @@ The class must have a no-arg constructor. By default GeoMesa will use heuristic-based query planning, which should work well for most situations. See :ref:`query_planning` for more details on query planning strategies. + +geomesa.type.converter.cache.expiry ++++++++++++++++++++++++++++++++++++ + +This property controls how long type conversions are cached in memory before being reloaded. It is specified as a duration, +e.g. ``1 minute`` or ``30 seconds``, with a default value of ``1 hour``. diff --git a/geomesa-utils-parent/geomesa-utils/src/main/scala/org/locationtech/geomesa/utils/geotools/converters/FastConverter.scala b/geomesa-utils-parent/geomesa-utils/src/main/scala/org/locationtech/geomesa/utils/geotools/converters/FastConverter.scala index 67bf5a482e6c..7a97b192f118 100644 --- a/geomesa-utils-parent/geomesa-utils/src/main/scala/org/locationtech/geomesa/utils/geotools/converters/FastConverter.scala +++ b/geomesa-utils-parent/geomesa-utils/src/main/scala/org/locationtech/geomesa/utils/geotools/converters/FastConverter.scala @@ -8,13 +8,16 @@ package org.locationtech.geomesa.utils.geotools.converters +import com.github.benmanes.caffeine.cache.{CacheLoader, Caffeine, LoadingCache} import com.typesafe.scalalogging.StrictLogging import org.geotools.api.filter.expression.Expression import org.geotools.data.util.InterpolationConverterFactory import org.geotools.util.factory.GeoTools import org.geotools.util.{Converter, Converters} +import org.locationtech.geomesa.utils.conf.GeoMesaSystemProperties.SystemProperty -import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.TimeUnit +import scala.collection.mutable.ArrayBuffer import scala.reflect.ClassTag import scala.util.control.NonFatal @@ -26,13 +29,31 @@ object FastConverter extends StrictLogging { import scala.collection.JavaConverters._ - private val factories = Converters.getConverterFactories(GeoTools.getDefaultHints).asScala.toArray.filter { - // exclude jai-related factories as it's not usually on the classpath - case _: InterpolationConverterFactory => false - case _ => true - } - - private val cache = new ConcurrentHashMap[(Class[_], Class[_]), Array[Converter]] + val ConverterCacheExpiry: SystemProperty = SystemProperty("geomesa.type.converter.cache.expiry", "1 hour") + + private val cache: LoadingCache[(Class[_], Class[_]), Array[Converter]] = + Caffeine.newBuilder().expireAfterWrite(ConverterCacheExpiry.toDuration.get.toMillis, TimeUnit.MILLISECONDS).build( + new CacheLoader[(Class[_], Class[_]), Array[Converter]]() { + override def load(key: (Class[_], Class[_])): Array[Converter] = { + val (from, to) = key + val factories = Converters.getConverterFactories(GeoTools.getDefaultHints).asScala.toArray.filter { + // exclude jai-related factories as it's not usually on the classpath + case _: InterpolationConverterFactory => false + case _ => true + } + logger.debug(s"Loaded ${factories.length} converter factories: ${factories.map(_.getClass.getName).mkString(", ")}") + val converters = factories.flatMap(factory => Option(factory.createConverter(from, to, null))) + logger.debug( + s"Found ${converters.length} converters for ${from.getName}->${to.getName}: " + + s"${converters.map(_.getClass.getName).mkString(", ")}") + if (to == classOf[String]) { + converters :+ ToStringConverter // add toString as a final fallback + } else { + converters + } + } + } + ) /** * Convert the value into the given type @@ -43,40 +64,23 @@ object FastConverter extends StrictLogging { * @return converted value, or null if it could not be converted */ def convert[T](value: Any, binding: Class[T]): T = { - if (value == null) { - return null.asInstanceOf[T] - } - - val clas = value.getClass - if (clas == binding) { + if (value == null || binding.isAssignableFrom(value.getClass)) { return value.asInstanceOf[T] } - val converters = getConverters(clas, binding) + val converters = cache.get((value.getClass, binding)) - var i = 0 - while (i < converters.length) { - try { - val result = converters(i).convert(value, binding) - if (result != null) { - return result - } - } catch { - case NonFatal(e) => - logger.trace(s"Error converting $value (of type ${value.getClass.getName}) " + - s"to ${binding.getName} using converter ${converters(i).getClass.getName}:", e) - } - i += 1 + val result = tryConvert(value, binding, converters) + if (result == null) { + val msg = + s"Could not convert '$value' (of type ${value.getClass.getName}) to ${binding.getName} " + + s"using ${converters.map(_.getClass.getName).mkString(", ")}" + logger.warn(msg) + logger.debug(msg, new RuntimeException()) } - - val msg = s"Could not convert '$value' (of type ${value.getClass.getName}) to ${binding.getName}" - logger.warn(msg) - logger.debug(msg, new Exception()) - - null.asInstanceOf[T] + result } - /** * Convert the value into one of the given type * @@ -91,34 +95,30 @@ object FastConverter extends StrictLogging { } val clas = value.getClass + val errors = ArrayBuffer.empty[(Class[_], Array[Converter])] while (bindings.hasNext) { val binding = bindings.next - if (clas == binding) { + if (binding.isAssignableFrom(clas)) { return value.asInstanceOf[T] } - val converters = getConverters(clas, binding) + val converters = cache.get((clas, binding)) - var i = 0 - while (i < converters.length) { - try { - val result = converters(i).convert(value, binding) - if (result != null) { - return result - } - } catch { - case NonFatal(e) => - logger.trace(s"Error converting $value (of type ${value.getClass.getName}) " + - s"to ${binding.getName} using converter ${converters(i).getClass.getName}:", e) - } - i += 1 + val result = tryConvert(value, binding, converters) + if (result != null) { + return result + } else { + errors += binding -> converters } } - logger.warn( - s"Could not convert '$value' (of type ${value.getClass.getName}) " + - s"to any of ${bindings.map(_.getName).mkString(", ")}") + val msg = + s"Could not convert '$value' (of type ${value.getClass.getName}) to any of:" + + errors.map { case (b, c) => s"${b.getClass.getName} using ${c.map(_.getClass.getName).mkString(", ")}"}.mkString("\n ", "\n ", "") + + logger.warn(msg) + logger.debug(msg, new RuntimeException()) null.asInstanceOf[T] } @@ -148,32 +148,30 @@ object FastConverter extends StrictLogging { def evaluate[T](expression: Expression, binding: Class[T]): T = convert(expression.evaluate(null), binding) /** - * Gets a cached converter, loading it if necessary + * Try to convert the value * - * @param from from - * @param to to - * @return + * @param value value + * @param binding expected return type + * @param converters converters to use + * @tparam T expected return type + * @return the converted value as type T, or null if could not convert */ - private def getConverters(from: Class[_], to: Class[_]): Array[Converter] = { - var converters = cache.get((from, to)) - - if (converters == null) { - if (from.eq(to) || from == to || to.isAssignableFrom(from)) { - converters = Array(IdentityConverter) - } else { - converters = factories.flatMap(factory => Option(factory.createConverter(from, to, null))) - if (to == classOf[String]) { - converters = converters :+ ToStringConverter // add toString as a final fallback + private def tryConvert[T](value: Any, binding: Class[T], converters: Array[Converter]): T = { + var i = 0 + while (i < converters.length) { + try { + val result = converters(i).convert(value, binding) + if (result != null) { + return result } + } catch { + case NonFatal(e) => + logger.trace(s"Error converting $value (of type ${value.getClass.getName}) " + + s"to ${binding.getName} using converter ${converters(i).getClass.getName}:", e) } - cache.put((from, to), converters) + i += 1 } - - converters - } - - private object IdentityConverter extends Converter { - override def convert[T](source: Any, target: Class[T]): T = source.asInstanceOf[T] + null.asInstanceOf[T] } private object ToStringConverter extends Converter {