From efd487e65958ab6815fa68096665eceb1f02cd0e Mon Sep 17 00:00:00 2001 From: Martin Gaievski Date: Thu, 14 Jul 2022 13:43:06 -0700 Subject: [PATCH] Adding String.format for exception error messages Signed-off-by: Martin Gaievski --- .../index/mapper/KNNVectorFieldMapper.java | 16 ++++---- .../knn/index/mapper/LegacyFieldMapper.java | 39 +++++++++++-------- .../knn/index/mapper/MethodFieldMapper.java | 2 +- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java index f8fbb4b87..45d34dd68 100644 --- a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java +++ b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java @@ -87,11 +87,13 @@ public static class Builder extends ParametrizedFieldMapper.Builder { } int value = XContentMapValues.nodeIntegerValue(o); if (value > MAX_DIMENSION) { - throw new IllegalArgumentException("Dimension value cannot be greater than " + MAX_DIMENSION + " for vector: " + name); + throw new IllegalArgumentException( + String.format("Dimension value cannot be greater than %s for vector: %s", MAX_DIMENSION, name) + ); } if (value <= 0) { - throw new IllegalArgumentException("Dimension value must be greater than 0 " + "for vector: " + name); + throw new IllegalArgumentException(String.format("Dimension value must be greater than 0 for vector: %s", name)); } return value; }, m -> toType(m).dimension); @@ -273,12 +275,12 @@ public Mapper.Builder parse(String name, Map node, ParserCont // is done before any mappers are built. Therefore, validation should be done during parsing // so that it can fail early. if (builder.knnMethodContext.get() != null && builder.modelId.get() != null) { - throw new IllegalArgumentException("Method and model can not be both specified in the mapping: " + name); + throw new IllegalArgumentException(String.format("Method and model can not be both specified in the mapping: %s", name)); } // Dimension should not be null unless modelId is used if (builder.dimension.getValue() == -1 && builder.modelId.get() == null) { - throw new IllegalArgumentException("Dimension value missing for vector: " + name); + throw new IllegalArgumentException(String.format("Dimension value missing for vector: %s", name)); } return builder; @@ -325,7 +327,7 @@ public Query existsQuery(QueryShardContext context) { public Query termQuery(Object value, QueryShardContext context) { throw new QueryShardException( context, - "KNN vector do not support exact searching, use KNN queries " + "instead: [" + name() + "]" + String.format("KNN vector do not support exact searching, use KNN queries instead: [%s]", name()) ); } @@ -401,14 +403,14 @@ protected void parseCreateField(ParseContext context, int dimension) throws IOEx void validateIfCircuitBreakerIsNotTriggered() { if (KNNSettings.isCircuitBreakerTriggered()) { throw new IllegalStateException( - "Indexing knn vector fields is rejected as circuit breaker triggered." + " Check _opendistro/_knn/stats for detailed state" + "Indexing knn vector fields is rejected as circuit breaker triggered. Check _opendistro/_knn/stats for detailed state" ); } } void validateIfKNNPluginEnabled() { if (!KNNSettings.isKNNPluginEnabled()) { - throw new IllegalStateException("KNN plugin is disabled. To enable " + "update knn.plugin.enabled setting to true"); + throw new IllegalStateException("KNN plugin is disabled. To enable update knn.plugin.enabled setting to true"); } } diff --git a/src/main/java/org/opensearch/knn/index/mapper/LegacyFieldMapper.java b/src/main/java/org/opensearch/knn/index/mapper/LegacyFieldMapper.java index eb9740a0e..868aec3e6 100644 --- a/src/main/java/org/opensearch/knn/index/mapper/LegacyFieldMapper.java +++ b/src/main/java/org/opensearch/knn/index/mapper/LegacyFieldMapper.java @@ -21,7 +21,14 @@ import static org.opensearch.knn.common.KNNConstants.SPACE_TYPE; /** - * Field mapper for original implementation + * Field mapper for original implementation. It defaults to using nmslib as the engine and retrieves parameters from index settings. + * + * Example of this mapper output: + * + * { + * "type": "knn_vector", + * "dimension": 128 + * } */ @Log4j2 public class LegacyFieldMapper extends KNNVectorFieldMapper { @@ -70,11 +77,11 @@ static String getSpaceType(Settings indexSettings) { String spaceType = indexSettings.get(KNNSettings.INDEX_KNN_SPACE_TYPE.getKey()); if (spaceType == null) { log.info( - "[KNN] The setting \"" - + METHOD_PARAMETER_SPACE_TYPE - + "\" was not set for the index. " - + "Likely caused by recent version upgrade. Setting the setting to the default value=" - + KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE + String.format( + "[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s", + METHOD_PARAMETER_SPACE_TYPE, + KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE + ) ); return KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE; } @@ -85,11 +92,11 @@ static String getM(Settings indexSettings) { String m = indexSettings.get(KNNSettings.INDEX_KNN_ALGO_PARAM_M_SETTING.getKey()); if (m == null) { log.info( - "[KNN] The setting \"" - + HNSW_ALGO_M - + "\" was not set for the index. " - + "Likely caused by recent version upgrade. Setting the setting to the default value=" - + KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M + String.format( + "[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s", + HNSW_ALGO_M, + KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M + ) ); return String.valueOf(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M); } @@ -100,11 +107,11 @@ static String getEfConstruction(Settings indexSettings) { String efConstruction = indexSettings.get(KNNSettings.INDEX_KNN_ALGO_PARAM_EF_CONSTRUCTION_SETTING.getKey()); if (efConstruction == null) { log.info( - "[KNN] The setting \"" - + HNSW_ALGO_EF_CONSTRUCTION - + "\" was not set for" - + " the index. Likely caused by recent version upgrade. Setting the setting to the default value=" - + KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION + String.format( + "[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s", + HNSW_ALGO_EF_CONSTRUCTION, + KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION + ) ); return String.valueOf(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION); } diff --git a/src/main/java/org/opensearch/knn/index/mapper/MethodFieldMapper.java b/src/main/java/org/opensearch/knn/index/mapper/MethodFieldMapper.java index a5c44f1e5..42c12a5db 100644 --- a/src/main/java/org/opensearch/knn/index/mapper/MethodFieldMapper.java +++ b/src/main/java/org/opensearch/knn/index/mapper/MethodFieldMapper.java @@ -53,7 +53,7 @@ public class MethodFieldMapper extends KNNVectorFieldMapper { Strings.toString(XContentFactory.jsonBuilder().map(knnEngine.getMethodAsMap(knnMethodContext))) ); } catch (IOException ioe) { - throw new RuntimeException("Unable to create KNNVectorFieldMapper: " + ioe); + throw new RuntimeException(String.format("Unable to create KNNVectorFieldMapper: %s", ioe)); } this.fieldType.freeze();