diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitor.java index d723b8dc88849..4f8eca8f27551 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitor.java @@ -345,9 +345,12 @@ public TSStatus visitDeleteData(DeleteDataNode node, DataRegion dataRegion) { return StatusUtils.OK; } catch (WriteProcessRejectException e) { return RpcUtils.getStatus(e.getErrorCode(), e.getMessage()); - } catch (IOException | IllegalPathException e) { + } catch (IOException e) { LOGGER.error(DataNodeMiscMessages.ERROR_EXECUTING_PLAN_NODE, node, e); - return new TSStatus(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode()); + return RpcUtils.getStatus(TSStatusCode.WRITE_PROCESS_ERROR, e.getMessage()); + } catch (IllegalPathException e) { + LOGGER.error(DataNodeMiscMessages.ERROR_EXECUTING_PLAN_NODE_CAUSED, node, e.getMessage()); + return RpcUtils.getStatus(TSStatusCode.WRITE_PROCESS_ERROR, e.getMessage()); } finally { dataRegion.writeUnlock(); } @@ -363,7 +366,7 @@ public TSStatus visitDeleteData( return RpcUtils.getStatus(e.getErrorCode(), e.getMessage()); } catch (final IOException e) { LOGGER.error(DataNodeMiscMessages.ERROR_EXECUTING_PLAN_NODE, node, e); - return new TSStatus(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode()); + return RpcUtils.getStatus(TSStatusCode.WRITE_PROCESS_ERROR, e.getMessage()); } } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java index 3c94440301dda..3ce8964a87652 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/DataRegion.java @@ -1850,7 +1850,8 @@ private void registerToTsFile(InsertNode node, TsFileProcessor tsFileProcessor) logger.error( StorageEngineMessages .STORAGE_LOG_REMOTE_REQUEST_CONFIG_NODE_FAILED_THAT_JUDGMENT_IF_TABLE_25FE3602, - e.getMessage()); + e.getMessage(), + e); CommonMetadataUtils.throwTableNotExistsException(getDatabaseName(), tableName); return null; // unreachable, throwTableNotExistsException always throws } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/timeindex/FileTimeIndexCacheRecorder.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/timeindex/FileTimeIndexCacheRecorder.java index 40159a6924bc6..8aacb519c9d12 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/timeindex/FileTimeIndexCacheRecorder.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/timeindex/FileTimeIndexCacheRecorder.java @@ -104,7 +104,7 @@ public void logFileTimeIndex(TsFileResource... tsFileResources) { writer.write(buffer); } catch (IOException e) { LOGGER.warn( - StorageEngineMessages.ERROR_RECORD_FILE_TIME_INDEX_CACHE, e.getMessage()); + StorageEngineMessages.ERROR_RECORD_FILE_TIME_INDEX_CACHE, e.getMessage(), e); } }); if (!result) { @@ -158,7 +158,7 @@ public void compactFileTimeIndexIfNeeded( } } catch (IOException e) { LOGGER.warn( - StorageEngineMessages.ERROR_COMPACT_FILE_TIME_INDEX_CACHE, e.getMessage()); + StorageEngineMessages.ERROR_COMPACT_FILE_TIME_INDEX_CACHE, e.getMessage(), e); } }); if (!result) { @@ -208,7 +208,7 @@ public void removeFileTimeIndexCache(int dataRegionId) { writer.close(); deleteFileOrDirectory(writer.getLogFile(), true); } catch (IOException e) { - LOGGER.warn(StorageEngineMessages.ERROR_CLOSE_FILE_TIME_INDEX_CACHE, e.getMessage()); + LOGGER.warn(StorageEngineMessages.ERROR_CLOSE_FILE_TIME_INDEX_CACHE, e.getMessage(), e); } } } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java index 45432e7ab3df5..61a57f1b10582 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java @@ -919,7 +919,8 @@ public Set getMemTableIds(long fileVersionId) { .STORAGE_LOG_FAIL_TO_READ_MEMTABLE_IDS_FROM_THE_WAL_FILE_OF_WAL_NODE_54B0056E, id, identifier, - e.getMessage()); + e.getMessage(), + e); } catch (IOException e) { logger.warn( StorageEngineMessages diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/recover/WALNodeRecoverTask.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/recover/WALNodeRecoverTask.java index 7efce7036563b..73d429e3ccb07 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/recover/WALNodeRecoverTask.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/recover/WALNodeRecoverTask.java @@ -332,7 +332,8 @@ private void recoverTsFiles() { StorageEngineMessages .STORAGE_LOG_FAIL_TO_READ_MEMTABLE_IDS_FROM_THE_WAL_FILE_OF_WAL_NODE_5325B5AB, walFile.getAbsoluteFile(), - e.getMessage()); + logDirectory.getName(), + e); } catch (IOException e) { logger.warn( StorageEngineMessages diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitorTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitorTest.java new file mode 100644 index 0000000000000..8d754bb3d10ce --- /dev/null +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataExecutionVisitorTest.java @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.iotdb.db.consensus.statemachine.dataregion; + +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.exception.IllegalPathException; +import org.apache.iotdb.commons.exception.IoTDBRuntimeException; +import org.apache.iotdb.commons.exception.SemanticException; +import org.apache.iotdb.commons.path.MeasurementPath; +import org.apache.iotdb.commons.path.PartialPath; +import org.apache.iotdb.commons.queryengine.plan.planner.plan.node.PlanNodeId; +import org.apache.iotdb.db.exception.runtime.TableLostRuntimeException; +import org.apache.iotdb.db.queryengine.plan.planner.plan.node.write.DeleteDataNode; +import org.apache.iotdb.db.queryengine.plan.planner.plan.node.write.InsertRowNode; +import org.apache.iotdb.db.queryengine.plan.planner.plan.node.write.InsertRowsNode; +import org.apache.iotdb.db.queryengine.plan.planner.plan.node.write.RelationalDeleteDataNode; +import org.apache.iotdb.db.storageengine.dataregion.DataRegion; +import org.apache.iotdb.rpc.TSStatusCode; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.apache.tsfile.enums.TSDataType; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.Collections; + +import static org.mockito.ArgumentMatchers.any; + +public class DataExecutionVisitorTest { + + private final Logger logger = (Logger) LoggerFactory.getLogger(DataExecutionVisitor.class); + private ListAppender appender; + + @Before + public void setUp() { + appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + } + + @After + public void tearDown() { + logger.detachAppender(appender); + appender.stop(); + } + + @Test + public void testInsertRowsSemanticFailureDoesNotLogStackTrace() throws Exception { + assertInsertRowsFailureWithoutStackTrace(new SemanticException("injected write failure")); + } + + @Test + public void testInsertRowsTableLostFailureDoesNotLogStackTrace() throws Exception { + assertInsertRowsFailureWithoutStackTrace( + new TableLostRuntimeException("test_db", "test_table")); + } + + private void assertInsertRowsFailureWithoutStackTrace(final IoTDBRuntimeException failure) + throws Exception { + final DataRegion dataRegion = Mockito.mock(DataRegion.class); + Mockito.doThrow(failure).when(dataRegion).insert(any(InsertRowsNode.class)); + + final InsertRowNode insertRowNode = + new InsertRowNode( + new PlanNodeId("write-log-test"), + new PartialPath("root.sg.device"), + false, + new String[] {"s1"}, + new TSDataType[] {TSDataType.INT64}, + 1L, + new Object[] {1L}, + false); + final InsertRowsNode insertRowsNode = + new InsertRowsNode( + new PlanNodeId("write-log-test"), + Collections.singletonList(0), + Collections.singletonList(insertRowNode)); + + final TSStatus status = new DataExecutionVisitor().visitInsertRows(insertRowsNode, dataRegion); + + Assert.assertEquals(failure.getErrorCode(), status.getCode()); + Assert.assertEquals(failure.getMessage(), status.getMessage()); + Assert.assertEquals(1, appender.list.size()); + Assert.assertTrue(appender.list.get(0).getFormattedMessage().contains(failure.getMessage())); + Assert.assertNull(appender.list.get(0).getThrowableProxy()); + } + + @Test + public void testTreeDeleteFailureReturnsMessage() throws Exception { + final DataRegion dataRegion = Mockito.mock(DataRegion.class); + Mockito.when(dataRegion.getDatabaseName()).thenReturn("root.sg"); + final DeleteDataNode node = createDeleteDataNode("root.sg.device.s1"); + final IOException failure = new IOException("injected tree delete failure"); + Mockito.doThrow(failure).when(dataRegion).deleteByDevice(any(MeasurementPath.class), any()); + + final TSStatus status = new DataExecutionVisitor().visitDeleteData(node, dataRegion); + + Assert.assertEquals(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode(), status.getCode()); + Assert.assertEquals(failure.getMessage(), status.getMessage()); + Assert.assertEquals(1, appender.list.size()); + Assert.assertNotNull(appender.list.get(0).getThrowableProxy()); + Assert.assertEquals( + failure.getClass().getName(), appender.list.get(0).getThrowableProxy().getClassName()); + Mockito.verify(dataRegion).writeUnlock(); + } + + @Test + public void testTreeDeleteIllegalPathDoesNotLogStackTrace() throws Exception { + final DataRegion dataRegion = Mockito.mock(DataRegion.class); + Mockito.when(dataRegion.getDatabaseName()).thenReturn("root..sg"); + final DeleteDataNode node = createDeleteDataNode("root.sg.device.s1"); + final IllegalPathException failure = new IllegalPathException("root..sg.**"); + + final TSStatus status = new DataExecutionVisitor().visitDeleteData(node, dataRegion); + + Assert.assertEquals(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode(), status.getCode()); + Assert.assertEquals(failure.getMessage(), status.getMessage()); + Assert.assertEquals(1, appender.list.size()); + Assert.assertTrue(appender.list.get(0).getFormattedMessage().contains(failure.getMessage())); + Assert.assertNull(appender.list.get(0).getThrowableProxy()); + Mockito.verify(dataRegion).writeUnlock(); + Mockito.verify(dataRegion, Mockito.never()).deleteByDevice(any(MeasurementPath.class), any()); + } + + @Test + public void testDirectTreeDeleteFailureReturnsMessage() throws Exception { + final DataRegion dataRegion = Mockito.mock(DataRegion.class); + Mockito.when(dataRegion.getDatabaseName()).thenReturn("root.sg"); + final DeleteDataNode node = createDeleteDataNode("root.sg.**"); + final IOException failure = new IOException("injected direct delete failure"); + Mockito.doThrow(failure).when(dataRegion).deleteDataDirectly(any(MeasurementPath.class), any()); + + final TSStatus status = new DataExecutionVisitor().visitDeleteData(node, dataRegion); + + Assert.assertEquals(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode(), status.getCode()); + Assert.assertEquals(failure.getMessage(), status.getMessage()); + Mockito.verify(dataRegion).writeUnlock(); + } + + @Test + public void testTableDeleteFailureReturnsMessage() throws Exception { + final DataRegion dataRegion = Mockito.mock(DataRegion.class); + final RelationalDeleteDataNode node = + new RelationalDeleteDataNode( + new PlanNodeId("delete-error-test"), Collections.emptyList(), "test_db"); + final IOException failure = new IOException("injected table delete failure"); + Mockito.doThrow(failure).when(dataRegion).deleteByTable(node); + + final TSStatus status = new DataExecutionVisitor().visitDeleteData(node, dataRegion); + + Assert.assertEquals(TSStatusCode.WRITE_PROCESS_ERROR.getStatusCode(), status.getCode()); + Assert.assertEquals(failure.getMessage(), status.getMessage()); + } + + private DeleteDataNode createDeleteDataNode(final String path) throws IllegalPathException { + return new DeleteDataNode( + new PlanNodeId("delete-error-test"), + Collections.singletonList(new MeasurementPath(path)), + Long.MIN_VALUE, + Long.MAX_VALUE); + } +}