From ebc43c59ac25a3c75b9ce089f90e9eaac6c74637 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 5 Sep 2016 22:20:34 +0200 Subject: [PATCH] SONAR-7578 do not start notification daemon on CE --- .../java/it/issue/IssueNotificationsTest.java | 2 +- .../notification/NotificationDaemon.java | 137 ++++++++++++++++++ .../notification/NotificationService.java | 115 +-------------- .../platformlevel/PlatformLevel4.java | 2 + ...eTest.java => NotificationDaemonTest.java} | 64 ++++---- 5 files changed, 176 insertions(+), 144 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/notification/NotificationDaemon.java rename server/sonar-server/src/test/java/org/sonar/server/notification/{NotificationServiceTest.java => NotificationDaemonTest.java} (85%) diff --git a/it/it-tests/src/test/java/it/issue/IssueNotificationsTest.java b/it/it-tests/src/test/java/it/issue/IssueNotificationsTest.java index 9c5f6d2c91a..efdd3c28bf6 100644 --- a/it/it-tests/src/test/java/it/issue/IssueNotificationsTest.java +++ b/it/it-tests/src/test/java/it/issue/IssueNotificationsTest.java @@ -198,7 +198,7 @@ public class IssueNotificationsTest extends AbstractIssueTest { } private static void waitUntilAllNotificationsAreDelivered() throws InterruptedException { - Thread.sleep(10000); + Thread.sleep(10_000L); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationDaemon.java b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationDaemon.java new file mode 100644 index 00000000000..3ac96fc3cb8 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationDaemon.java @@ -0,0 +1,137 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.notification; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import org.picocontainer.Startable; +import org.sonar.api.Properties; +import org.sonar.api.Property; +import org.sonar.api.config.Settings; +import org.sonar.api.notifications.Notification; +import org.sonar.api.server.ServerSide; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; + +@Properties({ + @Property( + key = NotificationDaemon.PROPERTY_DELAY, + defaultValue = "60", + name = "Delay of notifications, in seconds", + project = false, + global = false), + @Property( + key = NotificationDaemon.PROPERTY_DELAY_BEFORE_REPORTING_STATUS, + defaultValue = "600", + name = "Delay before reporting notification status, in seconds", + project = false, + global = false) +}) +@ServerSide +public class NotificationDaemon implements Startable { + private static final String THREAD_NAME_PREFIX = "sq-notification-service-"; + + private static final Logger LOG = Loggers.get(NotificationDaemon.class); + + public static final String PROPERTY_DELAY = "sonar.notifications.delay"; + public static final String PROPERTY_DELAY_BEFORE_REPORTING_STATUS = "sonar.notifications.runningDelayBeforeReportingStatus"; + + private final long delayInSeconds; + private final long delayBeforeReportingStatusInSeconds; + private final DefaultNotificationManager manager; + private final NotificationService service; + + private ScheduledExecutorService executorService; + private boolean stopping = false; + + public NotificationDaemon(Settings settings, DefaultNotificationManager manager, NotificationService service) { + this.delayInSeconds = settings.getLong(PROPERTY_DELAY); + this.delayBeforeReportingStatusInSeconds = settings.getLong(PROPERTY_DELAY_BEFORE_REPORTING_STATUS); + this.manager = manager; + this.service = service; + } + + @Override + public void start() { + executorService = Executors.newSingleThreadScheduledExecutor( + new ThreadFactoryBuilder() + .setNameFormat(THREAD_NAME_PREFIX + "%d") + .setPriority(Thread.MIN_PRIORITY) + .build()); + executorService.scheduleWithFixedDelay(() -> { + try { + processQueue(); + } catch (Exception e) { + LOG.error("Error in NotificationService", e); + } + }, 0, delayInSeconds, TimeUnit.SECONDS); + LOG.info("Notification service started (delay {} sec.)", delayInSeconds); + } + + @Override + public void stop() { + try { + stopping = true; + executorService.shutdown(); + executorService.awaitTermination(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + LOG.error("Error during stop of notification service", e); + } + LOG.info("Notification service stopped"); + } + + @VisibleForTesting + synchronized void processQueue() { + long start = now(); + long lastLog = start; + long notifSentCount = 0; + + Notification notifToSend = manager.getFromQueue(); + while (notifToSend != null) { + service.deliver(notifToSend); + notifSentCount++; + if (stopping) { + break; + } + long now = now(); + if (now - lastLog > delayBeforeReportingStatusInSeconds * 1000) { + long remainingNotifCount = manager.count(); + lastLog = now; + long spentTimeInMinutes = (now - start) / (60 * 1000); + log(notifSentCount, remainingNotifCount, spentTimeInMinutes); + } + notifToSend = manager.getFromQueue(); + } + } + + @VisibleForTesting + void log(long notifSentCount, long remainingNotifCount, long spentTimeInMinutes) { + LOG.info("{} notifications sent during the past {} minutes and {} still waiting to be sent", + notifSentCount, spentTimeInMinutes, remainingNotifCount); + } + + @VisibleForTesting + long now() { + return System.currentTimeMillis(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationService.java b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationService.java index 6e5aa86b926..c86451b343a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationService.java @@ -24,68 +24,30 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; import com.google.common.collect.SetMultimap; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; -import org.picocontainer.Startable; -import org.sonar.api.Properties; -import org.sonar.api.Property; -import org.sonar.api.config.Settings; +import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationChannel; -import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.db.DbClient; -@Properties({ - @Property( - key = NotificationService.PROPERTY_DELAY, - defaultValue = "60", - name = "Delay of notifications, in seconds", - project = false, - global = false), - @Property( - key = NotificationService.PROPERTY_DELAY_BEFORE_REPORTING_STATUS, - defaultValue = "600", - name = "Delay before reporting notification status, in seconds", - project = false, - global = false) -}) @ServerSide @ComputeEngineSide -public class NotificationService implements Startable { - private static final String THREAD_NAME_PREFIX = "sq-notification-service-"; +public class NotificationService { private static final Logger LOG = Loggers.get(NotificationService.class); - public static final String PROPERTY_DELAY = "sonar.notifications.delay"; - public static final String PROPERTY_DELAY_BEFORE_REPORTING_STATUS = "sonar.notifications.runningDelayBeforeReportingStatus"; - - private final long delayInSeconds; - private final long delayBeforeReportingStatusInSeconds; - private final DefaultNotificationManager manager; private final List dispatchers; private final DbClient dbClient; - private ScheduledExecutorService executorService; - private boolean stopping = false; - private final boolean disabled; - - public NotificationService(Settings settings, DefaultNotificationManager manager, DbClient dbClient, - NotificationDispatcher[] dispatchers) { - this.disabled = "ComputeEngineSettings".equals(settings.getClass().getSimpleName()); - this.delayInSeconds = settings.getLong(PROPERTY_DELAY); - this.delayBeforeReportingStatusInSeconds = settings.getLong(PROPERTY_DELAY_BEFORE_REPORTING_STATUS); - this.manager = manager; + public NotificationService(DbClient dbClient, NotificationDispatcher[] dispatchers) { this.dbClient = dbClient; this.dispatchers = ImmutableList.copyOf(dispatchers); } @@ -93,75 +55,8 @@ public class NotificationService implements Startable { /** * Default constructor when no dispatchers. */ - public NotificationService(Settings settings, DefaultNotificationManager manager, DbClient dbClient) { - this(settings, manager, dbClient, new NotificationDispatcher[0]); - } - - @Override - public void start() { - if (!disabled) { - executorService = - Executors.newSingleThreadScheduledExecutor( - new ThreadFactoryBuilder() - .setNameFormat(THREAD_NAME_PREFIX + "%d") - .setPriority(Thread.MIN_PRIORITY) - .build()); - executorService.scheduleWithFixedDelay(new Runnable() { - @Override - public void run() { - try { - processQueue(); - } catch (Exception e) { - LOG.error("Error in NotificationService", e); - } - } - }, 0, delayInSeconds, TimeUnit.SECONDS); - LOG.info("Notification service started (delay {} sec.)", delayInSeconds); - } - } - - @Override - public void stop() { - if (!disabled) { - try { - stopping = true; - executorService.shutdown(); - executorService.awaitTermination(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - LOG.error("Error during stop of notification service", e); - } - LOG.info("Notification service stopped"); - } - } - - @VisibleForTesting - synchronized void processQueue() { - long start = now(); - long lastLog = start; - long notifSentCount = 0; - - Notification notifToSend = manager.getFromQueue(); - while (notifToSend != null) { - deliver(notifToSend); - notifSentCount++; - if (stopping) { - break; - } - long now = now(); - if (now - lastLog > delayBeforeReportingStatusInSeconds * 1000) { - long remainingNotifCount = manager.count(); - lastLog = now; - long spentTimeInMinutes = (now - start) / (60 * 1000); - log(notifSentCount, remainingNotifCount, spentTimeInMinutes); - } - notifToSend = manager.getFromQueue(); - } - } - - @VisibleForTesting - void log(long notifSentCount, long remainingNotifCount, long spentTimeInMinutes) { - LOG.info("{} notifications sent during the past {} minutes and {} still waiting to be sent", - new Object[] {notifSentCount, spentTimeInMinutes, remainingNotifCount}); + public NotificationService(DbClient dbClient) { + this(dbClient, new NotificationDispatcher[0]); } @VisibleForTesting diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java index 6b465505031..92c84a23d5c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java @@ -135,6 +135,7 @@ import org.sonar.server.metric.DefaultMetricFinder; import org.sonar.server.metric.ws.MetricsWsModule; import org.sonar.server.notification.DefaultNotificationManager; import org.sonar.server.notification.NotificationCenter; +import org.sonar.server.notification.NotificationDaemon; import org.sonar.server.notification.NotificationService; import org.sonar.server.notification.email.AlertsEmailTemplate; import org.sonar.server.notification.email.EmailNotificationChannel; @@ -578,6 +579,7 @@ public class PlatformLevel4 extends PlatformLevel { NotificationCenter.class, DefaultNotificationManager.class, EmailsWsModule.class, + NotificationDaemon.class, // Tests TestsWs.class, diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationDaemonTest.java similarity index 85% rename from server/sonar-server/src/test/java/org/sonar/server/notification/NotificationServiceTest.java rename to server/sonar-server/src/test/java/org/sonar/server/notification/NotificationDaemonTest.java index 2df7b3c2915..9244a69b52b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationDaemonTest.java @@ -43,21 +43,21 @@ import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -public class NotificationServiceTest { +public class NotificationDaemonTest { private static String CREATOR_SIMON = "simon"; private static String CREATOR_EVGENY = "evgeny"; private static String ASSIGNEE_SIMON = "simon"; - DefaultNotificationManager manager = mock(DefaultNotificationManager.class); - Notification notification = mock(Notification.class); - NotificationChannel emailChannel = mock(NotificationChannel.class); - NotificationChannel gtalkChannel = mock(NotificationChannel.class); - NotificationDispatcher commentOnIssueAssignedToMe = mock(NotificationDispatcher.class); - NotificationDispatcher commentOnIssueCreatedByMe = mock(NotificationDispatcher.class); - NotificationDispatcher qualityGateChange = mock(NotificationDispatcher.class); - DbClient dbClient = mock(DbClient.class); - - private NotificationService service; + private DefaultNotificationManager manager = mock(DefaultNotificationManager.class); + private Notification notification = mock(Notification.class); + private NotificationChannel emailChannel = mock(NotificationChannel.class); + private NotificationChannel gtalkChannel = mock(NotificationChannel.class); + private NotificationDispatcher commentOnIssueAssignedToMe = mock(NotificationDispatcher.class); + private NotificationDispatcher commentOnIssueCreatedByMe = mock(NotificationDispatcher.class); + private NotificationDispatcher qualityGateChange = mock(NotificationDispatcher.class); + private DbClient dbClient = mock(DbClient.class); + private NotificationService service = new NotificationService(dbClient, new NotificationDispatcher[]{commentOnIssueAssignedToMe, commentOnIssueCreatedByMe, qualityGateChange}); + private NotificationDaemon underTest = null; private void setUpMocks() { when(emailChannel.getKey()).thenReturn("email"); @@ -72,9 +72,7 @@ public class NotificationServiceTest { Settings settings = new MapSettings().setProperty("sonar.notifications.delay", 1L); - service = new NotificationService(settings, manager, - dbClient, - new NotificationDispatcher[]{commentOnIssueAssignedToMe, commentOnIssueCreatedByMe, qualityGateChange}); + underTest = new NotificationDaemon(settings, manager, service); } /** @@ -93,9 +91,9 @@ public class NotificationServiceTest { doAnswer(addUser(ASSIGNEE_SIMON, emailChannel)).when(commentOnIssueAssignedToMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); doAnswer(addUser(CREATOR_SIMON, emailChannel)).when(commentOnIssueCreatedByMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); - service.start(); + underTest.start(); verify(emailChannel, timeout(2000)).deliver(notification, ASSIGNEE_SIMON); - service.stop(); + underTest.stop(); verify(gtalkChannel, never()).deliver(notification, ASSIGNEE_SIMON); } @@ -117,10 +115,10 @@ public class NotificationServiceTest { doAnswer(addUser(ASSIGNEE_SIMON, emailChannel)).when(commentOnIssueAssignedToMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); doAnswer(addUser(CREATOR_EVGENY, gtalkChannel)).when(commentOnIssueCreatedByMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); - service.start(); + underTest.start(); verify(emailChannel, timeout(2000)).deliver(notification, ASSIGNEE_SIMON); verify(gtalkChannel, timeout(2000)).deliver(notification, CREATOR_EVGENY); - service.stop(); + underTest.stop(); verify(emailChannel, never()).deliver(notification, CREATOR_EVGENY); verify(gtalkChannel, never()).deliver(notification, ASSIGNEE_SIMON); @@ -142,10 +140,10 @@ public class NotificationServiceTest { doAnswer(addUser(ASSIGNEE_SIMON, new NotificationChannel[]{emailChannel, gtalkChannel})) .when(commentOnIssueAssignedToMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); - service.start(); + underTest.start(); verify(emailChannel, timeout(2000)).deliver(notification, ASSIGNEE_SIMON); verify(gtalkChannel, timeout(2000)).deliver(notification, ASSIGNEE_SIMON); - service.stop(); + underTest.stop(); verify(emailChannel, never()).deliver(notification, CREATOR_EVGENY); verify(gtalkChannel, never()).deliver(notification, CREATOR_EVGENY); @@ -165,8 +163,8 @@ public class NotificationServiceTest { public void scenario4() { setUpMocks(); - service.start(); - service.stop(); + underTest.start(); + underTest.stop(); verify(emailChannel, never()).deliver(any(Notification.class), anyString()); verify(gtalkChannel, never()).deliver(any(Notification.class), anyString()); @@ -180,9 +178,9 @@ public class NotificationServiceTest { doAnswer(addUser(ASSIGNEE_SIMON, emailChannel)).when(commentOnIssueAssignedToMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); doAnswer(addUser(CREATOR_SIMON, emailChannel)).when(commentOnIssueCreatedByMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); - service.start(); + underTest.start(); verify(emailChannel, timeout(2000)).deliver(notification, ASSIGNEE_SIMON); - service.stop(); + underTest.stop(); verify(gtalkChannel, never()).deliver(notification, ASSIGNEE_SIMON); } @@ -192,8 +190,8 @@ public class NotificationServiceTest { setUpMocks(); doAnswer(addUser(null, gtalkChannel)).when(commentOnIssueCreatedByMe).dispatch(same(notification), any(NotificationDispatcher.Context.class)); - service.start(); - service.stop(); + underTest.start(); + underTest.stop(); verify(emailChannel, never()).deliver(any(Notification.class), anyString()); verify(gtalkChannel, never()).deliver(any(Notification.class), anyString()); @@ -210,7 +208,7 @@ public class NotificationServiceTest { public void getDispatchers_empty() { Settings settings = new MapSettings().setProperty("sonar.notifications.delay", 1L); - service = new NotificationService(settings, manager, dbClient); + service = new NotificationService(dbClient); assertThat(service.getDispatchers()).hasSize(0); } @@ -220,13 +218,13 @@ public class NotificationServiceTest { // Emulate 2 notifications in DB when(manager.getFromQueue()).thenReturn(notification).thenReturn(notification).thenReturn(null); when(manager.count()).thenReturn(1L).thenReturn(0L); - service = spy(service); + underTest = spy(underTest); // Emulate processing of each notification take 10 min to have a log each time - when(service.now()).thenReturn(0L).thenReturn(10 * 60 * 1000 + 1L).thenReturn(20 * 60 * 1000 + 2L); - service.start(); - verify(service, timeout(200)).log(1, 1, 10); - verify(service, timeout(200)).log(2, 0, 20); - service.stop(); + when(underTest.now()).thenReturn(0L).thenReturn(10 * 60 * 1000 + 1L).thenReturn(20 * 60 * 1000 + 2L); + underTest.start(); + verify(underTest, timeout(200)).log(1, 1, 10); + verify(underTest, timeout(200)).log(2, 0, 20); + underTest.stop(); } @Test -- 2.39.5