CTTY commented on code in PR #17655:
URL: https://github.com/apache/hudi/pull/17655#discussion_r2659281781
##########
hudi-common/src/main/java/org/apache/hudi/internal/schema/io/FileBasedInternalSchemaStorageManager.java:
##########
@@ -96,7 +95,7 @@ public void persistHistorySchemaStr(String instantTime,
String historySchemaStr)
timeline.saveAsComplete(false, metaClient.createNewInstant(
HoodieInstant.State.INFLIGHT, hoodieInstant.getAction(),
hoodieInstant.requestedTime()),
Option.of(HoodieInstantWriter.convertByteArrayToWriter(writeContent)));
- LOG.info(String.format("persist history schema success on commit time:
%s", instantTime));
+ log.info(String.format("persist history schema success on commit time:
%s", instantTime));
Review Comment:
nit: we can switch to use slf4j parameterized logging
##########
hudi-common/src/main/java/org/apache/hudi/metrics/m3/M3MetricsReporter.java:
##########
@@ -55,7 +54,7 @@ public M3MetricsReporter(HoodieMetricsConfig metricsConfig,
MetricRegistry regis
tagBuilder.put("service", metricsConfig.getM3Service());
tagBuilder.put("env", metricsConfig.getM3Env());
this.tags = tagBuilder.build();
- LOG.info(String.format("Building M3 Reporter with M3 tags mapping: %s",
tags));
+ log.info(String.format("Building M3 Reporter with M3 tags mapping: %s",
tags));
Review Comment:
Logging can be improved
##########
hudi-common/src/main/java/org/apache/hudi/metrics/m3/M3MetricsReporter.java:
##########
@@ -107,7 +106,7 @@ public void report() {
scopeReporter.report();
scopeReporter.stop();
} catch (Exception e) {
- LOG.error(String.format("Error reporting metrics to M3: %s", e));
+ log.error(String.format("Error reporting metrics to M3: %s", e));
Review Comment:
Logging can be improved
##########
hudi-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -106,14 +106,14 @@ private List<MetricsReporter>
addAdditionalMetricsExporters(HoodieMetricsConfig
if (reporter.isPresent()) {
reporterList.add(reporter.get());
} else {
- LOG.error(String.format("Could not create reporter using properties
path %s base path %s",
+ log.error(String.format("Could not create reporter using properties
path %s base path %s",
Review Comment:
Logging can be improved
##########
hudi-common/src/main/java/org/apache/hudi/internal/schema/io/FileBasedInternalSchemaStorageManager.java:
##########
@@ -167,7 +166,7 @@ public String
getHistorySchemaStrByGivenValidCommits(List<String> validCommits)
byte[] content;
try (InputStream is = storage.open(latestFilePath)) {
content = FileIOUtils.readAsByteArray(is);
- LOG.info(String.format("read history schema success from file :
%s", latestFilePath));
+ log.info(String.format("read history schema success from file :
%s", latestFilePath));
Review Comment:
Same here
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -2271,17 +2270,17 @@ public static String
deleteMetadataTablePartition(HoodieTableMetaClient dataMeta
if (backup) {
final StoragePath metadataPartitionBackupPath = new
StoragePath(metadataTablePartitionPath.getParent().getParent(),
String.format(".metadata_%s_%s", partitionPath,
HoodieInstantTimeGenerator.getCurrentInstantTimeStr()));
- LOG.info("Backing up MDT partition {} to {} before deletion",
partitionPath, metadataPartitionBackupPath);
+ log.info("Backing up MDT partition {} to {} before deletion",
partitionPath, metadataPartitionBackupPath);
try {
if (storage.rename(metadataTablePartitionPath,
metadataPartitionBackupPath)) {
return metadataPartitionBackupPath.toString();
}
} catch (Exception e) {
// If rename fails, we will try to delete the table instead
- LOG.error(String.format("Failed to backup MDT partition %s using
rename", partitionPath), e);
+ log.error(String.format("Failed to backup MDT partition %s using
rename", partitionPath), e);
Review Comment:
Logging can be improved
##########
hudi-common/src/main/java/org/apache/hudi/io/storage/HoodieNativeAvroHFileReader.java:
##########
@@ -140,7 +140,7 @@ public Set<Pair<String, Long>> filterRowKeys(Set<String>
candidateRowKeys) {
try {
return reader.seekTo(new UTF8StringKey(k)) ==
HFileReader.SEEK_TO_FOUND;
} catch (IOException e) {
- LOG.error("Failed to check key availability: " + k);
+ log.error("Failed to check key availability: " + k);
Review Comment:
Logging can be improved
##########
hudi-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -106,14 +106,14 @@ private List<MetricsReporter>
addAdditionalMetricsExporters(HoodieMetricsConfig
if (reporter.isPresent()) {
reporterList.add(reporter.get());
} else {
- LOG.error(String.format("Could not create reporter using properties
path %s base path %s",
+ log.error(String.format("Could not create reporter using properties
path %s base path %s",
propPath, metricConfig.getBasePath()));
}
}
} catch (IOException e) {
- LOG.error("Failed to add MetricsExporters", e);
+ log.error("Failed to add MetricsExporters", e);
}
- LOG.info("total additional metrics reporters added =" +
reporterList.size());
+ log.info("total additional metrics reporters added =" +
reporterList.size());
Review Comment:
Logging can be improved
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
