You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by dp...@apache.org on 2019/07/01 17:23:13 UTC
[ignite-teamcity-bot] branch master updated: IGNITE-11947: Fix for
handling failures of email/slack sending, retries added
This is an automated email from the ASF dual-hosted git repository.
dpavlov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite-teamcity-bot.git
The following commit(s) were added to refs/heads/master by this push:
new 8f69c4a IGNITE-11947: Fix for handling failures of email/slack sending, retries added
8f69c4a is described below
commit 8f69c4a4d3c6c84f217a4c87e03af28c97c5c2bd
Author: Dmitriy Pavlov <dp...@apache.org>
AuthorDate: Mon Jul 1 20:06:57 2019 +0300
IGNITE-11947: Fix for handling failures of email/slack sending, retries added
---
.../java/org/apache/ignite/ci/issue/Issue.java | 40 ++++++++++-
.../org/apache/ignite/ci/issue/IssuesStorage.java | 38 ++++++++--
.../org/apache/ignite/ci/mail/EmailSender.java | 56 +++++++--------
.../org/apache/ignite/ci/mail/SlackSender.java | 19 ++---
.../apache/ignite/ci/runners/ClientTmpHelper.java | 15 ++--
.../ignite/ci/tcbot/issue/IIssuesStorage.java | 4 +-
.../ignite/ci/tcbot/issue/IssueDetector.java | 83 +++++++++++++---------
.../apache/ignite/ci/tcbot/issue/Notification.java | 4 ++
.../src/main/webapp/js/issues-1.0.js | 13 +++-
9 files changed, 178 insertions(+), 94 deletions(-)
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/Issue.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/Issue.java
index e5c414a..2e99c52 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/Issue.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/Issue.java
@@ -23,12 +23,14 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
+import java.util.Map;
import java.util.TreeSet;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.apache.ignite.tcbot.persistence.Persisted;
import org.apache.ignite.tcbot.common.util.TimeUtil;
-import org.jetbrains.annotations.Nullable;
/**
* Issue used both for saving into DB and in UI (in issue history).
@@ -67,7 +69,13 @@ public class Issue {
/** Set of build tags detected. */
public Set<String> buildTags = new TreeSet<>();
- public java.util.Map<String, Integer> stat = new HashMap<>();
+ /** Statistics of subscribers for this issue. Filled accordignly recent update. */
+ public Map<String, Object> stat = new HashMap<>();
+
+ /** Notification failed: Map from address to exception text */
+ @Nullable public Map<String, String> notificationFailed = new HashMap<>();
+
+ public int notificationRetry = 0;
public Issue(IssueKey issueKey, IssueType type,
@Nullable Long buildStartTs) {
@@ -235,4 +243,32 @@ public class Issue {
public String type() {
return type;
}
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+ Issue issue = (Issue)o;
+ return notificationRetry == issue.notificationRetry &&
+ Objects.equals(type, issue.type) &&
+ Objects.equals(displayType, issue.displayType) &&
+ Objects.equals(trackedBranchName, issue.trackedBranchName) &&
+ Objects.equals(issueKey, issue.issueKey) &&
+ Objects.equals(changes, issue.changes) &&
+ Objects.equals(addressNotified, issue.addressNotified) &&
+ Objects.equals(webUrl, issue.webUrl) &&
+ Objects.equals(displayName, issue.displayName) &&
+ Objects.equals(buildStartTs, issue.buildStartTs) &&
+ Objects.equals(detectedTs, issue.detectedTs) &&
+ Objects.equals(buildTags, issue.buildTags) &&
+ Objects.equals(stat, issue.stat) &&
+ Objects.equals(notificationFailed, issue.notificationFailed);
+ }
+
+ /** {@inheritDoc} */
+ @Override public int hashCode() {
+ return Objects.hash(type, displayType, trackedBranchName, issueKey, changes, addressNotified, webUrl, displayName, buildStartTs, detectedTs, buildTags, stat, notificationFailed, notificationRetry);
+ }
}
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/IssuesStorage.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/IssuesStorage.java
index e8cd7e9..a18294d 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/IssuesStorage.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/issue/IssuesStorage.java
@@ -20,6 +20,7 @@ package org.apache.ignite.ci.issue;
import java.util.HashMap;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
+import javax.annotation.Nullable;
import javax.cache.Cache;
import javax.inject.Inject;
import javax.inject.Provider;
@@ -53,17 +54,40 @@ public class IssuesStorage implements IIssuesStorage {
}
/** {@inheritDoc} */
- @Override public boolean setNotified(IssueKey issueKey, String to) {
+ @Override public boolean getIsNewAndSetNotified(IssueKey issueKey, String to,
+ @Nullable Exception e) {
Issue issue = cache().get(issueKey);
if (issue == null)
return false;
- boolean add = issue.addressNotified.add(to);
+ boolean update;
+ if (e == null) {
+ if (issue.notificationRetry >= 2 && issue.notificationFailed.containsKey(to))
+ return false; // no more tries;
- if (add)
+ update = issue.addressNotified.add(to);
+
+ if (update && issue.notificationFailed != null)
+ issue.notificationFailed.remove(to);
+ }
+ else {
+ if (issue.notificationRetry < 2)
+ issue.addressNotified.remove(to);
+
+ issue.notificationRetry++;
+
+ if (issue.notificationFailed == null)
+ issue.notificationFailed = new HashMap<>();
+
+ issue.notificationFailed.put(to, e.getClass().getSimpleName() + ": " + e.getMessage());
+
+ update = true;
+ }
+
+ if (update)
cache().put(issueKey, issue);
- return add;
+ return update;
}
@Override
@@ -73,6 +97,8 @@ public class IssuesStorage implements IIssuesStorage {
if (issue == null)
return;
+ int hashCode = issue.hashCode();
+
if (issue.stat == null)
issue.stat = new HashMap<>();
@@ -80,8 +106,8 @@ public class IssuesStorage implements IIssuesStorage {
issue.stat.put("cntSubscribed", cntSubscribed);
issue.stat.put("cntTagsFilterPassed", cntTagsFilterPassed);
- cache().put(issueKey, issue);
-
+ if (issue.hashCode() != hashCode)
+ cache().put(issueKey, issue); // protect from odd writes
}
/** {@inheritDoc} */
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/EmailSender.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/EmailSender.java
index 3f8f075..895d596 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/EmailSender.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/EmailSender.java
@@ -35,8 +35,8 @@ import org.apache.ignite.tcbot.engine.conf.NotificationsConfig;
* Class for sending email with configured credentials.
*/
public class EmailSender {
- public static boolean sendEmail(String to, String subject, String html, String plainText,
- NotificationsConfig notifications) {
+ public static void sendEmail(String to, String subject, String html, String plainText,
+ NotificationsConfig notifications) throws MessagingException {
String user = notifications.emailUsernameMandatory();
@@ -58,42 +58,34 @@ public class EmailSender {
}
});
- try {
- // Create a default MimeMessage object.
- MimeMessage msg = new MimeMessage(ses);
+ // Create a default MimeMessage object.
+ MimeMessage msg = new MimeMessage(ses);
- // Set From: header field of the header.
- msg.setFrom(new InternetAddress(from));
+ // Set From: header field of the header.
+ msg.setFrom(new InternetAddress(from));
- // Set To: header field of the header.
- msg.addRecipient(Message.RecipientType.TO, new InternetAddress(to));
+ // Set To: header field of the header.
+ msg.addRecipient(Message.RecipientType.TO, new InternetAddress(to));
- // Set Subject: header field
- msg.setSubject(subject);
+ // Set Subject: header field
+ msg.setSubject(subject);
- final MimeBodyPart textPart = new MimeBodyPart();
- textPart.setContent(plainText, "text/plain");
- // HTML version
- final MimeBodyPart htmlPart = new MimeBodyPart();
- htmlPart.setContent(html, "text/html");
+ final MimeBodyPart textPart = new MimeBodyPart();
+ textPart.setContent(plainText, "text/plain");
+ // HTML version
+ final MimeBodyPart htmlPart = new MimeBodyPart();
+ htmlPart.setContent(html, "text/html");
- // Create the Multipart. Add BodyParts to it.
- final Multipart mp = new MimeMultipart("alternative");
- mp.addBodyPart(textPart);
- mp.addBodyPart(htmlPart);
- // Set Multipart as the message's content
- msg.setContent(mp);
+ // Create the Multipart. Add BodyParts to it.
+ final Multipart mp = new MimeMultipart("alternative");
+ mp.addBodyPart(textPart);
+ mp.addBodyPart(htmlPart);
+ // Set Multipart as the message's content
+ msg.setContent(mp);
- // Send message
- Transport.send(msg);
+ // Send message
+ Transport.send(msg);
- System.out.println("Sent message successfully to [" + to + "]...");
-
- return true;
- }
- catch (MessagingException mex) {
- mex.printStackTrace();
- }
- return false;
+ System.out.println("Sent message successfully to [" + to + "]...");
}
}
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/SlackSender.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/SlackSender.java
index d15d337..fa8e43b 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/SlackSender.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/mail/SlackSender.java
@@ -34,7 +34,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
*
*/
public class SlackSender {
- public static boolean sendMessage(String addr, String msg,
+ public static void sendMessage(String addr, String msg,
NotificationsConfig cfg) throws IOException {
String authTok = cfg.slackAuthToken();
Preconditions.checkState(!isNullOrEmpty(authTok), "notifications:\"{}\" property should be filled in branches.json");
@@ -49,11 +49,8 @@ public class SlackSender {
SlackChannel slackCh = ses.findChannelByName(ch);
- if (slackCh == null) {
- System.err.println("Failed to find channel [" + addr + "]: Notification not send [" + msg + "]");
-
- return false;
- }
+ if (slackCh == null)
+ throw new RuntimeException("Failed to find channel [" + addr + "]: Notification not send [" + msg + "]");
SlackMessageHandle<SlackMessageReply> handle = ses.sendMessage(slackCh, msg);
@@ -62,22 +59,16 @@ public class SlackSender {
else {
SlackUser user = ses.findUserByUserName(addr); //make sure bot is a member of the user.
- if (user == null) {
- System.err.println("Failed to find user [" + addr + "]: Notification not send [" + msg + "]");
-
- return false;
- }
+ if (user == null)
+ throw new RuntimeException("Failed to find user [" + addr + "]: Notification not send [" + msg + "]");
SlackMessageHandle<SlackMessageReply> handle = ses.sendMessageToUser(user, msg, null);
System.out.println("Message to user " + addr + " " + msg + "; acked: " + handle.isAcked());
-
}
}
finally {
ses.disconnect();
}
-
- return true;
}
}
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/runners/ClientTmpHelper.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/runners/ClientTmpHelper.java
index 58a20b1..75774d2 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/runners/ClientTmpHelper.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/runners/ClientTmpHelper.java
@@ -29,6 +29,11 @@ import org.apache.ignite.jiraignited.JiraTicketDao;
* Utility class for local connection to TC helper DB (server) and any manipulations with data needed.
*/
public class ClientTmpHelper {
+ public static void main(String[] args) {
+ //select some main option here.
+ mainDropIssHist(args);
+ }
+
/**
* @param args Args.
*/
@@ -47,7 +52,7 @@ public class ClientTmpHelper {
/**
* @param args Args.
*/
- public static void main0(String[] args) {
+ public static void mainDropIssHist(String[] args) {
Ignite ignite = TcHelperDb.startClient();
IgniteCache<Object, Object> cache = ignite.cache(IssuesStorage.BOT_DETECTED_ISSUES);
@@ -56,7 +61,10 @@ public class ClientTmpHelper {
issue -> {
Object key = issue.getKey();
Issue val = (Issue)issue.getValue();
- // val.addressNotified.clear();
+
+ if(val.issueKey.testOrBuildName.contains(
+ "GridCachePartitionEvictionDuringReadThroughSelfTest.testPartitionRent"))
+ val.addressNotified.clear();
cache.put(key, val);
}
@@ -74,9 +82,6 @@ public class ClientTmpHelper {
ignite.close();
}
- public static void main(String[] args) {
- mainDeleteTickets(args);
- }
public static void mainDeleteTickets(String[] args) {
Ignite ignite = TcHelperDb.startClient();
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IIssuesStorage.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IIssuesStorage.java
index c1bdb08..af7a992 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IIssuesStorage.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IIssuesStorage.java
@@ -18,6 +18,7 @@
package org.apache.ignite.ci.tcbot.issue;
import java.util.stream.Stream;
+import javax.annotation.Nullable;
import org.apache.ignite.ci.issue.Issue;
import org.apache.ignite.ci.issue.IssueKey;
@@ -38,9 +39,10 @@ public interface IIssuesStorage {
* Checks and saves address was notified (NotThreadSafe)
* @param key issue key.
* @param addr Address to register as notified.
+ * @param e Exception. Null means notification was successfull. Non null means notification failed
* @return update successful. This address was not notified before.
*/
- public boolean setNotified(IssueKey key, String addr);
+ public boolean getIsNewAndSetNotified(IssueKey key, String addr, @Nullable Exception e);
public void saveIssueSubscribersStat(IssueKey key, int cntSrvAllowed, int cntSubscribed, int cntTagsFilterPassed);
}
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IssueDetector.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IssueDetector.java
index 8e6ba32..8fd1e24 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IssueDetector.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/IssueDetector.java
@@ -19,7 +19,6 @@ package org.apache.ignite.ci.tcbot.issue;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -136,7 +135,7 @@ public class IssueDetector {
@SuppressWarnings({"WeakerAccess", "UnusedReturnValue"})
@AutoProfiling
@MonitoredTask(name = "Send Notifications")
- protected String sendNewNotificationsEx() throws IOException {
+ protected String sendNewNotificationsEx() {
List<INotificationChannel> channels = new ArrayList<>();
userStorage.allUsers()
@@ -224,10 +223,11 @@ public class IssueDetector {
if(!addrs.isEmpty())
hasSubscriptions.incrementAndGet();
- boolean nonNotifedFound = false;
+ boolean nonNotifedChFound = false;
+
for (String nextAddr : addrs) {
- if (issuesStorage.setNotified(issue.issueKey, nextAddr)) {
- nonNotifedFound = true;
+ if (issuesStorage.getIsNewAndSetNotified(issue.issueKey, nextAddr, null)) {
+ nonNotifedChFound = true;
toBeSent.computeIfAbsent(nextAddr, addr -> {
Notification notification = new Notification();
@@ -237,51 +237,70 @@ public class IssueDetector {
}).addIssue(issue);
}
}
- if (!nonNotifedFound)
- issuesStorage.saveIssueSubscribersStat(issue.issueKey, ctnSrvAllowed.get(),
- cntSubscibed.get(), cntTagsFilterPassed.get());
- if (nonNotifedFound)
+ if (!nonNotifedChFound) {
+ issuesStorage.saveIssueSubscribersStat(issue.issueKey,
+ ctnSrvAllowed.get(),
+ cntSubscibed.get(),
+ cntTagsFilterPassed.get());
+ }
+ else
neverSentBefore.incrementAndGet();
});
- String stat = issuesChecked.get() + " issues checked " +
- filteredFresh.get() + " detected recenty " +
- filteredBuildTs.get() + " for fresh builds " +
- filteredNotDisabled.get() + " not disabled " +
- hasSubscriptions.get() + " has subscriber " +
- neverSentBefore.get() + " non notified";
+ String stat = issuesChecked.get() + " issues checked, " +
+ filteredFresh.get() + " detected recenty, " +
+ filteredBuildTs.get() + " for fresh builds, " +
+ filteredNotDisabled.get() + " not disabled, " +
+ hasSubscriptions.get() + " has subscriber, " +
+ neverSentBefore.get() + " non sent before";
if (toBeSent.isEmpty())
return "Noting to notify, " + stat;
+
NotificationsConfig notifications = cfg.notifications();
- StringBuilder res = new StringBuilder();
- Collection<Notification> values = toBeSent.values();
- for (Notification next : values) {
- if (next.addr.startsWith(SLACK)) {
- String slackUser = next.addr.substring(SLACK.length());
+ Map<String, AtomicInteger> sndStat = new HashMap<>();
- List<String> messages = next.toSlackMarkup();
+ for (Notification next : toBeSent.values()) {
+ String addr = next.addr;
- for (String msg : messages) {
- final boolean snd = SlackSender.sendMessage(slackUser, msg, notifications);
+ try {
+ if (addr.startsWith(SLACK)) {
+ String slackUser = addr.substring(SLACK.length());
- res.append("Send ").append(slackUser).append(": ").append(snd);
- if (!snd)
- break;
+ List<String> messages = next.toSlackMarkup();
+
+ for (String msg : messages) {
+ SlackSender.sendMessage(slackUser, msg, notifications);
+
+ sndStat.computeIfAbsent(addr, k -> new AtomicInteger()).incrementAndGet();
+ }
+ }
+ else {
+ String builds = next.buildIdToIssue.keySet().toString();
+ String subj = "[MTCGA]: " + next.countIssues() + " new failures in builds " + builds + " needs to be handled";
+
+ EmailSender.sendEmail(addr, subj, next.toHtml(), next.toPlainText(), notifications);
+
+ sndStat.computeIfAbsent(addr, k -> new AtomicInteger()).incrementAndGet();
}
}
- else {
- String builds = next.buildIdToIssue.keySet().toString();
- String subj = "[MTCGA]: " + next.countIssues() + " new failures in builds " + builds + " needs to be handled";
+ catch (Exception e) {
+ e.printStackTrace();
+ logger.warn("Unable to notify address [" + addr + "] about build failures", e);
+
+ next.allIssues().forEach(issue -> {
+ IssueKey key = issue.issueKey();
+ // rollback successfull notification
+ issuesStorage.getIsNewAndSetNotified(key, addr, e);
+ });
- EmailSender.sendEmail(next.addr, subj, next.toHtml(), next.toPlainText(), notifications);
- res.append("Send ").append(next.addr).append(" subject: ").append(subj);
+ stat += " ;" + e.getClass().getSimpleName() + ": " + e.getMessage();
}
}
- return res + ", " + stat;
+ return "Send " + sndStat.toString() + "; Statistics: " + stat;
}
/**
diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/Notification.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/Notification.java
index 21ebb72..87fba01 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/Notification.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/tcbot/issue/Notification.java
@@ -17,6 +17,7 @@
package org.apache.ignite.ci.tcbot.issue;
+import java.util.stream.Stream;
import org.apache.ignite.ci.issue.Issue;
import org.apache.ignite.ci.web.model.Version;
import org.apache.ignite.tcbot.common.util.TimeUtil;
@@ -193,4 +194,7 @@ public class Notification {
return sb.toString();
}
+ public Stream<Issue> allIssues() {
+ return this.buildIdToIssue.values().stream().flatMap(Collection::stream);
+ }
}
diff --git a/ignite-tc-helper-web/src/main/webapp/js/issues-1.0.js b/ignite-tc-helper-web/src/main/webapp/js/issues-1.0.js
index 3ce3105..1a282be 100644
--- a/ignite-tc-helper-web/src/main/webapp/js/issues-1.0.js
+++ b/ignite-tc-helper-web/src/main/webapp/js/issues-1.0.js
@@ -26,14 +26,16 @@ function showIssues(result) {
res += " " + result.issues.length;
- res += "<br>";
+ res += ": <br>";
for (var i = 0; i < result.issues.length; i++) {
var issue = result.issues[i];
var color = 'red';
var issueTitle = '';
- res += " <span style='border-color: " + color + "; width:6px; height:6px; display: inline-block; border-width: 4px; color: black; border-style: solid;' title='" + issueTitle + "'></span> ";
+ //res += " <span style='border-color: " + color + "; width:6px; height:6px; display: inline-block; border-width: 4px; color: black; border-style: solid;' title='" + issueTitle + "'></span> ";
+
+ res += "• ";
res += issue.type;
@@ -56,6 +58,13 @@ function showIssues(result) {
res += JSON.stringify(issue.stat);
}
+ if(isDefinedAndFilled(issue.notificationFailed)) {
+ res += " notificationFailed=";
+ res += JSON.stringify(issue.notificationFailed);
+ }
+
+ res += " retry=" + issue.notificationRetry;
+
res += "<br><br>";
}