Permalink
Browse files

Misc. fixes (#173)

* Fix some SLF4J logger calls

These issues were identified by SLF4J Helper for NetBeans IDE:
http://plugins.netbeans.org/plugin/72557/

* Delete an extra comma

* Log the case where an exception occurs

* Utilize parameterized logging and remove calls to toString()/String.format()

As suggested in the SLF4J FAQ: https://www.slf4j.org/faq.html#logging_performance
parameterized logging can improve the efficiency of logging calls when
logging at the specified level is disabled.

In addition, per the FAQ: https://www.slf4j.org/faq.html#paramException
since SLF4J 1.6.0 it is possible to use parameterized logging and also
log an exception/throwable.

toString() is called automatically: https://www.slf4j.org/faq.html#string_contents

* Add line breaks to some long lines
  • Loading branch information...
Daniel Trebbien authored and tgianos committed Jul 25, 2017
1 parent c5177a9 commit 1578d79326c0275ba872bf76e84b858832b68489
@@ -81,7 +81,7 @@ public boolean evaluatePartitionExpression(final String partitionExpression, fin
} catch (IllegalArgumentException e) {
throw e;
} catch (Throwable t) {
log.warn("Caught unexpected exception during evaluatePartitionExpression,", t);
log.warn("Caught unexpected exception during evaluatePartitionExpression", t);
return false;
}
}
@@ -115,14 +115,14 @@ public DatabaseInfo get(
@Nonnull @NonNull final QualifiedName name
) {
final String keyspace = name.getDatabaseName();
log.debug("Attempting to get keyspace metadata for keyspace {} for request", keyspace, context);
log.debug("Attempting to get keyspace metadata for keyspace {} for request {}", keyspace, context);
try {
final KeyspaceMetadata keyspaceMetadata = this.getCluster().getMetadata().getKeyspace(keyspace);
if (keyspaceMetadata == null) {
throw new DatabaseNotFoundException(name);
}
log.debug("Successfully found the keyspace metadata for {} for request", name, context);
log.debug("Successfully found the keyspace metadata for {} for request {}", name, context);
return DatabaseInfo.builder().name(name).build();
} catch (final DriverException de) {
log.error(de.getMessage(), de);
@@ -172,7 +172,7 @@ public boolean exists(
@Nonnull @NonNull final QualifiedName name
) {
final String keyspace = name.getDatabaseName();
log.debug("Checking if keyspace {} exists for request", keyspace, context);
log.debug("Checking if keyspace {} exists for request {}", keyspace, context);
try {
final boolean exists = this.getCluster().getMetadata().getKeyspace(keyspace) != null;
log.debug("Keyspace {} {} for request {}", keyspace, exists ? "exists" : "doesn't exist", context);
@@ -109,6 +109,8 @@ public MySqlConnectorDatabaseService(
log.debug("Finished listing database names for catalog {} for request {}", catalogName, context);
return results;
} catch (final SQLException se) {
log.debug("An exception occurred listing database names for catalog {} for request {}",
catalogName, context, se);
throw this.getExceptionMapper().toConnectorException(se, name);
}
}
@@ -105,7 +105,7 @@ public Type toMetacatType(@Nonnull @NonNull final String type) {
case "enum":
case "set":
default:
log.info("Encountered " + splitType[0] + " type. Returning Unknown type.");
log.info("Encountered {} type. Returning Unknown type.", splitType[0]);
return BaseType.UNKNOWN;
}
}
@@ -86,7 +86,7 @@ public void stop() {
try {
connectorFactory.stop();
} catch (Throwable t) {
log.error(String.format("Error shutting down connector: %s", connectorFactory.getName()), t);
log.error("Error shutting down connector: {}", connectorFactory.getName(), t);
}
});
}
@@ -124,7 +124,7 @@ public synchronized void createConnection(final String catalogName,
connectorContext.getConfiguration());
catalogs.put(catalogName, catalogConfig);
} else {
log.warn("No plugin for connector with type %s", connectorType);
log.warn("No plugin for connector with type {}", connectorType);
}
}
@@ -80,7 +80,7 @@ void counterIncrement(final String counterKey) {
if (counterHashMap.containsKey(counterKey)) {
this.counterHashMap.get(counterKey).increment();
} else {
log.error("SNS Notification does not suport counter for " + counterKey);
log.error("SNS Notification does not suport counter for {}", counterKey);
}
}
@@ -174,7 +174,7 @@ public void metacatDeleteTablePostEventHandler(final MetacatDeleteTablePostEvent
es.getIdsByQualifiedName(ElasticSearchDoc.Type.partition.name(), dto.getName());
es.delete(ElasticSearchDoc.Type.partition.name(), partitionIdsToBeDeleted);
} catch (Exception e) {
log.warn("Failed deleting the partitions for the dropped table/view:{}", dto.getName().toString());
log.warn("Failed deleting the partitions for the dropped table/view:{}", dto.getName());
}
}
});
@@ -261,8 +261,8 @@ private Void partitionsCleanUp(final QualifiedName tableName, final List<Qualifi
ElasticSearchDoc.Type.partition.name(),
Lists.newArrayList(tableName), refreshMarker, excludeQualifiedNames, PartitionDto.class);
if (!unmarkedPartitionDtos.isEmpty()) {
log.info("Start deleting unmarked partitions({}) for table {}", unmarkedPartitionDtos.size(),
tableName.toString());
log.info("Start deleting unmarked partitions({}) for table {}",
unmarkedPartitionDtos.size(), tableName);
try {
final List<String> unmarkedPartitionNames = unmarkedPartitionDtos.stream()
.map(p -> p.getDefinitionName().getPartitionName()).collect(Collectors.toList());
@@ -273,8 +273,8 @@ private Void partitionsCleanUp(final QualifiedName tableName, final List<Qualifi
p.getDefinitionName().getPartitionName()))
.map(p -> p.getDefinitionName().toString()).collect(Collectors.toList());
if (!partitionIds.isEmpty()) {
log.info("Deleting unused partitions({}) for table {}:{}", partitionIds.size(),
tableName.toString(), partitionIds);
log.info("Deleting unused partitions({}) for table {}:{}",
partitionIds.size(), tableName, partitionIds);
elasticSearchUtil.delete(ElasticSearchDoc.Type.partition.name(), partitionIds);
final List<HasMetadata> deletePartitionDtos = unmarkedPartitionDtos.stream()
.filter(
@@ -284,12 +284,12 @@ private Void partitionsCleanUp(final QualifiedName tableName, final List<Qualifi
userMetadataService.deleteMetadatas("admin", deletePartitionDtos);
}
} catch (Exception e) {
log.warn("Failed deleting the unmarked partitions for table {}", tableName.toString());
log.warn("Failed deleting the unmarked partitions for table {}", tableName);
}
log.info("End deleting unmarked partitions for table {}", tableName.toString());
log.info("End deleting unmarked partitions for table {}", tableName);
}
} catch (Exception e) {
log.warn("Failed getting the unmarked partitions for table {}", tableName.toString());
log.warn("Failed getting the unmarked partitions for table {}", tableName);
}
return null;
}
@@ -435,7 +435,7 @@ private void deleteUnmarkedEntities(final List<QualifiedName> qNames,
return result;
}).collect(Collectors.toList());
log.info("Unmarked tables({}): {}", unmarkedTableNames.size(), unmarkedTableNames);
log.info("Deleting tables({}): {}", deleteTableDtos.size());
log.info("Deleting tables({})", deleteTableDtos.size());
if (!deleteTableDtos.isEmpty()) {
final List<String> deleteTableNames = deleteTableDtos.stream().map(
dto -> dto.getName().toString()).collect(Collectors.toList());
@@ -560,7 +560,7 @@ private void deleteUnmarkedEntities(final List<QualifiedName> qNames,
databaseDto.getName().getDatabaseName(), s))
.collect(Collectors.toList());
log.info("Full refresh of database {} for tables({}): {}",
databaseDto.getName().toString(),
databaseDto.getName(),
databaseDto.getTables().size(), databaseDto.getTables());
return processTables(databaseDto.getName(), tableNames);
}).filter(NOT_NULL).collect(Collectors.toList());
@@ -220,7 +220,7 @@ private void updateDocs(final String type, final List<String> ids,
return null;
});
} catch (Exception e) {
log.error(String.format("Failed updating metadata of type %s with ids %s", type, ids), e);
log.error("Failed updating metadata of type {} with ids {}", type, ids, e);
this.elasticSearchMetric.getElasticSearchBulkUpdateFailureCounter().increment();
log("ElasticSearchUtil.updatDocs", type, ids.toString(), null, e.getMessage(), e, true);
}
@@ -532,7 +532,7 @@ private void hardDeleteDoc(final String type, final List<String> ids) {
return null;
});
} catch (Exception e) {
log.error(String.format("Failed deleting metadata of type %s with ids %s", type, ids), e);
log.error("Failed deleting metadata of type {} with ids {}", type, ids, e);
this.elasticSearchMetric.getElasticSearchBulkDeleteFailureCounter().increment();
log("ElasticSearchUtil.bulkDelete", type, ids.toString(), null, e.getMessage(), e, true);
}
@@ -598,7 +598,7 @@ private void ensureMigrationByCopy(final String type, final List<String> ids) {
*/
private void ensureMigrationBySave(final String type, final List<ElasticSearchDoc> docs) {
if (!Strings.isNullOrEmpty(config.getMergeEsIndex())) {
log.info("Bulk save to mergeEsIndex = " + config.getMergeEsIndex());
log.info("Bulk save to mergeEsIndex = {}", config.getMergeEsIndex());
bulkSaveToIndex(type, docs, config.getMergeEsIndex());
}
}
@@ -635,7 +635,7 @@ private void softDeleteDoc(
return null;
});
} catch (Exception e) {
log.error(String.format("Failed soft deleting metadata of type %s with ids %s", type, ids), e);
log.error("Failed soft deleting metadata of type {} with ids {}", type, ids, e);
this.elasticSearchMetric.getElasticSearchBulkDeleteFailureCounter().increment();
log("ElasticSearchUtil.bulkSoftDelete", type, ids.toString(), null, e.getMessage(), e, true);
}
@@ -656,9 +656,8 @@ private void saveToIndex(final String type, final String id, final String body,
return null;
});
} catch (Exception e) {
log.error(
String.format("Failed saving metadata of"
+ " index %s type %s with id %s: %s", index, type, id, body), e);
log.error("Failed saving metadata of"
+ " index {} type {} with id {}: {}", index, type, id, body, e);
this.elasticSearchMetric.getElasticSearchSaveFailureCounter().increment();
log("ElasticSearchUtil.saveToIndex", type, id, null, e.getMessage(), e, true, index);
}
@@ -710,7 +709,7 @@ private void bulkSaveToIndex(final String type, final List<ElasticSearchDoc> doc
return null;
});
} catch (Exception e) {
log.error(String.format("Failed saving metadatas of index %s type %s", index, type), e);
log.error("Failed saving metadatas of index {} type {}", index, type, e);
this.elasticSearchMetric.getElasticSearchBulkSaveFailureCounter().increment();
final List<String> docIds = docs.stream().map(ElasticSearchDoc::getId).collect(Collectors.toList());
log("ElasticSearchUtil.bulkSaveToIndex", type, docIds.toString(), null, e.getMessage(), e, true, index);
@@ -97,7 +97,7 @@ public void start() throws Exception {
log.info("initializing thrift server {}", getServerName());
final ThreadFactory threadFactory = new ThreadFactoryBuilder()
.setNameFormat(threadPoolNameFormat)
.setUncaughtExceptionHandler((t, e) -> log.error("Uncaught exception in thread: " + t.getName(), e))
.setUncaughtExceptionHandler((t, e) -> log.error("Uncaught exception in thread: {}", t.getName(), e))
.build();
final ExecutorService executorService = new ThreadPoolExecutor(
Math.min(2, config.getThriftServerMaxWorkerThreads()),
@@ -131,11 +131,12 @@ public void run() {
server.serve();
} catch (Throwable t) {
if (!stopping.get()) {
log.error("Unexpected exception in " + getServerName()
+ ". This probably means that the worker "
+ " pool was exhausted. Increase 'metacat.thrift.server_max_worker_threads' from "
+ config.getThriftServerMaxWorkerThreads() + " or throttle the number of requests. "
+ "This server thread is not in a bad state so starting a new one.", t);
log.error("Unexpected exception in {}. This probably "
+ "means that the worker pool was exhausted. "
+ "Increase 'metacat.thrift.server_max_worker_threads' "
+ "from {} or throttle the number of requests. "
+ "This server thread is not in a bad state so starting a new one.",
getServerName(), config.getThriftServerMaxWorkerThreads(), t);
startServing(executorService, serverTransport);
} else {
log.debug("stopping serving");
@@ -675,7 +675,9 @@ public boolean drop_partition_with_environment_context(
final QualifiedName partitionName = getPartitionDtoByName(tableDto, partName).getName();
if (deleteData) {
log.warn("Ignoring command to delete data for {}/{}/{}/{}", partitionName);
log.warn("Ignoring command to delete data for {}/{}/{}/{}",
catalogName, tableDto.getName().getDatabaseName(), tableDto.getName().getTableName(),
partitionName.getPartitionName());
}
partV1.deletePartitions(

0 comments on commit 1578d79

Please sign in to comment.