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 <[email protected]> Authored: Mon Nov 21 10:45:56 2016 +0100 Committer: Stephan Erb <[email protected]> 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();
