From 9b655ce6f188932e2044baacf186896b216d8603 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Thu, 25 May 2017 14:40:09 +0100 Subject: [PATCH] [ML] Improve logging while removing expired data (elastic/x-pack-elasticsearch#1554) relates elastic/x-pack-elasticsearch#1286 Original commit: elastic/x-pack-elasticsearch@4f938fa14b6366fd0c32db1df9621880d2aaf786 --- .../xpack/ml/action/DeleteModelSnapshotAction.java | 6 ++++-- .../java/org/elasticsearch/xpack/ml/job/JobManager.java | 2 +- .../org/elasticsearch/xpack/ml/job/messages/Messages.java | 2 +- .../ml/job/retention/ExpiredModelSnapshotsRemover.java | 3 +-- .../xpack/ml/job/retention/ExpiredResultsRemover.java | 7 +++++-- .../elasticsearch/xpack/ml/job/messages/MessagesTests.java | 4 ++-- 6 files changed, 14 insertions(+), 10 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteModelSnapshotAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteModelSnapshotAction.java index ea776effa87..8afb2ccad63 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteModelSnapshotAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/DeleteModelSnapshotAction.java @@ -184,8 +184,10 @@ public class DeleteModelSnapshotAction extends Action() { @Override public void onResponse(BulkResponse bulkResponse) { - auditor.info(request.getJobId(), Messages.getMessage(Messages.JOB_AUDIT_SNAPSHOT_DELETED, - deleteCandidate.getDescription())); + String msg = Messages.getMessage(Messages.JOB_AUDIT_SNAPSHOT_DELETED, deleteCandidate.getSnapshotId(), + deleteCandidate.getDescription()); + auditor.info(request.getJobId(), msg); + logger.debug("[{}] {}", request.getJobId(), msg); // We don't care about the bulk response, just that it succeeded listener.onResponse(new DeleteModelSnapshotAction.Response(true)); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index de37dd203f9..c3a6f6dae58 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -274,7 +274,7 @@ public class JobManager extends AbstractComponent { // ------- CheckedConsumer apiResponseHandler = jobDeleted -> { if (jobDeleted) { - logger.info("Job [" + jobId + "] deleted."); + logger.info("Job [" + jobId + "] deleted"); auditor.info(jobId, Messages.getMessage(Messages.JOB_AUDIT_DELETED)); actionListener.onResponse(new DeleteJobAction.Response(true)); } else { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java index 2349716d3f4..a75117489d2 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java @@ -51,7 +51,7 @@ public final class Messages { public static final String JOB_AUDIT_DELETED = "Job deleted"; public static final String JOB_AUDIT_OLD_RESULTS_DELETED = "Deleted results prior to {1}"; public static final String JOB_AUDIT_REVERTED = "Job model snapshot reverted to ''{0}''"; - public static final String JOB_AUDIT_SNAPSHOT_DELETED = "Job model snapshot ''{0}'' deleted"; + public static final String JOB_AUDIT_SNAPSHOT_DELETED = "Model snapshot [{0}] with description ''{1}'' deleted"; public static final String JOB_CONFIG_BYFIELD_INCOMPATIBLE_FUNCTION = "by_field_name cannot be used with function ''{0}''"; public static final String JOB_CONFIG_CATEGORIZATION_FILTERS_CONTAINS_DUPLICATES = "categorization_filters contain duplicates"; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredModelSnapshotsRemover.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredModelSnapshotsRemover.java index 07c0adb6437..34bc90ab7b2 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredModelSnapshotsRemover.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredModelSnapshotsRemover.java @@ -63,7 +63,7 @@ public class ExpiredModelSnapshotsRemover extends AbstractExpiredJobDataRemover onFinish.run(); return; } - LOGGER.info("Removing model snapshots of job [{}] that have a timestamp before [{}]", job.getId(), cutoffEpochMs); + LOGGER.debug("Removing model snapshots of job [{}] that have a timestamp before [{}]", job.getId(), cutoffEpochMs); SearchRequest searchRequest = new SearchRequest(); searchRequest.indices(AnomalyDetectorsIndex.jobResultsAliasedName(job.getId())); @@ -111,7 +111,6 @@ public class ExpiredModelSnapshotsRemover extends AbstractExpiredJobDataRemover client.execute(DeleteModelSnapshotAction.INSTANCE, deleteSnapshotRequest, new ActionListener() { @Override public void onResponse(DeleteModelSnapshotAction.Response response) { - LOGGER.trace("[{}] Deleted expired snapshot [{}]", modelSnapshot.getJobId(), modelSnapshot.getSnapshotId()); try { deleteModelSnapshots(modelSnapshotIterator, onFinish); } catch (Exception e) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java index 86f4b694f0a..007c7342a81 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/retention/ExpiredResultsRemover.java @@ -56,14 +56,16 @@ public class ExpiredResultsRemover extends AbstractExpiredJobDataRemover { @Override protected void removeDataBefore(Job job, long cutoffEpochMs, Runnable onFinish) { - LOGGER.info("Removing results of job [{}] that have a timestamp before [{}]", job.getId(), cutoffEpochMs); + LOGGER.debug("Removing results of job [{}] that have a timestamp before [{}]", job.getId(), cutoffEpochMs); DeleteByQueryRequest request = createDBQRequest(job, cutoffEpochMs); client.execute(DeleteByQueryAction.INSTANCE, request, new ActionListener() { @Override public void onResponse(BulkByScrollResponse bulkByScrollResponse) { try { - auditResultsWereDeleted(job.getId(), cutoffEpochMs); + if (bulkByScrollResponse.getDeleted() > 0) { + auditResultsWereDeleted(job.getId(), cutoffEpochMs); + } onFinish.run(); } catch (Exception e) { onFailure(e); @@ -99,6 +101,7 @@ public class ExpiredResultsRemover extends AbstractExpiredJobDataRemover { ZonedDateTime zonedDateTime = ZonedDateTime.ofInstant(instant, ZoneOffset.systemDefault()); String formatted = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(zonedDateTime); String msg = Messages.getMessage(Messages.JOB_AUDIT_OLD_RESULTS_DELETED, formatted); + LOGGER.debug("[{}] {}", jobId, msg); auditor.info(jobId, msg); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/messages/MessagesTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/messages/MessagesTests.java index f7f23241d13..0f76699d046 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/messages/MessagesTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/messages/MessagesTests.java @@ -18,7 +18,7 @@ public class MessagesTests extends ESTestCase { String formattedMessage = Messages.getMessage(Messages.DATAFEED_CONFIG_INVALID_OPTION_VALUE, "field-name", "field-value"); assertEquals("Invalid field-name value 'field-value' in datafeed configuration", formattedMessage); - formattedMessage = Messages.getMessage(Messages.JOB_AUDIT_SNAPSHOT_DELETED, "foo-job"); - assertEquals("Job model snapshot 'foo-job' deleted", formattedMessage); + formattedMessage = Messages.getMessage(Messages.JOB_AUDIT_SNAPSHOT_DELETED, "snapshot_foo", "snapshot description"); + assertEquals("Model snapshot [snapshot_foo] with description 'snapshot description' deleted", formattedMessage); } }