From da002d8b39aab3db65e594dbec048db35f5a6392 Mon Sep 17 00:00:00 2001 From: Alexander Kolbasov Date: Wed, 18 Oct 2017 18:59:59 -0700 Subject: [PATCH] HIVE-17806 Create directory for metrics file if it doesn't exist --- .../metrics/metrics2/JsonFileMetricsReporter.java | 35 ++++++++++++++++------ .../metrics/metrics2/TestCodahaleMetrics.java | 12 ++++++++ .../hive/metastore/metrics/JsonReporter.java | 29 +++++++++++++----- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java b/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java index 96243cb74a..b9be8bd182 100644 --- a/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java +++ b/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java @@ -77,6 +77,9 @@ // Permissions for the metrics file private static final FileAttribute> FILE_ATTRS = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-r--r--")); + // Permissions for metric directory + private static final FileAttribute> DIR_ATTRS = + PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x")); // Thread name for reporter thread private static final String JSON_REPORTER_THREAD_NAME = "json-metric-reporter"; @@ -86,8 +89,8 @@ private final long interval; // Location of JSON file private final Path path; - // tmpdir is the dirname(path) - private final Path tmpDir; + // Directory where path resides + private final Path metricsDir; public JsonFileMetricsReporter(MetricRegistry registry, HiveConf conf) { this.metricRegistry = registry; @@ -99,13 +102,25 @@ public JsonFileMetricsReporter(MetricRegistry registry, HiveConf conf) { String pathString = conf.getVar(HiveConf.ConfVars.HIVE_METRICS_JSON_FILE_LOCATION); path = Paths.get(pathString).toAbsolutePath(); LOGGER.info("Reporting metrics to {}", path); - // We want to use tmpDir i the same directory as the destination file to support atomic + // We want to use metricsDir in the same directory as the destination file to support atomic // move of temp file to the destination metrics file - tmpDir = path.getParent(); + metricsDir = path.getParent(); } @Override public void start() { + // Create metrics directory if it is not present + if (!metricsDir.toFile().exists()) { + LOGGER.warn("Metrics directory {} does not exist, creating one", metricsDir); + try { + // createDirectories creates all non-existent parent directories + Files.createDirectories(metricsDir, DIR_ATTRS); + } catch (IOException e) { + LOGGER.error("Failed to create directory {}: {}", metricsDir, e.getMessage()); + return; + } + } + executorService = Executors.newScheduledThreadPool(1, new ThreadFactoryBuilder().setNameFormat(JSON_REPORTER_THREAD_NAME).build()); executorService.scheduleWithFixedDelay(this,0, interval, TimeUnit.MILLISECONDS); @@ -113,7 +128,9 @@ public void start() { @Override public void close() { - executorService.shutdown(); + if (executorService != null) { + executorService.shutdown(); + } } @Override @@ -131,7 +148,7 @@ public void run() { // Metrics are first dumped to a temp file which is then renamed to the destination try { - tmpFile = Files.createTempFile(tmpDir, "hmetrics", "json", FILE_ATTRS); + tmpFile = Files.createTempFile(metricsDir, "hmetrics", "json", FILE_ATTRS); } catch (IOException e) { LOGGER.error("failed to create temp file for JSON metrics", e); return; @@ -157,8 +174,8 @@ public void run() { try { Files.move(tmpFile, path, StandardCopyOption.REPLACE_EXISTING); } catch (Exception e) { - LOGGER.error("Unable to rename temp file {} to {}", tmpFile, path, e); - return; + LOGGER.error("Unable to rename temp file {} to {}", tmpFile, path); + LOGGER.error("Exception during rename", e); } } catch (Throwable t) { // catch all errors (throwable and execptions to prevent subsequent tasks from being suppressed) @@ -170,7 +187,7 @@ public void run() { try { Files.delete(tmpFile); } catch (Exception e) { - LOGGER.error("failed to delete yemporary metrics file {}", tmpFile, e); + LOGGER.error("failed to delete temporary metrics file " + tmpFile, e); } } } diff --git a/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java b/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java index 254af7d431..cfc3a479ac 100644 --- a/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java +++ b/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java @@ -35,6 +35,10 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.FileReader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -48,15 +52,23 @@ */ public class TestCodahaleMetrics { + private static final Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir")); private static File jsonReportFile; private static MetricRegistry metricRegistry; private static final long REPORT_INTERVAL_MS = 100; @BeforeClass public static void setUp() throws Exception { + if (!tmpDir.toFile().exists()) { + System.out.println("Creating directory " + tmpDir); + Files.createDirectories(tmpDir, + PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x"))); + } + HiveConf conf = new HiveConf(); jsonReportFile = File.createTempFile("TestCodahaleMetrics", ".json"); + System.out.println("Json metrics saved in " + jsonReportFile.getAbsolutePath()); conf.setVar(HiveConf.ConfVars.HIVE_METRICS_CLASS, CodahaleMetrics.class.getCanonicalName()); conf.setVar(HiveConf.ConfVars.HIVE_CODAHALE_METRICS_REPORTER_CLASSES, diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java index f71bb25463..04a5f02a91 100644 --- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java +++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java @@ -82,13 +82,16 @@ private static final FileAttribute> FILE_ATTRS = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-r--r--")); + // Permissions for metric directory + private static final FileAttribute> DIR_ATTRS = + PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x")); private final MetricRegistry registry; private ObjectWriter jsonWriter; // Location of JSON file private final Path path; - // tmpdir is the dirname(path) - private final Path tmpDir; + // Directory where path resides + private final Path metricsDir; private JsonReporter(MetricRegistry registry, String name, MetricFilter filter, TimeUnit rateUnit, TimeUnit durationUnit, Configuration conf) { @@ -96,14 +99,25 @@ private JsonReporter(MetricRegistry registry, String name, MetricFilter filter, String pathString = MetastoreConf.getVar(conf, MetastoreConf.ConfVars .METRICS_JSON_FILE_LOCATION); path = Paths.get(pathString).toAbsolutePath(); LOG.info("Reporting metrics to {}", path); - // We want to use tmpDir i the same directory as the destination file to support atomic + // We want to use metricsDir in the same directory as the destination file to support atomic // move of temp file to the destination metrics file - tmpDir = path.getParent(); + metricsDir = path.getParent(); this.registry = registry; } @Override public void start(long period, TimeUnit unit) { + // Create metrics directory if it is not present + if (!metricsDir.toFile().exists()) { + LOG.warn("Metrics directory {} does not exist, creating one", metricsDir); + try { + // createDirectories creates all non-existent parent directories + Files.createDirectories(metricsDir, DIR_ATTRS); + } catch (IOException e) { + LOG.warn("Failed to initialize JSON reporter: failed to create directory {}: {}", metricsDir, e.getMessage()); + return; + } + } jsonWriter = new ObjectMapper().registerModule(new MetricsModule(TimeUnit.MILLISECONDS, TimeUnit.MILLISECONDS, false)).writerWithDefaultPrettyPrinter(); super.start(period, unit); @@ -125,7 +139,7 @@ public void report(SortedMap sortedMap, SortedMap sortedMap, SortedMap sortedMap, SortedMap