From 1a32be08487a6ed13bb35b0941846b265e75d9ed Mon Sep 17 00:00:00 2001 From: Hector Oswaldo Caballero Date: Fri, 17 Mar 2017 11:20:49 -0400 Subject: Fix project creation logs Log was showing project was created successfully even if it failed. Change-Id: Ie37e6f2f58e1c8b4e33f61bf2d81ee43698b8aa3 --- .../com/googlesource/gerrit/plugins/replication/ReplicationQueue.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java index 9a68d32..1370bd3 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java @@ -233,10 +233,8 @@ public class ReplicationQueue private boolean createProject(URIish replicateURI, String head) { if (!replicateURI.isRemote()) { createLocally(replicateURI, head); - repLog.info("Created local repository: " + replicateURI); } else if (isSSH(replicateURI)) { createRemoteSsh(replicateURI, head); - repLog.info("Created remote repository: " + replicateURI); } else { repLog.warn( String.format( @@ -258,6 +256,7 @@ public class ReplicationQueue u.disableRefLog(); u.link(head); } + repLog.info("Created local repository: {}", uri); } catch (IOException e) { repLog.error(String.format("Error creating local repository %s:\n", uri.getPath()), e); } @@ -272,6 +271,7 @@ public class ReplicationQueue OutputStream errStream = newErrorBufferStream(); try { executeRemoteSsh(uri, cmd, errStream); + repLog.info("Created remote repository: {}", uri); } catch (IOException e) { repLog.error( String.format( -- cgit v1.2.3 From 2752c2eef75df785cdba21ccd5441929559a986b Mon Sep 17 00:00:00 2001 From: Hector Oswaldo Caballero Date: Wed, 21 Feb 2018 21:17:51 -0500 Subject: Use logger built-in formatter Change-Id: I6b46309f07ca4c402ae7a51cfb462c1be6bec333 --- .../replication/AutoReloadConfigDecorator.java | 8 +- .../gerrit/plugins/replication/Destination.java | 7 +- .../gerrit/plugins/replication/PushOne.java | 52 ++++++------- .../replication/ReplicationFileBasedConfig.java | 4 +- .../plugins/replication/ReplicationQueue.java | 89 +++++++++++----------- 5 files changed, 78 insertions(+), 82 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java index 55c7072..8b6b8fc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java @@ -75,11 +75,9 @@ public class AutoReloadConfigDecorator implements ReplicationConfig { this.currentConfig = newConfig; this.currentConfigTs = lastModified; log.info( - "Configuration reloaded: " - + currentConfig.getDestinations(FilterType.ALL).size() - + " destinations, " - + discarded - + " replication events discarded"); + "Configuration reloaded: {} destinations, {} replication events discarded", + currentConfig.getDestinations(FilterType.ALL).size(), + discarded); } } } catch (Exception e) { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java index f8e2d7b..b2dd382 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java @@ -121,7 +121,7 @@ public class Destination { builder.add(g.getUUID()); addRecursiveParents(g.getUUID(), builder, groupIncludeCache); } else { - repLog.warn(String.format("Group \"%s\" not recognized, removing from authGroup", name)); + repLog.warn("Group \"{}\" not recognized, removing from authGroup", name); } } remoteUser = replicationUserFactory.create(new ListGroupMembership(builder.build())); @@ -201,7 +201,7 @@ public class Destination { int cnt = 0; if (pool != null) { for (Runnable r : pool.getQueue()) { - repLog.warn(String.format("Cancelling replication event %s", r)); + repLog.warn("Cancelling replication event {}", r); } cnt = pool.shutdownNow().size(); pool.unregisterWorkQueue(); @@ -505,8 +505,7 @@ public class Destination { } else if (remoteNameStyle.equals("basenameOnly")) { name = FilenameUtils.getBaseName(name); } else if (!remoteNameStyle.equals("slash")) { - repLog.debug( - String.format("Unknown remoteNameStyle: %s, falling back to slash", remoteNameStyle)); + repLog.debug("Unknown remoteNameStyle: {}, falling back to slash", remoteNameStyle); } String replacedPath = ReplicationQueue.replaceName(uri.getPath(), name, isSingleProjectMatch()); diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java index 525c990..8f61bfa 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java @@ -220,10 +220,10 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { if (ALL_REFS.equals(ref)) { delta.clear(); pushAllRefs = true; - repLog.trace("Added all refs for replication to " + uri); + repLog.trace("Added all refs for replication to {}", uri); } else if (!pushAllRefs) { delta.add(ref); - repLog.trace("Added ref " + ref + " for replication to " + uri); + repLog.trace("Added ref {} for replication to {}", ref, uri); } } @@ -307,13 +307,13 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { if (!pool.requestRunway(this)) { if (!canceled) { repLog.info( - "Rescheduling replication to " + uri + " to avoid collision with an in-flight push."); + "Rescheduling replication to {} to avoid collision with an in-flight push.", uri); pool.reschedule(this, Destination.RetryReason.COLLISION); } return; } - repLog.info("Replication to " + uri + " started..."); + repLog.info("Replication to {} started...", uri); Timer1.Context context = metrics.start(config.getName()); try { long startedAt = context.getStartTime(); @@ -323,15 +323,11 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { runImpl(); long elapsed = NANOSECONDS.toMillis(context.stop()); repLog.info( - "Replication to " - + uri - + " completed in " - + (elapsed) - + "ms, " - + (delay) - + "ms delay, " - + retryCount - + " retries"); + "Replication to {} completed in {}ms, {}ms delay, {} retries", + uri, + elapsed, + delay, + retryCount); } catch (RepositoryNotFoundException e) { stateLog.error( "Cannot replicate " + projectName + "; Local repository error: " + e.getMessage(), @@ -345,7 +341,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { if (msg.contains("access denied") || msg.contains("no such repository")) { createRepository(); } else { - repLog.error("Cannot replicate " + projectName + "; Remote repository error: " + msg); + repLog.error("Cannot replicate {}; Remote repository error: {}", projectName, msg); } } catch (NoRemoteRepositoryException e) { @@ -355,12 +351,12 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { } catch (TransportException e) { Throwable cause = e.getCause(); if (cause instanceof JSchException && cause.getMessage().startsWith("UnknownHostKey:")) { - repLog.error("Cannot replicate to " + uri + ": " + cause.getMessage()); + repLog.error("Cannot replicate to {}: {}", uri, cause.getMessage()); } else if (e instanceof LockFailureException) { lockRetryCount++; // The LockFailureException message contains both URI and reason // for this failure. - repLog.error("Cannot replicate to " + e.getMessage()); + repLog.error("Cannot replicate to {}: {}", uri, e.getMessage()); // The remote push operation should be retried. if (lockRetryCount <= maxLockRetries) { @@ -371,16 +367,16 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { } } else { repLog.error( - "Giving up after " - + lockRetryCount - + " of this error during replication to " - + e.getMessage()); + "Giving up after {} occurrences of this error: {} during replication to {}", + lockRetryCount, + e.getMessage(), + uri); } } else { if (canceledWhileRunning.get()) { logCanceledWhileRunningException(e); } else { - repLog.error("Cannot replicate to " + uri, e); + repLog.error("Cannot replicate to {}", uri, e); // The remote push operation should be retried. pool.reschedule(this, Destination.RetryReason.TRANSPORT_ERROR); } @@ -398,7 +394,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { } private void logCanceledWhileRunningException(TransportException e) { - repLog.info("Cannot replicate to " + uri + "." + " It was canceled while running", e); + repLog.info("Cannot replicate to {}. It was canceled while running", uri, e); } private void createRepository() { @@ -406,14 +402,14 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { try { Ref head = git.exactRef(Constants.HEAD); if (replicationQueue.createProject(projectName, head != null ? head.getName() : null)) { - repLog.warn("Missing repository created; retry replication to " + uri); + repLog.warn("Missing repository created; retry replication to {}", uri); pool.reschedule(this, Destination.RetryReason.REPOSITORY_MISSING); } else { repLog.warn( - "Missing repository could not be created when replicating " - + uri - + ". You can only create missing repositories locally, over SSH or when " - + "using adminUrl in replication.config. See documentation for more information."); + "Missing repository could not be created when replicating {}. " + + "You can only create missing repositories locally, over SSH or when " + + "using adminUrl in replication.config. See documentation for more information.", + uri); } } catch (IOException ioe) { stateLog.error( @@ -448,7 +444,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { return new PushResult(); } - repLog.info("Push to " + uri + " references: " + todo); + repLog.info("Push to {} references: {}", uri, todo); return tn.push(NullProgressMonitor.INSTANCE, todo); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java index 82e68ed..bec4f20 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java @@ -82,11 +82,11 @@ public class ReplicationFileBasedConfig implements ReplicationConfig { private List allDestinations(DestinationFactory destinationFactory) throws ConfigInvalidException, IOException { if (!config.getFile().exists()) { - log.warn("Config file " + config.getFile() + " does not exist; not replicating"); + log.warn("Config file {} does not exist; not replicating", config.getFile()); return Collections.emptyList(); } if (config.getFile().length() == 0) { - log.info("Config file " + config.getFile() + " is empty; not replicating"); + log.info("Config file {} is empty; not replicating", config.getFile()); return Collections.emptyList(); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java index 1370bd3..ebf9278 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java @@ -105,7 +105,7 @@ public class ReplicationQueue running = false; int discarded = config.shutdown(); if (discarded > 0) { - repLog.warn(String.format("Canceled %d replication events during shutdown", discarded)); + repLog.warn("Canceled {} replication events during shutdown", discarded); } } @@ -193,19 +193,19 @@ public class ReplicationQueue try { uri = new URIish(url); } catch (URISyntaxException e) { - repLog.warn(String.format("adminURL '%s' is invalid: %s", url, e.getMessage())); + repLog.warn("adminURL '{}' is invalid: {}", url, e.getMessage()); continue; } String path = replaceName(uri.getPath(), projectName.get(), config.isSingleProjectMatch()); if (path == null) { - repLog.warn(String.format("adminURL %s does not contain ${name}", uri)); + repLog.warn("adminURL {} does not contain ${name}", uri); continue; } uri = uri.setPath(path); if (!isSSH(uri)) { - repLog.warn(String.format("adminURL '%s' is invalid: only SSH is supported", uri)); + repLog.warn("adminURL '{}' is invalid: only SSH is supported", uri); continue; } @@ -237,11 +237,10 @@ public class ReplicationQueue createRemoteSsh(replicateURI, head); } else { repLog.warn( - String.format( - "Cannot create new project on remote site %s." - + " Only local paths and SSH URLs are supported" - + " for remote repository creation", - replicateURI)); + "Cannot create new project on remote site {}." + + " Only local paths and SSH URLs are supported" + + " for remote repository creation", + replicateURI); return false; } return true; @@ -258,7 +257,7 @@ public class ReplicationQueue } repLog.info("Created local repository: {}", uri); } catch (IOException e) { - repLog.error(String.format("Error creating local repository %s:\n", uri.getPath()), e); + repLog.error("Error creating local repository {}:\n", uri.getPath(), e); } } @@ -274,12 +273,14 @@ public class ReplicationQueue repLog.info("Created remote repository: {}", uri); } catch (IOException e) { repLog.error( - String.format( - "Error creating remote repository at %s:\n" - + " Exception: %s\n" - + " Command: %s\n" - + " Output: %s", - uri, e, cmd, errStream), + "Error creating remote repository at {}:\n" + + " Exception: {}\n" + + " Command: {}\n" + + " Output: {}", + uri, + e, + cmd, + errStream, e); } } @@ -287,17 +288,16 @@ public class ReplicationQueue private void deleteProject(URIish replicateURI) { if (!replicateURI.isRemote()) { deleteLocally(replicateURI); - repLog.info("Deleted local repository: " + replicateURI); + repLog.info("Deleted local repository: {}", replicateURI); } else if (isSSH(replicateURI)) { deleteRemoteSsh(replicateURI); - repLog.info("Deleted remote repository: " + replicateURI); + repLog.info("Deleted remote repository: {}", replicateURI); } else { repLog.warn( - String.format( - "Cannot delete project on remote site %s." - + " Only local paths and SSH URLs are supported" - + " for remote repository deletion", - replicateURI)); + "Cannot delete project on remote site {}. " + + "Only local paths and SSH URLs are supported" + + " for remote repository deletion", + replicateURI); } } @@ -305,7 +305,7 @@ public class ReplicationQueue try { recursivelyDelete(new File(uri.getPath())); } catch (IOException e) { - repLog.error(String.format("Error deleting local repository %s:\n", uri.getPath()), e); + repLog.error("Error deleting local repository {}:\n", uri.getPath(), e); } } @@ -335,12 +335,14 @@ public class ReplicationQueue executeRemoteSsh(uri, cmd, errStream); } catch (IOException e) { repLog.error( - String.format( - "Error deleting remote repository at %s:\n" - + " Exception: %s\n" - + " Command: %s\n" - + " Output: %s", - uri, e, cmd, errStream), + "Error deleting remote repository at {}:\n" + + " Exception: {}\n" + + " Command: {}\n" + + " Output: {}", + uri, + e, + cmd, + errStream, e); } } @@ -352,11 +354,10 @@ public class ReplicationQueue updateHeadRemoteSsh(replicateURI, newHead); } else { repLog.warn( - String.format( - "Cannot update HEAD of project on remote site %s." - + " Only local paths and SSH URLs are supported" - + " for remote HEAD update.", - replicateURI)); + "Cannot update HEAD of project on remote site {}." + + " Only local paths and SSH URLs are supported" + + " for remote HEAD update.", + replicateURI); } } @@ -369,12 +370,15 @@ public class ReplicationQueue executeRemoteSsh(uri, cmd, errStream); } catch (IOException e) { repLog.error( - String.format( - "Error updating HEAD of remote repository at %s to %s:\n" - + " Exception: %s\n" - + " Command: %s\n" - + " Output: %s", - uri, newHead, e, cmd, errStream), + "Error updating HEAD of remote repository at {} to {}:\n" + + " Exception: {}\n" + + " Command: {}\n" + + " Output: {}", + uri, + newHead, + e, + cmd, + errStream, e); } } @@ -386,8 +390,7 @@ public class ReplicationQueue u.link(newHead); } } catch (IOException e) { - repLog.error( - String.format("Failed to update HEAD of repository %s to %s", uri.getPath(), newHead), e); + repLog.error("Failed to update HEAD of repository {} to {}", uri.getPath(), newHead, e); } } -- cgit v1.2.3 From 454bfe4289d30636fc13dc22a89f16ef57dd0326 Mon Sep 17 00:00:00 2001 From: Hector Oswaldo Caballero Date: Wed, 21 Feb 2018 21:42:06 -0500 Subject: Fix project deletion logs Log was showing project was deleted successfully even if it failed. Change-Id: I7be19991619ce1f205884a8b6658d9a7ed4109c7 --- .../com/googlesource/gerrit/plugins/replication/ReplicationQueue.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java index ebf9278..0f367aa 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java @@ -288,10 +288,8 @@ public class ReplicationQueue private void deleteProject(URIish replicateURI) { if (!replicateURI.isRemote()) { deleteLocally(replicateURI); - repLog.info("Deleted local repository: {}", replicateURI); } else if (isSSH(replicateURI)) { deleteRemoteSsh(replicateURI); - repLog.info("Deleted remote repository: {}", replicateURI); } else { repLog.warn( "Cannot delete project on remote site {}. " @@ -304,6 +302,7 @@ public class ReplicationQueue private static void deleteLocally(URIish uri) { try { recursivelyDelete(new File(uri.getPath())); + repLog.info("Deleted local repository: {}", uri); } catch (IOException e) { repLog.error("Error deleting local repository {}:\n", uri.getPath(), e); } @@ -333,6 +332,7 @@ public class ReplicationQueue OutputStream errStream = newErrorBufferStream(); try { executeRemoteSsh(uri, cmd, errStream); + repLog.info("Deleted remote repository: {}", uri); } catch (IOException e) { repLog.error( "Error deleting remote repository at {}:\n" -- cgit v1.2.3 From 19c302973e255e2f5e733acf72591fcbc3d805b6 Mon Sep 17 00:00:00 2001 From: Hector Oswaldo Caballero Date: Thu, 22 Feb 2018 13:45:50 -0500 Subject: Fix creating missing repository When replicating to a destination where the repository does not exist, updating the HEAD reference failed because the passed reference name was not absolute. When this happened, an unchecked exception was triggered but never showed in the logs. In fact, the replication logs were showing only the message announcing the replication started, but no additional information about the success or failure of the operation was displayed. Avoid this by resolving symbolic references and checking the reference is absolute before trying to update the HEAD in the new repository. Change-Id: I18295a37a04413ba64bf7e9ee31640ee23c4c1e0 --- .../com/googlesource/gerrit/plugins/replication/PushOne.java | 10 +++++++++- .../gerrit/plugins/replication/ReplicationQueue.java | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java index 8f61bfa..64f5152 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java @@ -401,7 +401,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { if (pool.isCreateMissingRepos()) { try { Ref head = git.exactRef(Constants.HEAD); - if (replicationQueue.createProject(projectName, head != null ? head.getName() : null)) { + if (replicationQueue.createProject(projectName, head != null ? getName(head) : null)) { repLog.warn("Missing repository created; retry replication to {}", uri); pool.reschedule(this, Destination.RetryReason.REPOSITORY_MISSING); } else { @@ -422,6 +422,14 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning { } } + private String getName(Ref ref) { + Ref target = ref; + while (target.isSymbolic()) { + target = target.getTarget(); + } + return target.getName(); + } + private void runImpl() throws IOException { PushResult res; try (Transport tn = Transport.open(git, uri)) { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java index 0f367aa..57893d8 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java @@ -250,7 +250,7 @@ public class ReplicationQueue try (Repository repo = new FileRepository(uri.getPath())) { repo.create(true /* bare */); - if (head != null) { + if (head != null && head.startsWith(Constants.R_REFS)) { RefUpdate u = repo.updateRef(Constants.HEAD); u.disableRefLog(); u.link(head); -- cgit v1.2.3