Skip to content

Commit

Permalink
GEOMESA-3416 Log more details when type conversion fails (#3234)
Browse files Browse the repository at this point in the history 
* Add reloading cache for type conversions to mitigate transitory classpath issues
  • Loading branch information
@elahrvivaz
elahrvivaz authored Nov 13, 2024
1 parent 21d797d commit 9fae716
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 74 deletions.
6 changes: 6 additions & 0 deletions docs/user/datastores/runtime_config.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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``.
146 changes: 72 additions & 74 deletions ...ils/src/main/scala/org/locationtech/geomesa/utils/geotools/converters/FastConverter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
*
Expand All @@ -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]
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 9fae716

Please sign in to comment.