ctubbsii commented on code in PR #5019:
URL: https://github.com/apache/accumulo/pull/5019#discussion_r1823333456
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -441,11 +438,7 @@ public CompactionStats call() throws IOException,
CompactionCanceledException {
iters.add(iter);
} catch (Exception e) {
-
- ProblemReports.getInstance(context).report(new
ProblemReport(extent.tableId(),
- ProblemType.FILE_READ, dataFile.getNormalizedPathStr(), e));
-
- log.warn("Some problem opening data file {} {}", dataFile,
e.getMessage(), e);
+ log.warn("Some problem opening data file {} {} {}", dataFile, extent,
e.getMessage(), e);
Review Comment:
For these log messages, I don't think it's useful to print the
`e.getMessage()` separately from the stack trace like this. Sometimes, it can
be useful if it makes sense in context, but here, it's just printing it rather
than providing any additional context. So, it's enough to just include the
exception, which will automatically include the exception's message in the
stack trace.
```suggestion
log.warn("Some problem opening data file {} {}", dataFile, extent,
e);
```
I think this same change can be made for all of these.
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java:
##########
@@ -114,6 +118,8 @@ public void upgradeMetadata(ServerContext context) {
removeCompactColumnsFromTable(context, AccumuloTable.METADATA.tableName());
LOG.info("Removing bulk file columns from metadata table");
removeBulkFileColumnsFromTable(context,
AccumuloTable.METADATA.tableName());
+ LOG.info("Removing problems reports from metadata table");
+ removeMetadataProblemReports(context);
Review Comment:
Should we show the existing problems if any are found, or should we require
that no problems exist before an upgrade? (I'm leaning towards the former).
My concern would be that an upgrade erases evidence of an unresolved problem
that should be addressed.
--
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]
