From 7aacb99d2f369cde699af36a4272e80f75401a65 Mon Sep 17 00:00:00 2001 From: Sid Narayan Date: Mon, 26 Oct 2020 13:50:34 -0700 Subject: [PATCH 1/3] PersistableSlidingWindow#load() modification - PersistableSlidingWindow#load() will no longer throw an IOException when it is called with a Path that doesn't exist --- .../framework/api/aggregators/PersistableSlidingWindow.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java index 60e00ba62..ad227dee6 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java @@ -72,6 +72,10 @@ protected synchronized void load(Path path) throws IOException { if (!enablePersistence) { return; } + if (!Files.exists(path)) { + LOG.debug("{}#load({}) called but the file doesn't exist", this.getClass().getSimpleName(), path); + return; + } LineIterator it = FileUtils.lineIterator(path.toFile(), "UTF-8"); try { while (it.hasNext()) { From ed0ab850cf3005e8e92e33e485d375550cc877e7 Mon Sep 17 00:00:00 2001 From: Sid Narayan Date: Mon, 26 Oct 2020 16:49:22 -0700 Subject: [PATCH 2/3] Move load() validation resposiblity to caller --- .../aggregators/PersistableSlidingWindow.java | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java index ad227dee6..5817ecf35 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java @@ -49,17 +49,15 @@ public PersistableSlidingWindow(int slidingWindowSize, Path filePath) { super(slidingWindowSize, timeUnit); this.pathToFile = filePath; - if (this.pathToFile == null) { - this.enablePersistence = false; - } else { - this.enablePersistence = true; - // setting the last write time to now will cause our first write to occur 5 minutes after construction - this.lastWriteTimeEpochMs = Instant.now().toEpochMilli(); - try { + this.enablePersistence = this.pathToFile != null; + // setting the last write time to now will cause our first write to occur 5 minutes after construction + this.lastWriteTimeEpochMs = Instant.now().toEpochMilli(); + try { + if (enablePersistence && Files.exists(filePath)) { load(this.pathToFile); - } catch (IOException ex) { - LOG.error("Unable to load previous data from {} into {}", this.pathToFile, getClass().getSimpleName()); } + } catch (IOException ex) { + LOG.error("Unable to load previous data from {} into {}", this.pathToFile, getClass().getSimpleName()); } } @@ -69,13 +67,6 @@ public PersistableSlidingWindow(int slidingWindowSize, * @throws IOException If there is an error reading the file */ protected synchronized void load(Path path) throws IOException { - if (!enablePersistence) { - return; - } - if (!Files.exists(path)) { - LOG.debug("{}#load({}) called but the file doesn't exist", this.getClass().getSimpleName(), path); - return; - } LineIterator it = FileUtils.lineIterator(path.toFile(), "UTF-8"); try { while (it.hasNext()) { From 372dd94613d270e737360394ebdf514f51b6f7bd Mon Sep 17 00:00:00 2001 From: Sid Narayan Date: Tue, 27 Oct 2020 10:56:08 -0700 Subject: [PATCH 3/3] Added more logging --- .../aggregators/PersistableSlidingWindow.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java index 5817ecf35..49364f6c5 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/framework/api/aggregators/PersistableSlidingWindow.java @@ -50,14 +50,21 @@ public PersistableSlidingWindow(int slidingWindowSize, super(slidingWindowSize, timeUnit); this.pathToFile = filePath; this.enablePersistence = this.pathToFile != null; - // setting the last write time to now will cause our first write to occur 5 minutes after construction + if (!enablePersistence) { + LOG.debug("Persistence is not enabled for {}:{}", this.getClass().getSimpleName(), this); + return; + } this.lastWriteTimeEpochMs = Instant.now().toEpochMilli(); - try { - if (enablePersistence && Files.exists(filePath)) { + if (Files.exists(filePath)) { + try { load(this.pathToFile); + } catch (IOException ex) { + LOG.error("Unable to load previous data from {} into {}", this.pathToFile, + getClass().getSimpleName(), ex); } - } catch (IOException ex) { - LOG.error("Unable to load previous data from {} into {}", this.pathToFile, getClass().getSimpleName()); + } else { + LOG.warn("{}:{} attempted to load data from {}, but the file doesn't exist", + this.getClass().getSimpleName(), this, filePath); } }