diff --git a/bin/autodetect/Main.cc b/bin/autodetect/Main.cc index 7ca5a3233..a18761a0e 100644 --- a/bin/autodetect/Main.cc +++ b/bin/autodetect/Main.cc @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include @@ -119,14 +119,23 @@ int main(int argc, char** argv) { std::size_t maxAnomalyRecords{100}; bool memoryUsage{false}; bool validElasticLicenseKeyConfirmed{false}; - if (ml::autodetect::CCmdLineParser::parse( - argc, argv, configFile, filtersConfigFile, eventsConfigFile, - modelConfigFile, logProperties, logPipe, delimiter, lengthEncodedInput, - timeFormat, quantilesStateFile, deleteStateFiles, bucketPersistInterval, - namedPipeConnectTimeout, inputFileName, isInputFileNamedPipe, outputFileName, - isOutputFileNamedPipe, restoreFileName, isRestoreFileNamedPipe, - persistFileName, isPersistFileNamedPipe, isPersistInForeground, - maxAnomalyRecords, memoryUsage, validElasticLicenseKeyConfirmed) == false) { + std::unique_ptr removeQuantilesStateOnFailure; + + const bool parseSuccess = ml::autodetect::CCmdLineParser::parse( + argc, argv, configFile, filtersConfigFile, eventsConfigFile, + modelConfigFile, logProperties, logPipe, delimiter, lengthEncodedInput, + timeFormat, quantilesStateFile, deleteStateFiles, bucketPersistInterval, + namedPipeConnectTimeout, inputFileName, isInputFileNamedPipe, outputFileName, + isOutputFileNamedPipe, restoreFileName, isRestoreFileNamedPipe, + persistFileName, isPersistFileNamedPipe, isPersistInForeground, + maxAnomalyRecords, memoryUsage, validElasticLicenseKeyConfirmed); + + if (!quantilesStateFile.empty()) { + removeQuantilesStateOnFailure = std::make_unique( + quantilesStateFile, deleteStateFiles); + } + + if (parseSuccess == false) { return EXIT_FAILURE; } @@ -293,9 +302,6 @@ int main(int argc, char** argv) { LOG_FATAL(<< "Failed to restore quantiles and initialize normalizer"); return EXIT_FAILURE; } - if (deleteStateFiles) { - std::remove(quantilesStateFile.c_str()); - } } // The categorizer knows how to assign categories to records @@ -346,5 +352,14 @@ int main(int argc, char** argv) { // message indicating early exit then the process has probably core dumped LOG_DEBUG(<< "ML anomaly detector job exiting"); + // No need for a warning here so we reset the cleanup function and delete the file explicitly if requested. + removeQuantilesStateOnFailure.reset(); + if (deleteStateFiles) { + if (std::remove(quantilesStateFile.c_str()) != 0) { + LOG_WARN(<< "Failed to delete quantiles state file '" + << quantilesStateFile << "': " << strerror(errno)); + } + } + return EXIT_SUCCESS; } diff --git a/bin/normalize/Main.cc b/bin/normalize/Main.cc index 9aa8e9489..c25b6e71b 100644 --- a/bin/normalize/Main.cc +++ b/bin/normalize/Main.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -63,11 +64,20 @@ int main(int argc, char** argv) { bool deleteStateFiles{false}; bool writeCsv{false}; bool validElasticLicenseKeyConfirmed{false}; - if (ml::normalize::CCmdLineParser::parse( - argc, argv, modelConfigFile, logProperties, logPipe, bucketSpan, - lengthEncodedInput, namedPipeConnectTimeout, inputFileName, - isInputFileNamedPipe, outputFileName, isOutputFileNamedPipe, quantilesStateFile, - deleteStateFiles, writeCsv, validElasticLicenseKeyConfirmed) == false) { + std::unique_ptr removeQuantilesStateOnFailure; + + const bool parseSuccess = ml::normalize::CCmdLineParser::parse( + argc, argv, modelConfigFile, logProperties, logPipe, bucketSpan, + lengthEncodedInput, namedPipeConnectTimeout, inputFileName, + isInputFileNamedPipe, outputFileName, isOutputFileNamedPipe, quantilesStateFile, + deleteStateFiles, writeCsv, validElasticLicenseKeyConfirmed); + + if (!quantilesStateFile.empty()) { + removeQuantilesStateOnFailure = std::make_unique( + quantilesStateFile, deleteStateFiles); + } + + if (parseSuccess == false) { return EXIT_FAILURE; } @@ -158,9 +168,6 @@ int main(int argc, char** argv) { LOG_FATAL(<< "Failed to initialize normalizer"); return EXIT_FAILURE; } - if (deleteStateFiles) { - std::remove(quantilesStateFile.c_str()); - } } // Now handle the numbers to be normalised from stdin @@ -176,5 +183,14 @@ int main(int argc, char** argv) { // message indicating early exit then the process has probably core dumped LOG_DEBUG(<< "ML normalizer exiting"); + // No need for a warning here so we reset the cleanup function and delete the file explicitly if requested. + removeQuantilesStateOnFailure.reset(); + if (deleteStateFiles) { + if (std::remove(quantilesStateFile.c_str()) != 0) { + LOG_WARN(<< "Failed to delete quantiles state file '" + << quantilesStateFile << "': " << strerror(errno)); + } + } + return EXIT_SUCCESS; } diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index 20e500660..251c8d8eb 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -27,6 +27,13 @@ //=== Bug Fixes //=== Regressions + +== {es} version 9.4.0 + +=== Enhancements + +* Better error handling regarding quantiles state documents (See {ml-pull}[#2894]) + == {es} version 9.3.0 === Enhancements diff --git a/include/core/CStateFileRemover.h b/include/core/CStateFileRemover.h new file mode 100644 index 000000000..d1d9ff237 --- /dev/null +++ b/include/core/CStateFileRemover.h @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#ifndef INCLUDED_ml_core_CStateFileRemover_h +#define INCLUDED_ml_core_CStateFileRemover_h + +#include + +#include + +namespace ml { +namespace core { + +//! \brief +//! Ensures that deletion of state files occurs even on process failure. +//! +//! DESCRIPTION:\n +//! A helper to ensure that quantiles state files always get deleted on failure. +//! They may also be explicitly be deleted on request as well but that is handled separately by the happy path. +//! +//! IMPLEMENTATION DECISIONS:\n +//! Not copyable or moveable. No default construction. +class CStateFileRemover { +public: + CStateFileRemover() = delete; + CStateFileRemover(const CStateFileRemover&) = delete; + CStateFileRemover& operator=(const CStateFileRemover&) = delete; + CStateFileRemover(CStateFileRemover&&) = delete; + CStateFileRemover& operator=(CStateFileRemover&&) = delete; + explicit CStateFileRemover(const std::string& quantilesStateFile, + bool deleteStateFiles = false) + : m_QuantilesStateFile{quantilesStateFile}, m_DeleteStateFiles{deleteStateFiles} {} + ~CStateFileRemover() { + // Always delete quantiles state files if requested to do so, even on failure, + // else we run the risk of filling the disk after repeated failures. + // They should still exist in ES should they need to be examined. + if (m_QuantilesStateFile.empty() || m_DeleteStateFiles == false) { + return; + } + LOG_DEBUG(<< "Deleting quantiles state file '" << m_QuantilesStateFile << "'"); + if (std::remove(m_QuantilesStateFile.c_str()) != 0) { + LOG_WARN(<< "Failed to delete quantiles state file '" + << m_QuantilesStateFile << "': " << strerror(errno)); + } + } + +private: + std::string m_QuantilesStateFile; + bool m_DeleteStateFiles{false}; +}; +} +} + +#endif // INCLUDED_ml_core_CStateFileRemover_h