From d77637344d039fa51da6750687e5db94dd5c7a57 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 26 Sep 2024 01:14:37 -0700 Subject: [PATCH] log.warn anytime a column is relying on ArrayIngestMode.MVD (#17164) * log.warn anytime a column is relying on ArrayIngestMode.MVD --- .../druid/msq/sql/MSQTaskSqlEngine.java | 2 +- .../druid/msq/util/DimensionSchemaUtils.java | 13 ++++- .../overlord/RemoteTaskRunnerTest.java | 2 +- .../java/util/emitter/EmittingLogger.java | 56 +++++++++++++++---- .../java/util/emitter/service/AlertEvent.java | 7 +++ 5 files changed, 66 insertions(+), 14 deletions(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java index bdebe32a16f..1964ad3de4c 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java @@ -387,7 +387,7 @@ public class MSQTaskSqlEngine implements SqlEngine final ColumnType oldDruidType = Calcites.getColumnTypeForRelDataType(oldSqlTypeField.getType()); final RelDataType newSqlType = rootRel.getRowType().getFieldList().get(columnIndex).getType(); final ColumnType newDruidType = - DimensionSchemaUtils.getDimensionType(Calcites.getColumnTypeForRelDataType(newSqlType), arrayIngestMode); + DimensionSchemaUtils.getDimensionType(columnName, Calcites.getColumnTypeForRelDataType(newSqlType), arrayIngestMode); if (newDruidType.isArray() && oldDruidType.is(ValueType.STRING) || (newDruidType.is(ValueType.STRING) && oldDruidType.isArray())) { diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/DimensionSchemaUtils.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/DimensionSchemaUtils.java index e08c0e5ded6..3ad1eb48448 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/DimensionSchemaUtils.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/DimensionSchemaUtils.java @@ -27,6 +27,8 @@ import org.apache.druid.data.input.impl.StringDimensionSchema; import org.apache.druid.error.InvalidInput; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.emitter.EmittingLogger; +import org.apache.druid.java.util.emitter.service.AlertEvent; import org.apache.druid.segment.AutoTypeColumnSchema; import org.apache.druid.segment.DimensionHandlerUtils; import org.apache.druid.segment.column.ColumnCapabilities; @@ -42,6 +44,7 @@ import javax.annotation.Nullable; */ public class DimensionSchemaUtils { + private static final EmittingLogger LOG = new EmittingLogger(DimensionSchemaUtils.class); /** * Creates a dimension schema for creating {@link org.apache.druid.data.input.InputSourceReader}. @@ -89,7 +92,7 @@ public class DimensionSchemaUtils return new AutoTypeColumnSchema(column, null); } else { // dimensionType may not be identical to queryType, depending on arrayIngestMode. - final ColumnType dimensionType = getDimensionType(queryType, arrayIngestMode); + final ColumnType dimensionType = getDimensionType(column, queryType, arrayIngestMode); if (dimensionType.getType() == ValueType.STRING) { return new StringDimensionSchema( @@ -121,6 +124,7 @@ public class DimensionSchemaUtils * @throws org.apache.druid.error.DruidException if there is some problem */ public static ColumnType getDimensionType( + final String columnName, @Nullable final ColumnType queryType, final ArrayIngestMode arrayIngestMode ) @@ -132,6 +136,13 @@ public class DimensionSchemaUtils ValueType elementType = queryType.getElementType().getType(); if (elementType == ValueType.STRING) { if (arrayIngestMode == ArrayIngestMode.MVD) { + final String msgFormat = "Inserting a multi-value string column[%s] relying on deprecated" + + " ArrayIngestMode.MVD. This query should be rewritten to use the ARRAY_TO_MV" + + " operator to insert ARRAY types as multi-value strings."; + LOG.makeWarningAlert(msgFormat, columnName) + .severity(AlertEvent.Severity.DEPRECATED) + .addData("feature", "ArrayIngestMode.MVD") + .emit(); return ColumnType.STRING; } else { return queryType; diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/RemoteTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/RemoteTaskRunnerTest.java index dec98e05291..c1a77c1f4b4 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/RemoteTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/RemoteTaskRunnerTest.java @@ -1073,7 +1073,7 @@ public class RemoteTaskRunnerTest EasyMock.expect(worker.getHost()).andReturn("host").atLeastOnce(); EasyMock.replay(worker); ServiceEmitter emitter = EasyMock.createMock(ServiceEmitter.class); - Capture capturedArgument = Capture.newInstance(); + Capture capturedArgument = Capture.newInstance(); emitter.emit(EasyMock.capture(capturedArgument)); EasyMock.expectLastCall().atLeastOnce(); EmittingLogger.registerEmitter(emitter); diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java b/processing/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java index 6531c6bd3a8..709da2f0a41 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java @@ -29,8 +29,11 @@ import org.apache.druid.java.util.emitter.service.AlertBuilder; import org.apache.druid.java.util.emitter.service.ServiceEmitter; import javax.annotation.Nullable; +import java.util.Map; /** + * {@link Logger} which also has an {@link ServiceEmitter}. Primarily useful for constructing and emitting "alerts" in + * the form of {@link AlertBuilder}, which will log when {@link AlertBuilder#emit()} is called. */ public class EmittingLogger extends Logger { @@ -62,12 +65,34 @@ public class EmittingLogger extends Logger return new EmittingLogger(getSlf4jLogger(), false); } - public AlertBuilder makeAlert(String message, Object... objects) + /** + * Make an {@link AlertBuilder} which will call {@link #warn(String, Object...)} when {@link AlertBuilder#emit()} is + * called. + */ + public AlertBuilder makeWarningAlert(String message, Object... objects) { - return makeAlert(null, message, objects); + return makeAlert(null, false, message, objects); } + /** + * Make an {@link AlertBuilder} which will call {@link #error(String, Object...)} when {@link AlertBuilder#emit()} is + * called. + */ + public AlertBuilder makeAlert(String message, Object... objects) + { + return makeAlert(null, true, message, objects); + } + + /** + * Make an {@link AlertBuilder} which will call {@link #error(Throwable, String, Object...)} when + * {@link AlertBuilder#emit()} is called. + */ public AlertBuilder makeAlert(@Nullable Throwable t, String message, Object... objects) + { + return makeAlert(t, true, message, objects); + } + + public AlertBuilder makeAlert(@Nullable Throwable t, boolean isError, String message, Object... objects) { if (emitter == null) { final String errorMessage = StringUtils.format( @@ -87,20 +112,22 @@ public class EmittingLogger extends Logger throw e; } - return new EmittingAlertBuilder(t, StringUtils.format(message, objects), emitter) + return new LoggingAlertBuilder(t, StringUtils.format(message, objects), emitter, isError) .addData("class", className); } - public class EmittingAlertBuilder extends AlertBuilder + public class LoggingAlertBuilder extends AlertBuilder { private final Throwable t; + private final boolean isError; private volatile boolean emitted = false; - private EmittingAlertBuilder(Throwable t, String description, ServiceEmitter emitter) + private LoggingAlertBuilder(Throwable t, String description, ServiceEmitter emitter, boolean isError) { super(description, emitter); this.t = t; + this.isError = isError; addThrowable(t); } @@ -126,15 +153,22 @@ public class EmittingLogger extends Logger private void logIt(String format) { if (t == null) { - error(format, description, dataMap); + if (isError) { + error(format, description, dataMap); + } else { + warn(format, description, dataMap); + } } else { // Filter out the stack trace from the message, because it should be in the logline already if it's wanted. - error( - t, - format, - description, - Maps.filterKeys(dataMap, Predicates.not(Predicates.equalTo("exceptionStackTrace"))) + final Map filteredDataMap = Maps.filterKeys( + dataMap, + Predicates.not(Predicates.equalTo("exceptionStackTrace")) ); + if (isError) { + error(t, format, description, filteredDataMap); + } else { + warn(t, format, description, filteredDataMap); + } } } } diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/service/AlertEvent.java b/processing/src/main/java/org/apache/druid/java/util/emitter/service/AlertEvent.java index e2a7987c15a..0c960a86860 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/service/AlertEvent.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/service/AlertEvent.java @@ -186,6 +186,13 @@ public class AlertEvent implements Event { return "service-failure"; } + }, + DEPRECATED { + @Override + public String toString() + { + return "deprecated"; + } }; public static final Severity DEFAULT = COMPONENT_FAILURE;