KYLIN-2856 Refactor slow query detector and history
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/aa9e73e4 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/aa9e73e4 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/aa9e73e4 Branch: refs/heads/master Commit: aa9e73e4f726c8466f95864f3a88357c3deaa795 Parents: 55c6ee7 Author: Li Yang <liy...@apache.org> Authored: Thu Sep 7 13:22:33 2017 +0800 Committer: Hongbin Ma <m...@kyligence.io> Committed: Thu Sep 7 20:03:20 2017 +0800 ---------------------------------------------------------------------- .../apache/kylin/common/KylinConfigBase.java | 2 +- .../kylin/metadata/badquery/BadQueryEntry.java | 17 +++++----- .../badquery/BadQueryHistoryManager.java | 35 +++++--------------- .../badquery/BadQueryHistoryManagerTest.java | 10 +++--- .../kylin/rest/service/BadQueryDetector.java | 30 ++--------------- 5 files changed, 26 insertions(+), 68 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/aa9e73e4/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java ---------------------------------------------------------------------- diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java index 647b953..5f2e8ad 100644 --- a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java +++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java @@ -1023,7 +1023,7 @@ abstract public class KylinConfigBase implements Serializable { } public int getBadQueryHistoryNum() { - return Integer.parseInt(getOptional("kylin.query.badquery-history-number", "10")); + return Integer.parseInt(getOptional("kylin.query.badquery-history-number", "50")); } public int getBadQueryDefaultAlertingSeconds() { http://git-wip-us.apache.org/repos/asf/kylin/blob/aa9e73e4/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryEntry.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryEntry.java b/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryEntry.java index 71ce24b..60ce4ce 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryEntry.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryEntry.java @@ -26,6 +26,7 @@ import org.apache.kylin.common.util.DateFormat; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonProperty; +@SuppressWarnings("serial") @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE, getterVisibility = JsonAutoDetect.Visibility.NONE, isGetterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE) public class BadQueryEntry extends RootPersistentEntity implements Comparable<BadQueryEntry> { @@ -117,13 +118,11 @@ public class BadQueryEntry extends RootPersistentEntity implements Comparable<Ba @Override public int compareTo(BadQueryEntry obj) { - if (this.startTime == obj.startTime) { - return 0; - } else if (this.startTime > obj.startTime) { - return 1; - } else { - return -1; - } + int comp = Long.compare(this.startTime, obj.startTime); + if (comp != 0) + return comp; + else + return this.sql.compareTo(obj.sql); } @Override @@ -140,10 +139,10 @@ public class BadQueryEntry extends RootPersistentEntity implements Comparable<Ba BadQueryEntry entry = (BadQueryEntry) o; - if (!sql.equals(entry.sql)) + if (startTime != entry.startTime) return false; - if (startTime != entry.startTime) + if (!sql.equals(entry.sql)) return false; return true; http://git-wip-us.apache.org/repos/asf/kylin/blob/aa9e73e4/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManager.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManager.java index c7eb133..a916254 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManager.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManager.java @@ -86,45 +86,26 @@ public class BadQueryHistoryManager { return badQueryHistory; } - public BadQueryHistory addEntryToProject(BadQueryEntry badQueryEntry, String project) throws IOException { + public BadQueryHistory upsertEntryToProject(BadQueryEntry badQueryEntry, String project) throws IOException { if (StringUtils.isEmpty(project) || badQueryEntry.getAdj() == null || badQueryEntry.getSql() == null) throw new IllegalArgumentException(); BadQueryHistory badQueryHistory = getBadQueriesForProject(project); NavigableSet<BadQueryEntry> entries = badQueryHistory.getEntries(); - if (entries.size() >= kylinConfig.getBadQueryHistoryNum()) { + + entries.remove(badQueryEntry); // in case the entry already exists and this call means to update + + entries.add(badQueryEntry); + + int maxSize = kylinConfig.getBadQueryHistoryNum(); + if (entries.size() > maxSize) { entries.pollFirst(); } - entries.add(badQueryEntry); - - getStore().putResource(badQueryHistory.getResourcePath(), badQueryHistory, BAD_QUERY_INSTANCE_SERIALIZER); - return badQueryHistory; - } - - public BadQueryHistory updateEntryToProject(BadQueryEntry badQueryEntry, String project) throws IOException { - if (StringUtils.isEmpty(project) || badQueryEntry.getAdj() == null || badQueryEntry.getSql() == null) - throw new IllegalArgumentException(); - BadQueryHistory badQueryHistory = getBadQueriesForProject(project); - NavigableSet<BadQueryEntry> entries = badQueryHistory.getEntries(); - BadQueryEntry entry = entries.floor(badQueryEntry); - entry.setAdj(badQueryEntry.getAdj()); - entry.setRunningSec(badQueryEntry.getRunningSec()); - entry.setServer(badQueryEntry.getServer()); - entry.setThread(badQueryEntry.getThread()); getStore().putResource(badQueryHistory.getResourcePath(), badQueryHistory, BAD_QUERY_INSTANCE_SERIALIZER); - return badQueryHistory; } - public BadQueryHistory addEntryToProject(String sql, long startTime, String adj, float runningSecs, String server, String threadName, String user, String project) throws IOException { - return addEntryToProject(new BadQueryEntry(sql, adj, startTime, runningSecs, server, threadName, user), project); - } - - public BadQueryHistory updateEntryToProject(String sql, long startTime, String adj, float runningSecs, String server, String threadName, String user, String project) throws IOException { - return updateEntryToProject(new BadQueryEntry(sql, adj, startTime, runningSecs, server, threadName, user), project); - } - public void removeBadQueryHistory(String project) throws IOException { getStore().deleteResource(getResourcePathForProject(project)); } http://git-wip-us.apache.org/repos/asf/kylin/blob/aa9e73e4/core-metadata/src/test/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManagerTest.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/test/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManagerTest.java b/core-metadata/src/test/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManagerTest.java index 690e1fe..a2384cb 100644 --- a/core-metadata/src/test/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManagerTest.java +++ b/core-metadata/src/test/java/org/apache/kylin/metadata/badquery/BadQueryHistoryManagerTest.java @@ -64,7 +64,8 @@ public class BadQueryHistoryManagerTest extends LocalFileMetadataTestCase { public void testAddEntryToProject() throws IOException { KylinConfig kylinConfig = getTestConfig(); BadQueryHistoryManager manager = BadQueryHistoryManager.getInstance(kylinConfig); - BadQueryHistory history = manager.addEntryToProject("sql", 1459362239992L, "adj", 100, "server", "t-0", "user", "default"); + BadQueryEntry entry = new BadQueryEntry("sql", "adj", 1459362239992L, 100, "server", "t-0", "user"); + BadQueryHistory history = manager.upsertEntryToProject(entry, "default"); NavigableSet<BadQueryEntry> entries = history.getEntries(); assertEquals(3, entries.size()); @@ -79,7 +80,8 @@ public class BadQueryHistoryManagerTest extends LocalFileMetadataTestCase { assertEquals("t-0", newEntry.getThread()); for (int i = 0; i < kylinConfig.getBadQueryHistoryNum(); i++) { - history = manager.addEntryToProject("sql", 1459362239993L + i, "adj", 100 + i, "server", "t-0", "user", "default"); + BadQueryEntry tmp = new BadQueryEntry("sql", "adj", 1459362239993L + i, 100 + i, "server", "t-0", "user"); + history = manager.upsertEntryToProject(tmp, "default"); } assertEquals(kylinConfig.getBadQueryHistoryNum(), history.getEntries().size()); } @@ -89,8 +91,8 @@ public class BadQueryHistoryManagerTest extends LocalFileMetadataTestCase { KylinConfig kylinConfig = getTestConfig(); BadQueryHistoryManager manager = BadQueryHistoryManager.getInstance(kylinConfig); - manager.addEntryToProject("sql", 1459362239000L, "adj", 100, "server", "t-0", "user", "default"); - BadQueryHistory history = manager.updateEntryToProject("sql", 1459362239000L, "adj2", 120, "server2", "t-1", "user", "default"); + manager.upsertEntryToProject(new BadQueryEntry("sql", "adj", 1459362239000L, 100, "server", "t-0", "user"), "default"); + BadQueryHistory history = manager.upsertEntryToProject(new BadQueryEntry("sql", "adj2", 1459362239000L, 120, "server2", "t-1", "user"), "default"); NavigableSet<BadQueryEntry> entries = history.getEntries(); BadQueryEntry newEntry = entries.floor(new BadQueryEntry("sql", "adj2", 1459362239000L, 120, "server2", "t-1", "user")); http://git-wip-us.apache.org/repos/asf/kylin/blob/aa9e73e4/server-base/src/main/java/org/apache/kylin/rest/service/BadQueryDetector.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/BadQueryDetector.java b/server-base/src/main/java/org/apache/kylin/rest/service/BadQueryDetector.java index 64f91b1..617584a 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/BadQueryDetector.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/BadQueryDetector.java @@ -23,13 +23,10 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; -import java.util.NavigableSet; -import java.util.TreeSet; import java.util.concurrent.ConcurrentMap; import org.apache.kylin.common.KylinConfig; -import org.apache.kylin.common.util.Pair; +import org.apache.kylin.metadata.badquery.BadQueryEntry; import org.apache.kylin.metadata.badquery.BadQueryHistoryManager; import org.apache.kylin.rest.request.SQLRequest; import org.slf4j.Logger; @@ -184,18 +181,6 @@ public class BadQueryDetector extends Thread { private class PersistenceNotifier implements Notifier { BadQueryHistoryManager badQueryManager = BadQueryHistoryManager.getInstance(kylinConfig); String serverHostname; - NavigableSet<Pair<Long, String>> cacheQueue = new TreeSet<>(new Comparator<Pair<Long, String>>() { - @Override - public int compare(Pair<Long, String> o1, Pair<Long, String> o2) { - if (o1.equals(o2)) { - return 0; - } else if (o1.getFirst().equals(o2.getFirst())) { - return o2.getSecond().compareTo(o2.getSecond()); - } else { - return (int) (o1.getFirst() - o2.getFirst()); - } - } - }); public PersistenceNotifier() { try { @@ -209,17 +194,8 @@ public class BadQueryDetector extends Thread { @Override public void badQueryFound(String adj, float runningSec, long startTime, String project, String sql, String user, Thread t) { try { - long cachingSeconds = (kylinConfig.getBadQueryDefaultAlertingSeconds() + 1) * 30L; - Pair<Long, String> sqlPair = new Pair<>(startTime, sql); - if (!cacheQueue.contains(sqlPair)) { - badQueryManager.addEntryToProject(sql, startTime, adj, runningSec, serverHostname, t.getName(), user, project); - cacheQueue.add(sqlPair); - while (!cacheQueue.isEmpty() && (System.currentTimeMillis() - cacheQueue.first().getFirst() > cachingSeconds * 1000 || cacheQueue.size() > kylinConfig.getBadQueryHistoryNum() * 3)) { - cacheQueue.pollFirst(); - } - } else { - badQueryManager.updateEntryToProject(sql, startTime, adj, runningSec, serverHostname, t.getName(), user, project); - } + BadQueryEntry entry = new BadQueryEntry(sql, adj, startTime, runningSec, serverHostname, t.getName(), user); + badQueryManager.upsertEntryToProject(entry, project); } catch (IOException e) { logger.error("Error in bad query persistence.", e); }
