Till Westmann has submitted this change and it was merged. Change subject: [ASTERIXDB-1990][ASTERIXDB-1991][STO] Log deletion failure ......................................................................
[ASTERIXDB-1990][ASTERIXDB-1991][STO] Log deletion failure - user model changes: no - storage format changes: no - interface changes: no Details: - Instead of failing operations due to file deletion exceptions, we will log the failure. - Adapt tests to ensure that the failure log is generated. Change-Id: I7cf7c76f0613467ab307eca42f5ac0834a60fa44 Reviewed-on: https://asterix-gerrit.ics.uci.edu/1926 Sonar-Qube: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Contrib: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java M hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/AbstractIndexLifecycleTest.java A hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/RuntimeLogsMonitor.java 3 files changed, 159 insertions(+), 10 deletions(-) Approvals: Anon. E. Moose #1000171: Till Westmann: Looks good to me, approved Jenkins: Verified; No violations found; ; Verified diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java index 0e70759..329b24d 100644 --- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java +++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java @@ -19,8 +19,12 @@ package org.apache.hyracks.api.util; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.util.logging.Level; +import java.util.logging.Logger; import org.apache.commons.io.FileUtils; import org.apache.hyracks.api.exceptions.ErrorCode; @@ -33,6 +37,9 @@ */ public class IoUtil { + public static final String FILE_NOT_FOUND_MSG = "Deleting non-existing file!"; + private static final Logger LOGGER = Logger.getLogger(IoUtil.class.getName()); + private IoUtil() { } @@ -42,7 +49,7 @@ * @param fileRef * the file to be deleted * @throws HyracksDataException - * if the file doesn't exist or if it couldn't be deleted + * if the file couldn't be deleted */ public static void delete(FileReference fileRef) throws HyracksDataException { delete(fileRef.getFile()); @@ -54,7 +61,7 @@ * @param file * the file to be deleted * @throws HyracksDataException - * if the file doesn't exist or if it couldn't be deleted + * if the file couldn't be deleted */ public static void delete(File file) throws HyracksDataException { try { @@ -63,6 +70,8 @@ } else { Files.delete(file.toPath()); } + } catch (NoSuchFileException | FileNotFoundException e) { + LOGGER.log(Level.WARNING, FILE_NOT_FOUND_MSG + ": " + e.getMessage(), e); } catch (IOException e) { throw HyracksDataException.create(ErrorCode.CANNOT_DELETE_FILE, e, file.getAbsolutePath()); } diff --git a/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/AbstractIndexLifecycleTest.java b/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/AbstractIndexLifecycleTest.java index aac4df5..8fc4275 100644 --- a/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/AbstractIndexLifecycleTest.java +++ b/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/AbstractIndexLifecycleTest.java @@ -18,11 +18,19 @@ */ package org.apache.hyracks.storage.am.common; +import java.nio.file.NoSuchFileException; +import java.util.logging.Level; +import java.util.logging.LogRecord; + import org.apache.hyracks.api.exceptions.HyracksDataException; +import org.apache.hyracks.api.util.IoUtil; import org.apache.hyracks.storage.common.IIndex; +import org.apache.hyracks.util.RuntimeLogsMonitor; import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; public abstract class AbstractIndexLifecycleTest { @@ -39,6 +47,16 @@ protected abstract void clearCheckableInsertions() throws Exception; + @BeforeClass + public static void startLogsMonitor() { + RuntimeLogsMonitor.monitor("org.apache.hyracks"); + } + + @AfterClass + public static void afterClass() { + RuntimeLogsMonitor.stop(); + } + @Before public abstract void setup() throws Exception; @@ -47,6 +65,7 @@ @Test public void validSequenceTest() throws Exception { + RuntimeLogsMonitor.reset(); // Double create is invalid index.create(); Assert.assertTrue(persistentStateExists()); @@ -81,16 +100,12 @@ checkInsertions(); index.deactivate(); - // Double destroy is invalid + // Double destroy is idempotent but should log NoSuchFileException index.destroy(); Assert.assertFalse(persistentStateExists()); - exceptionCaught = false; - try { - index.destroy(); - } catch (Exception e) { - exceptionCaught = true; - } - Assert.assertTrue(exceptionCaught); + index.destroy(); + final LogRecord fileNotFoundWarnLog = new LogRecord(Level.WARNING, IoUtil.FILE_NOT_FOUND_MSG); + Assert.assertTrue(RuntimeLogsMonitor.count(fileNotFoundWarnLog) > 0); Assert.assertFalse(persistentStateExists()); } diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/RuntimeLogsMonitor.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/RuntimeLogsMonitor.java new file mode 100644 index 0000000..d2eb7fd --- /dev/null +++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/RuntimeLogsMonitor.java @@ -0,0 +1,125 @@ +/* + * 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.hyracks.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +/** + * RuntimeLogsMonitor is used to store the generated runtime logs in-memory + * and provides API to search the stored logs. + */ +public class RuntimeLogsMonitor { + + private static final InMemoryHandler IN_MEMORY_HANDLER = new InMemoryHandler(); + private static final List<Logger> MONITORED_LOGGERS = new ArrayList<>(); + private static List<LogRecord> logs; + + private RuntimeLogsMonitor() { + reset(); + } + + /** + * Starts monitoring the logger by storing its generated logs in-memory. By default + * only logs with level WARNING and above are stored + * + * @param loggerName + */ + public static void monitor(String loggerName) { + final Logger logger = Logger.getLogger(loggerName); + for (Handler handler : logger.getHandlers()) { + if (handler == IN_MEMORY_HANDLER) { + return; + } + } + MONITORED_LOGGERS.add(logger); + Logger.getLogger(loggerName).addHandler(IN_MEMORY_HANDLER); + } + + /** + * Discards any stored logs + */ + public static void reset() { + logs = new ArrayList<>(); + } + + /** + * Calculates the count based on {@code logRecord} level and message. + * if any stored log has the same level as {@code logRecord} and + * the log's message contains {@code logRecord} message, it is considered + * as an occurrence + * + * @param logRecord + * @return The number of found logs that match {@code logRecord} + */ + public static long count(LogRecord logRecord) { + return logs.stream() + .filter(storedLog -> storedLog.getLevel().equals(logRecord.getLevel()) && storedLog.getMessage() + .contains(logRecord.getMessage())).count(); + } + + /** + * Sets the stored logs minimum level + * + * @param lvl + */ + public static void setLevel(Level lvl) { + IN_MEMORY_HANDLER.setLevel(lvl); + } + + /** + * Stops monitoring any monitored loggers and discards any + * stored logs + */ + public static void stop() { + for (Logger logger : MONITORED_LOGGERS) { + logger.removeHandler(IN_MEMORY_HANDLER); + } + reset(); + MONITORED_LOGGERS.clear(); + } + + private static class InMemoryHandler extends Handler { + private InMemoryHandler() { + super.setLevel(Level.WARNING); + setFilter(record -> record.getLevel().intValue() >= getLevel().intValue()); + } + + @Override + public void publish(LogRecord lr) { + if (isLoggable(lr)) { + logs.add(lr); + } + } + + @Override + public void flush() { + // nothing to flush + } + + @Override + public void close() { + // nothing to close + } + } +} -- To view, visit https://asterix-gerrit.ics.uci.edu/1926 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7cf7c76f0613467ab307eca42f5ac0834a60fa44 Gerrit-PatchSet: 12 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]>
