You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2019/12/31 07:36:21 UTC
[GitHub] [incubator-hudi] sev7e0 opened a new pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
sev7e0 opened a new pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161
## What is the purpose of the pull request
Redo hudi-common log statements using SLF4J
## Brief change log
## Verify this pull request
This pull request is a trivial rework / code cleanup without any test coverage.
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362360577
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/FileSystemViewManager.java
##########
@@ -158,8 +158,8 @@ private static HoodieTableFileSystemView createInMemoryFileSystemView(Serializab
*/
private static RemoteHoodieTableFileSystemView createRemoteFileSystemView(SerializableConfiguration conf,
FileSystemViewStorageConfig viewConf, HoodieTableMetaClient metaClient) {
- LOG.info("Creating remote view for basePath " + metaClient.getBasePath() + ". Server="
- + viewConf.getRemoteViewServerHost() + ":" + viewConf.getRemoteViewServerPort());
+ LOG.info("Creating remote view for basePath {}. Server={}:{}",
+ metaClient.getBasePath(), viewConf.getRemoteViewServerHost(), viewConf.getRemoteViewServerPort());
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 closed pull request #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 closed pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364646974
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/SpillableMapBasedFileSystemView.java
##########
@@ -80,8 +79,7 @@ public SpillableMapBasedFileSystemView(HoodieTableMetaClient metaClient, HoodieT
protected Map<HoodieFileGroupId, Pair<String, CompactionOperation>> createFileIdToPendingCompactionMap(
Map<HoodieFileGroupId, Pair<String, CompactionOperation>> fgIdToPendingCompaction) {
try {
- LOG.info("Creating Pending Compaction map using external spillable Map. Max Mem=" + maxMemoryForPendingCompaction
- + ", BaseDir=" + baseStoreDir);
+ LOG.info("Creating Pending Compaction map using external spillable Map. Max Mem={}, BaseDir={}", maxMemoryForPendingCompaction, baseStoreDir);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364644370
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -324,7 +324,7 @@ public static HoodieTableMetaClient initTableType(Configuration hadoopConf, Stri
*/
public static HoodieTableMetaClient initDatasetAndGetMetaClient(Configuration hadoopConf, String basePath,
Properties props) throws IOException {
- LOG.info("Initializing " + basePath + " as hoodie dataset " + basePath);
+ LOG.info("Initializing {} as hoodie dataset {}", basePath, basePath);
Review comment:
change to `LOG.info("Initializing {} as hoodie dataset", basePath);`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362395706
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java
##########
@@ -222,8 +222,8 @@ public Writer build() throws IOException, InterruptedException {
// Use rollover write token as write token to create new log file with tokens
logWriteToken = rolloverLogWriteToken;
}
- LOG.info("Computed the next log version for " + logFileId + " in " + parentPath + " as " + logVersion
- + " with write-token " + logWriteToken);
+ LOG.info("Computed the next log version for {} in {} as {} with write-token {}",
+ logFileId, parentPath, logVersion, logWriteToken);
Review comment:
sorry, i remove it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364786423
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -157,8 +157,7 @@ protected boolean isPartitionAvailableInStore(String partitionPath) {
@Override
protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fileGroups) {
- LOG.info("Resetting and adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Resetting and adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}", partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
In the previous modification, I used two lines, but they were not adopted. I read our verification rules is `<property name="max" value="200"/>`, I'm a little confused now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363365
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/TimelineDiffHelper.java
##########
@@ -64,8 +64,8 @@ public static TimelineDiffResult getNewInstantsForIncrementalSync(HoodieTimeline
if (!lostPendingCompactions.isEmpty()) {
// If a compaction is unscheduled, fall back to complete refresh of fs view since some log files could have been
// moved. Its unsafe to incrementally sync in that case.
- LOG.warn("Some pending compactions are no longer in new timeline (unscheduled ?). They are :"
- + lostPendingCompactions);
+ LOG.warn("Some pending compactions are no longer in new timeline (unscheduled ?). They are :{}",
+ lostPendingCompactions);
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358217
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFileReader.java
##########
@@ -113,7 +113,7 @@ public void run() {
try {
close();
} catch (Exception e) {
- LOG.warn("unable to close input stream for log file " + logFile, e);
+ LOG.warn(String.format("unable to close input stream for log file %s", logFile), e);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362357721
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java
##########
@@ -101,8 +101,8 @@ public void trySave(int taskPartitionId) {
fs.rename(tmpMetaPath, metaPath);
}
} catch (IOException ioe) {
- LOG.warn("Error trying to save partition metadata (this is okay, as long as atleast 1 of these succced), "
- + partitionPath, ioe);
+ LOG.warn(String.format("Error trying to save partition metadata (this is okay, as long as atleast 1 of these "
Review comment:
Since we have used slf4j, we may not need to use `String.format`. The slf4j framework supports `placeholder`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363156
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/SpillableMapBasedFileSystemView.java
##########
@@ -66,8 +66,8 @@ public SpillableMapBasedFileSystemView(HoodieTableMetaClient metaClient, HoodieT
@Override
protected Map<String, List<HoodieFileGroup>> createPartitionToFileGroups() {
try {
- LOG.info("Creating Partition To File groups map using external spillable Map. Max Mem=" + maxMemoryForFileGroupMap
- + ", BaseDir=" + baseStoreDir);
+ LOG.info("Creating Partition To File groups map using external spillable Map. Max Mem={}, BaseDir={}",
+ maxMemoryForPendingCompaction, baseStoreDir);
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358621
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/AbstractTableFileSystemView.java
##########
@@ -123,8 +123,8 @@ protected void refreshTimeline(HoodieTimeline visibleActiveTimeline) {
}
});
long storePartitionsTs = timer.endTimer();
- LOG.info("addFilesToView: NumFiles=" + statuses.length + ", FileGroupsCreationTime=" + fgBuildTimeTakenMs
- + ", StoreTimeTaken=" + storePartitionsTs);
+ LOG.info("addFilesToView: NumFiles={}, FileGroupsCreationTime={}, StoreTimeTaken={}",
+ statuses.length, fgBuildTimeTakenMs, storePartitionsTs);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363091
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -157,8 +157,8 @@ protected boolean isPartitionAvailableInStore(String partitionPath) {
@Override
protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fileGroups) {
- LOG.info("Resetting and adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Resetting and adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}",
+ partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364646700
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -184,8 +183,7 @@ protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fi
// record that partition is loaded.
rocksDB.put(schemaHelper.getColFamilyForStoredPartitions(), lookupKey, Boolean.TRUE);
- LOG.info("Finished adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Finished adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}", partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363123
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -184,8 +184,8 @@ protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fi
// record that partition is loaded.
rocksDB.put(schemaHelper.getColFamilyForStoredPartitions(), lookupKey, Boolean.TRUE);
- LOG.info("Finished adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Finished adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}",
+ partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363178
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/SpillableMapBasedFileSystemView.java
##########
@@ -80,8 +80,8 @@ public SpillableMapBasedFileSystemView(HoodieTableMetaClient metaClient, HoodieT
protected Map<HoodieFileGroupId, Pair<String, CompactionOperation>> createFileIdToPendingCompactionMap(
Map<HoodieFileGroupId, Pair<String, CompactionOperation>> fgIdToPendingCompaction) {
try {
- LOG.info("Creating Pending Compaction map using external spillable Map. Max Mem=" + maxMemoryForPendingCompaction
- + ", BaseDir=" + baseStoreDir);
+ LOG.info("Creating Pending Compaction map using external spillable Map. Max Mem={}, BaseDir={}",
+ maxMemoryForPendingCompaction, baseStoreDir);
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364648898
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/collection/DiskBasedMap.java
##########
@@ -111,9 +111,7 @@ private void initFile(File writeOnlyFile) throws IOException {
writeOnlyFile.getParentFile().mkdir();
}
writeOnlyFile.createNewFile();
- LOG.info("Spilling to file location " + writeOnlyFile.getAbsolutePath() + " in host ("
- + InetAddress.getLocalHost().getHostAddress() + ") with hostname (" + InetAddress.getLocalHost().getHostName()
- + ")");
+ LOG.info("Spilling to file location {} in host ({}) with hostname ({})", writeOnlyFile.getAbsolutePath(), InetAddress.getLocalHost().getHostAddress(), InetAddress.getLocalHost().getHostName());
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on issue #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-570919373
> @sev7e0 Thanks for opening this PR, Could you please reopen and merge this PR to redo-log branch? more context can be found https://lists.apache.org/thread.html/9dc1f3a590413a5224a1a5ad835353e11b2b754e1ec7ad1ca0a55053%40%3Cdev.hudi.apache.org%3E, Thanks.
Ok~
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-570111790
Similar comment on how we are ensuring the log statements are indeed working properly inside spark and hive and presto after this change..
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364646565
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -157,8 +157,7 @@ protected boolean isPartitionAvailableInStore(String partitionPath) {
@Override
protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fileGroups) {
- LOG.info("Resetting and adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Resetting and adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}", partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
would be changed to two lines.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364786885
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/FailSafeConsistencyGuard.java
##########
@@ -95,7 +95,7 @@ public void waitForFilesVisibility(String dirPath, List<String> files, FileVisib
switch (event) {
case DISAPPEAR:
- LOG.info("Following files are visible" + candidateFiles);
+ LOG.info("Following files are visible{}", candidateFiles);
Review comment:
Ok, change it now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363473
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/collection/DiskBasedMap.java
##########
@@ -111,9 +111,8 @@ private void initFile(File writeOnlyFile) throws IOException {
writeOnlyFile.getParentFile().mkdir();
}
writeOnlyFile.createNewFile();
- LOG.info("Spilling to file location " + writeOnlyFile.getAbsolutePath() + " in host ("
- + InetAddress.getLocalHost().getHostAddress() + ") with hostname (" + InetAddress.getLocalHost().getHostName()
- + ")");
+ LOG.info("Spilling to file location {} in host ({}) with hostname ({})", writeOnlyFile.getAbsolutePath(),
+ InetAddress.getLocalHost().getHostAddress(), InetAddress.getLocalHost().getHostName());
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362383524
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java
##########
@@ -101,8 +101,8 @@ public void trySave(int taskPartitionId) {
fs.rename(tmpMetaPath, metaPath);
}
} catch (IOException ioe) {
- LOG.warn("Error trying to save partition metadata (this is okay, as long as atleast 1 of these succced), "
- + partitionPath, ioe);
+ LOG.warn(String.format("Error trying to save partition metadata (this is okay, as long as atleast 1 of these "
Review comment:
It's my error. I'll make changes right away
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358513
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java
##########
@@ -63,12 +63,12 @@
public static final SimpleDateFormat COMMIT_FORMATTER = new SimpleDateFormat("yyyyMMddHHmmss");
public static final Set<String> VALID_EXTENSIONS_IN_ACTIVE_TIMELINE = new HashSet<>(Arrays.asList(
- new String[]{COMMIT_EXTENSION, INFLIGHT_COMMIT_EXTENSION, REQUESTED_COMMIT_EXTENSION, DELTA_COMMIT_EXTENSION,
Review comment:
Can we revert this change? This PR aims to replace the log framework, right? Let's do one thing in a PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
smarthi commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362386748
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java
##########
@@ -101,8 +101,8 @@ public void trySave(int taskPartitionId) {
fs.rename(tmpMetaPath, metaPath);
}
} catch (IOException ioe) {
- LOG.warn("Error trying to save partition metadata (this is okay, as long as atleast 1 of these succced), "
- + partitionPath, ioe);
+ LOG.warn("Error trying to save partition metadata (this is okay, as long as atleast 1 of these succced), {}",
+ partitionPath, ioe);
Review comment:
remove extra indent ??
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362357743
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java
##########
@@ -111,7 +111,7 @@ public void trySave(int taskPartitionId) {
fs.delete(tmpMetaPath, false);
}
} catch (IOException ioe) {
- LOG.warn("Error trying to clean up temporary files for " + partitionPath, ioe);
+ LOG.warn(String.format("Error trying to clean up temporary files for %s", partitionPath), ioe);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on issue #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-572360741
cc @leesf
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364782798
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -324,7 +324,7 @@ public static HoodieTableMetaClient initTableType(Configuration hadoopConf, Stri
*/
public static HoodieTableMetaClient initDatasetAndGetMetaClient(Configuration hadoopConf, String basePath,
Properties props) throws IOException {
- LOG.info("Initializing " + basePath + " as hoodie dataset " + basePath);
+ LOG.info("Initializing {} as hoodie dataset {}", basePath, basePath);
Review comment:
Thanks, change it now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364648753
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/TimelineDiffHelper.java
##########
@@ -64,8 +64,7 @@ public static TimelineDiffResult getNewInstantsForIncrementalSync(HoodieTimeline
if (!lostPendingCompactions.isEmpty()) {
// If a compaction is unscheduled, fall back to complete refresh of fs view since some log files could have been
// moved. Its unsafe to incrementally sync in that case.
- LOG.warn("Some pending compactions are no longer in new timeline (unscheduled ?). They are :"
- + lostPendingCompactions);
+ LOG.warn("Some pending compactions are no longer in new timeline (unscheduled ?). They are :{}", lostPendingCompactions);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on issue #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-572872998
> > Thanks for opening this PR @sev7e0 ! Looks good overall, only left some minor comments.
>
> Thanks for your review @leesf ! I'm thinking we need reduce our verification rules about `LineLength` in checkstyle. Now is `<property name="max" value="200"/>`, how about 130?
200 is decided when introducing spotless plugin, It should play well with currently checkstyle, after bringing back the spotless plugin, let's evaluate it again. And currently keep it as is would be better.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358327
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java
##########
@@ -93,11 +93,11 @@
}
if (!isAppendSupported) {
this.logFile = logFile.rollOver(fs, rolloverLogWriteToken);
- LOG.info("Append not supported.. Rolling over to " + logFile);
+ LOG.info("Append not supported.. Rolling over to {}", logFile);
createNewFile();
}
} else {
- LOG.info(logFile + " does not exist. Create a new file");
+ LOG.info("{} does not exist. Create a new file", logFile.getPath());
Review comment:
Shall we use `logFile.getPath()` 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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364786423
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/RocksDbBasedFileSystemView.java
##########
@@ -157,8 +157,7 @@ protected boolean isPartitionAvailableInStore(String partitionPath) {
@Override
protected void storePartitionView(String partitionPath, List<HoodieFileGroup> fileGroups) {
- LOG.info("Resetting and adding new partition (" + partitionPath + ") to ROCKSDB based file-system view at "
- + config.getRocksdbBasePath() + ", Total file-groups=" + fileGroups.size());
+ LOG.info("Resetting and adding new partition ({}) to ROCKSDB based file-system view at {}, Total file-groups={}", partitionPath, config.getRocksdbBasePath(), fileGroups.size());
Review comment:
In the previous modification, I used two lines, but they were not adopted. I read our verification rules is `<property name="max" value="200"/>`, I'm a little confused now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358375
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java
##########
@@ -180,8 +180,8 @@ private Writer rolloverIfNeeded() throws IOException, InterruptedException {
if (getCurrentSize() > sizeThreshold) {
// TODO - make an end marker which seals the old log file (no more appends possible to that
// file).
- LOG.info("CurrentSize " + getCurrentSize() + " has reached threshold " + sizeThreshold
- + ". Rolling over to the next version");
+ LOG.info("CurrentSize {} has reached threshold {}. Rolling over to the next version",
+ getCurrentSize(), sizeThreshold);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363286
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/FailSafeConsistencyGuard.java
##########
@@ -104,7 +104,7 @@ public void waitForFilesVisibility(String dirPath, List<String> files, FileVisib
return candidateFiles.isEmpty();
}
} catch (IOException ioe) {
- LOG.warn("Got IOException waiting for file event. Have tried " + retryNum + " time(s)", ioe);
+ LOG.warn(String.format("Got IOException waiting for file event. Have tried %s time(s)", retryNum), ioe);
Review comment:
ditto about using placeholder
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf merged pull request #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf merged pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364648475
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/RocksDBDAO.java
##########
@@ -352,8 +352,8 @@ public void delete(String columnFamilyName, String key) {
}
}
- LOG.info("Prefix Search for (query=" + prefix + ") on " + columnFamilyName + ". Total Time Taken (msec)="
- + timer.endTimer() + ". Serialization Time taken(micro)=" + timeTakenMicro + ", num entries=" + results.size());
+ LOG.info("Prefix Search for (query={}) on {}. Total Time Taken (msec)={}. Serialization Time taken(micro)={}, num entries={}", prefix, columnFamilyName, timer.endTimer(), timeTakenMicro,
Review comment:
Too long, would be changed to two lines.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364647724
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/FailSafeConsistencyGuard.java
##########
@@ -95,7 +95,7 @@ public void waitForFilesVisibility(String dirPath, List<String> files, FileVisib
switch (event) {
case DISAPPEAR:
- LOG.info("Following files are visible" + candidateFiles);
+ LOG.info("Following files are visible{}", candidateFiles);
Review comment:
add a blank would be better `LOG.info("Following files are visible {}", candidateFiles);`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r364646882
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/view/SpillableMapBasedFileSystemView.java
##########
@@ -66,8 +66,7 @@ public SpillableMapBasedFileSystemView(HoodieTableMetaClient metaClient, HoodieT
@Override
protected Map<String, List<HoodieFileGroup>> createPartitionToFileGroups() {
try {
- LOG.info("Creating Partition To File groups map using external spillable Map. Max Mem=" + maxMemoryForFileGroupMap
- + ", BaseDir=" + baseStoreDir);
+ LOG.info("Creating Partition To File groups map using external spillable Map. Max Mem={}, BaseDir={}", maxMemoryForPendingCompaction, baseStoreDir);
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] leesf commented on issue #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
leesf commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-570736629
@sev7e0 Thanks for opening this PR, Could you please reopen and merge this PR to redo-log branch? more context can be found https://lists.apache.org/thread.html/9dc1f3a590413a5224a1a5ad835353e11b2b754e1ec7ad1ca0a55053%40%3Cdev.hudi.apache.org%3E, Thanks.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 opened a new pull request #1161:
[HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 opened a new pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161
## What is the purpose of the pull request
Redo hudi-common log statements using SLF4J
## Brief change log
## Verify this pull request
This pull request is a trivial rework / code cleanup without any test coverage.
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362363335
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/RocksDBDAO.java
##########
@@ -352,8 +352,8 @@ public void delete(String columnFamilyName, String key) {
}
}
- LOG.info("Prefix Search for (query=" + prefix + ") on " + columnFamilyName + ". Total Time Taken (msec)="
- + timer.endTimer() + ". Serialization Time taken(micro)=" + timeTakenMicro + ", num entries=" + results.size());
+ LOG.info("Prefix Search for (query={}) on {}. Total Time Taken (msec)={}. Serialization Time taken(micro)={}, "
+ + "num entries={}", prefix, columnFamilyName, timer.endTimer(), timeTakenMicro, results.size());
Review comment:
ditto about indent
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] sev7e0 commented on issue #1161: [HUDI-457]Redo
hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#issuecomment-572608900
> Thanks for opening this PR @sev7e0 ! Looks good overall, only left some minor comments.
Thanks for your review @leesf ! I'm thinking we need reduce our verification rules about `LineLength` in checkstyle. Now is `<property name="max" value="200"/>`, how about 130?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
smarthi commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362386992
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java
##########
@@ -222,8 +222,8 @@ public Writer build() throws IOException, InterruptedException {
// Use rollover write token as write token to create new log file with tokens
logWriteToken = rolloverLogWriteToken;
}
- LOG.info("Computed the next log version for " + logFileId + " in " + parentPath + " as " + logVersion
- + " with write-token " + logWriteToken);
+ LOG.info("Computed the next log version for {} in {} as {} with write-token {}",
+ logFileId, parentPath, logVersion, logWriteToken);
Review comment:
remove extra indent ???
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1161: [HUDI-457]Redo hudi-common log statements using SLF4J
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1161: [HUDI-457]Redo hudi-common log statements using SLF4J
URL: https://github.com/apache/incubator-hudi/pull/1161#discussion_r362358202
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordScanner.java
##########
@@ -196,34 +196,34 @@ public void scan() {
HoodieLogBlock lastBlock = currentInstantLogBlocks.peek();
// handle corrupt blocks separately since they may not have metadata
if (lastBlock.getBlockType() == CORRUPT_BLOCK) {
- LOG.info("Rolling back the last corrupted log block read in " + logFile.getPath());
+ LOG.info("Rolling back the last corrupted log block read in {}", logFile.getPath());
currentInstantLogBlocks.pop();
numBlocksRolledBack++;
} else if (lastBlock.getBlockType() != CORRUPT_BLOCK
&& targetInstantForCommandBlock.contentEquals(lastBlock.getLogBlockHeader().get(INSTANT_TIME))) {
// rollback last data block or delete block
- LOG.info("Rolling back the last log block read in " + logFile.getPath());
+ LOG.info("Rolling back the last log block read in {}", logFile.getPath());
currentInstantLogBlocks.pop();
numBlocksRolledBack++;
} else if (!targetInstantForCommandBlock
.contentEquals(currentInstantLogBlocks.peek().getLogBlockHeader().get(INSTANT_TIME))) {
// invalid or extra rollback block
- LOG.warn("TargetInstantTime " + targetInstantForCommandBlock
- + " invalid or extra rollback command block in " + logFile.getPath());
+ LOG.warn("TargetInstantTime {} invalid or extra rollback command block in {}",
+ targetInstantForCommandBlock, logFile.getPath());
Review comment:
Here, maybe there is one redundant indent?
----------------------------------------------------------------
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
With regards,
Apache Git Services