You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by GitBox <gi...@apache.org> on 2020/11/03 19:42:00 UTC
[GitHub] [ambari] hapylestat commented on a change in pull request #3254: AMBARI-25569 Reassess Ambari Metrics data migration - 2nd part
hapylestat commented on a change in pull request #3254:
URL: https://github.com/apache/ambari/pull/3254#discussion_r516771850
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -53,7 +53,7 @@ public AbstractPhoenixMetricsCopier(String inputTableName, String outputTableNam
@Override
public void run(){
LOG.info(String.format("Copying %s metrics from %s to %s", metricNames, inputTable, outputTable));
- final long startTimer = System.currentTimeMillis();
+ final long st = System.currentTimeMillis();
Review comment:
what is st? can we rename it to something more meaning like timerStart and estimatedTime for example to timerDelta
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -64,15 +64,15 @@ public void run(){
} catch (SQLException e) {
LOG.error(e);
} finally {
- final long estimatedTime = System.currentTimeMillis() - startTimer;
+ final long estimatedTime = System.currentTimeMillis() - st;
LOG.debug(String.format("Copying took %s seconds from table %s to table %s for metric names %s", estimatedTime/ 1000.0, inputTable, outputTable, metricNames));
saveMetricsProgress();
}
}
private String getMetricNamesLikeClause() {
- StringBuilder sb = new StringBuilder();
+ StringBuilder sb = new StringBuilder(256);
Review comment:
my god....can we compact sb.append clauses below like
sb
.append("METRIC_NAME LIKE '")
.append(metricName)
.append("'");
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -53,7 +53,7 @@ public AbstractPhoenixMetricsCopier(String inputTableName, String outputTableNam
@Override
public void run(){
LOG.info(String.format("Copying %s metrics from %s to %s", metricNames, inputTable, outputTable));
- final long startTimer = System.currentTimeMillis();
+ final long st = System.currentTimeMillis();
Review comment:
no need for final due to "effectively final" concept since java 8
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -64,15 +64,15 @@ public void run(){
} catch (SQLException e) {
LOG.error(e);
} finally {
- final long estimatedTime = System.currentTimeMillis() - startTimer;
+ final long estimatedTime = System.currentTimeMillis() - st;
LOG.debug(String.format("Copying took %s seconds from table %s to table %s for metric names %s", estimatedTime/ 1000.0, inputTable, outputTable, metricNames));
saveMetricsProgress();
}
}
private String getMetricNamesLikeClause() {
- StringBuilder sb = new StringBuilder();
+ StringBuilder sb = new StringBuilder(256);
Review comment:
thx in advance
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -64,15 +64,15 @@ public void run(){
} catch (SQLException e) {
LOG.error(e);
} finally {
- final long estimatedTime = System.currentTimeMillis() - startTimer;
+ final long estimatedTime = System.currentTimeMillis() - st;
Review comment:
no need for final keyword
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/MetricsDataMigrationLauncher.java
##########
@@ -114,8 +112,29 @@ public MetricsDataMigrationLauncher(String whitelistedFilePath, String processed
LOG.info(String.format("Split metric names into %s batches with size of %s", metricNamesBatches.size(), batchSize));
}
+ private long calculateStartEpochTime(Long startDay) {
+ final long days;
+ if (startDay == null) {
+ LOG.info("No starting day have been provided.");
Review comment:
"No starting day have been provided, setting default: {DEFAULT_START_DAYS}"
##########
File path: ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/ambari/metrics/core/timeline/upgrade/core/AbstractPhoenixMetricsCopier.java
##########
@@ -116,18 +116,19 @@ private void saveMetricsProgress() {
LOG.info("Skipping metrics progress save as the file is null");
return;
}
- synchronized (this.processedMetricsFile) {
- for (String metricName : metricNames) {
- try {
+
+ for (String metricName : metricNames) {
+ try {
+ synchronized (this.processedMetricsFile) {
this.processedMetricsFile.append(inputTable).append(":").append(metricName).append(System.lineSeparator());
- } catch (IOException e) {
- LOG.error(e);
}
+ } catch (IOException e) {
+ LOG.error(e);
}
}
}
- protected String getQueryHint(Long startTime) {
+ protected String getQueryHint(long startTime) {
Review comment:
can we compact the sb.append here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org