You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by se...@apache.org on 2016/11/21 09:46:17 UTC
aurora git commit: Adopt built-in string formatting in
Preconditions.checkState and our logger.
Repository: aurora
Updated Branches:
refs/heads/master 91ec949bb -> 99164834a
Adopt built-in string formatting in Preconditions.checkState and our logger.
Inspired by https://reviews.apache.org/r/53928/ this replaces many usages of
`String.format` with the built-in formatting in `Preconditions.checkState` and
our logger. This has the advantage that the formatting is only done when
necessary. A couple of other usages are replaced with `String.join` or simple
string concatenation which tends to be faster than the more powerful
`Sting.format`.
Reviewed at https://reviews.apache.org/r/53933/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/99164834
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/99164834
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/99164834
Branch: refs/heads/master
Commit: 99164834a3f23fa50576d2044743d2cb6d7da535
Parents: 91ec949
Author: Stephan Erb <se...@apache.org>
Authored: Mon Nov 21 10:45:56 2016 +0100
Committer: Stephan Erb <se...@apache.org>
Committed: Mon Nov 21 10:45:56 2016 +0100
----------------------------------------------------------------------
.../apache/aurora/scheduler/TaskStatusHandlerImpl.java | 4 +---
.../org/apache/aurora/scheduler/events/Webhook.java | 2 +-
.../apache/aurora/scheduler/http/LeaderRedirect.java | 4 ++--
.../aurora/scheduler/mesos/MesosTaskFactory.java | 6 +++---
.../apache/aurora/scheduler/offers/OfferManager.java | 2 +-
.../aurora/scheduler/pruning/TaskHistoryPruner.java | 2 +-
.../aurora/scheduler/reconciliation/TaskTimeout.java | 2 +-
.../aurora/scheduler/resources/ResourceMapper.java | 4 +++-
.../aurora/scheduler/scheduling/TaskScheduler.java | 7 ++++---
.../aurora/scheduler/state/StateManagerImpl.java | 13 +++++++++----
.../apache/aurora/scheduler/state/TaskAssigner.java | 3 +--
.../scheduler/thrift/aop/LoggingInterceptor.java | 8 ++++----
.../scheduler/updater/JobUpdateControllerImpl.java | 2 +-
13 files changed, 32 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java b/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java
index 51215b6..6afafe8 100644
--- a/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java
@@ -62,8 +62,6 @@ public class TaskStatusHandlerImpl extends AbstractExecutionThreadService
@VisibleForTesting
static final String DISK_LIMIT_DISPLAY = "Task used more disk than requested.";
- private static final String STATUS_STAT_FORMAT = "status_update_%s_%s";
-
private final Storage storage;
private final StateManager stateManager;
private final Driver driver;
@@ -181,7 +179,7 @@ public class TaskStatusHandlerImpl extends AbstractExecutionThreadService
@VisibleForTesting
static String statName(TaskStatus status, StateChangeResult result) {
- return String.format(STATUS_STAT_FORMAT, status.getReason(), result);
+ return "status_update_" + status.getReason() + "_" + result;
}
private static Optional<String> formatMessage(TaskStatus status) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/events/Webhook.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/events/Webhook.java b/src/main/java/org/apache/aurora/scheduler/events/Webhook.java
index 321cab3..3e8e38a 100644
--- a/src/main/java/org/apache/aurora/scheduler/events/Webhook.java
+++ b/src/main/java/org/apache/aurora/scheduler/events/Webhook.java
@@ -70,7 +70,7 @@ public class Webhook implements EventSubscriber {
*/
@Subscribe
public void taskChangedState(TaskStateChange stateChange) {
- LOG.debug("Got an event: " + stateChange.toString());
+ LOG.debug("Got an event: {}", stateChange.toString());
// Old state is not present because a scheduler just failed over. In that case we do not want to
// resend the entire state.
if (stateChange.getOldState().isPresent()) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
index 6ea780c..662d6d5 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
@@ -194,10 +194,10 @@ class LeaderRedirect implements Closeable {
LOG.warn("No serviceGroupMonitor in host set, will not redirect despite not being leader.");
return Optional.absent();
case 1:
- LOG.debug("Found leader scheduler at " + hostSet);
+ LOG.debug("Found leader scheduler at {}", hostSet);
return Optional.of(Iterables.getOnlyElement(hostSet));
default:
- LOG.error("Multiple serviceGroupMonitor detected, will not redirect: " + hostSet);
+ LOG.error("Multiple serviceGroupMonitor detected, will not redirect: {}", hostSet);
return Optional.absent();
}
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java b/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
index abbe81a..3d5c3bd 100644
--- a/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
@@ -120,7 +120,7 @@ public interface MesosTaskFactory {
}
private static String getJobSourceName(IJobKey jobkey) {
- return String.format("%s.%s.%s", jobkey.getRole(), jobkey.getEnvironment(), jobkey.getName());
+ return String.join(".", jobkey.getRole(), jobkey.getEnvironment(), jobkey.getName());
}
private static String getJobSourceName(ITaskConfig task) {
@@ -133,12 +133,12 @@ public interface MesosTaskFactory {
@VisibleForTesting
static String getInstanceSourceName(ITaskConfig task, int instanceId) {
- return String.format("%s.%s", getJobSourceName(task), instanceId);
+ return String.join(".", getJobSourceName(task), Integer.toString(instanceId));
}
@VisibleForTesting
static String getInverseJobSourceName(IJobKey job) {
- return String.format("%s.%s.%s", job.getName(), job.getEnvironment(), job.getRole());
+ return String.join(".", job.getName(), job.getEnvironment(), job.getRole());
}
private static byte[] serializeTask(IAssignedTask task) throws SchedulerException {
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
index 3b56921..6c2b6d2 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
@@ -201,7 +201,7 @@ public interface OfferManager extends EventSubscriber {
}
void decline(OfferID id) {
- LOG.debug("Declining offer " + id);
+ LOG.debug("Declining offer {}", id);
driver.declineOffer(id, getOfferFilter());
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java b/src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java
index c672826..bb973ca 100644
--- a/src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java
+++ b/src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java
@@ -151,7 +151,7 @@ public class TaskHistoryPruner implements EventSubscriber {
final String taskId,
long timeRemaining) {
- LOG.debug("Prune task " + taskId + " in " + timeRemaining + " ms.");
+ LOG.debug("Prune task {} in {} ms.", taskId, timeRemaining);
executor.execute(
shutdownOnError(
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskTimeout.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskTimeout.java b/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskTimeout.java
index a90cbff..2dc9bc2 100644
--- a/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskTimeout.java
+++ b/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskTimeout.java
@@ -131,7 +131,7 @@ class TaskTimeout extends AbstractIdleService implements EventSubscriber {
} else {
// Our service is not yet started. We don't want to lose track of the task, so
// we will try again later.
- LOG.debug("Retrying timeout of task " + taskId + " in " + NOT_STARTED_RETRY);
+ LOG.debug("Retrying timeout of task {} in {}", taskId, NOT_STARTED_RETRY);
// TODO(wfarner): This execution should not wait for a transaction, but a second executor
// would be weird.
executor.execute(this, NOT_STARTED_RETRY);
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java b/src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java
index ccfd997..dc57d57 100644
--- a/src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java
@@ -80,7 +80,9 @@ public interface ResourceMapper<T> {
checkState(
availablePorts.size() >= requestedPorts.size(),
- String.format("Insufficient ports %d when matching %s", availablePorts.size(), task));
+ "Insufficient ports %d when matching %s",
+ availablePorts.size(),
+ task);
Iterator<Integer> ports = availablePorts.iterator();
Map<String, Integer> portMap =
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java
index 31edb1d..203f62b 100644
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java
+++ b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java
@@ -135,14 +135,14 @@ public interface TaskScheduler extends EventSubscriber {
private Set<String> scheduleTasks(MutableStoreProvider store, Iterable<String> tasks) {
ImmutableSet<String> taskIds = ImmutableSet.copyOf(tasks);
String taskIdValues = Joiner.on(",").join(taskIds);
- LOG.debug("Attempting to schedule tasks " + taskIdValues);
+ LOG.debug("Attempting to schedule tasks {}", taskIdValues);
ImmutableSet<IAssignedTask> assignedTasks =
ImmutableSet.copyOf(Iterables.transform(
store.getTaskStore().fetchTasks(Query.taskScoped(taskIds).byStatus(PENDING)),
IScheduledTask::getAssignedTask));
if (Iterables.isEmpty(assignedTasks)) {
- LOG.warn("Failed to look up all tasks in a scheduling round: " + taskIdValues);
+ LOG.warn("Failed to look up all tasks in a scheduling round: {}", taskIdValues);
return taskIds;
}
@@ -151,7 +151,8 @@ public interface TaskScheduler extends EventSubscriber {
.collect(Collectors.groupingBy(t -> t.getTask()))
.entrySet()
.size() == 1,
- "Found multiple task groups for " + taskIdValues);
+ "Found multiple task groups for %s",
+ taskIdValues);
Map<String, IAssignedTask> assignableTaskMap =
assignedTasks.stream().collect(toMap(t -> t.getTaskId(), t -> t));
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
index 1c4a621..7b70c41 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
@@ -190,7 +190,9 @@ public class StateManagerImpl implements StateManager {
Preconditions.checkState(
changeResult == SUCCESS,
- "Attempt to assign task " + taskId + " to " + slaveHost + " failed");
+ "Attempt to assign task %s to %s failed",
+ taskId,
+ slaveHost);
return mutated.getAssignedTask();
}
@@ -285,7 +287,8 @@ public class StateManagerImpl implements StateManager {
case SAVE_STATE:
Preconditions.checkState(
upToDateTask.isPresent(),
- "Operation expected task " + taskId + " to be present.");
+ "Operation expected task %s to be present.",
+ taskId);
Optional<IScheduledTask> mutated = taskStore.mutateTask(taskId, task1 -> {
ScheduledTask mutableTask = task1.newBuilder();
@@ -303,7 +306,8 @@ public class StateManagerImpl implements StateManager {
case RESCHEDULE:
Preconditions.checkState(
upToDateTask.isPresent(),
- "Operation expected task " + taskId + " to be present.");
+ "Operation expected task %s to be present.",
+ taskId);
LOG.info("Task being rescheduled: " + taskId);
ScheduleStatus newState;
@@ -340,7 +344,8 @@ public class StateManagerImpl implements StateManager {
case DELETE:
Preconditions.checkState(
upToDateTask.isPresent(),
- "Operation expected task " + taskId + " to be present.");
+ "Operation expected task %s to be present.",
+ taskId);
events.add(deleteTasks(taskStore, ImmutableSet.of(taskId)));
break;
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java b/src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
index 4c61762..c1e7b03 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
@@ -203,8 +203,7 @@ public interface TaskAssigner {
// Never attempt to match this offer/groupKey pair again.
offerManager.banOffer(offer.getOffer().getId(), groupKey);
}
- LOG.debug("Agent " + offer.getOffer().getHostname()
- + " vetoed task " + taskId + ": " + vetoes);
+ LOG.debug("Agent {} vetoed task {}: {}", offer.getOffer().getHostname(), taskId, vetoes);
}
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java
index 3f11d8e..fc89c81 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java
@@ -74,18 +74,18 @@ class LoggingInterceptor implements MethodInterceptor {
}
}
String methodName = invocation.getMethod().getName();
- String message = String.format("%s(%s)", methodName, String.join(", ", argStrings));
- LOG.info(message);
+ String messageArgs = String.join(", ", argStrings);
+ LOG.info("{}({})", methodName, messageArgs);
try {
return invocation.proceed();
} catch (Storage.TransientStorageException e) {
- LOG.warn("Uncaught transient exception while handling " + message, e);
+ LOG.warn("Uncaught transient exception while handling {}({})", methodName, messageArgs, e);
return Responses.addMessage(Responses.empty(), ResponseCode.ERROR_TRANSIENT, e);
} catch (RuntimeException e) {
// We need shiro's exceptions to bubble up to the Shiro servlet filter so we intentionally
// do not swallow them here.
Throwables.throwIfInstanceOf(e, ShiroException.class);
- LOG.warn("Uncaught exception while handling " + message, e);
+ LOG.warn("Uncaught exception while handling {}({})", methodName, messageArgs, e);
return Responses.addMessage(Responses.empty(), ResponseCode.ERROR, e);
}
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/99164834/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
index 2a92a5a..80e97d7 100644
--- a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
@@ -488,7 +488,7 @@ class JobUpdateControllerImpl implements JobUpdateController {
if (action == ROLL_BACK) {
updates.remove(job);
} else {
- checkState(!updates.containsKey(job), "Updater already exists for " + job);
+ checkState(!updates.containsKey(job), "Updater already exists for %s", job);
}
IJobUpdate jobUpdate = updateStore.fetchJobUpdate(key).get();