From 7ca72a39b1c2cce8a35b2e79871145e4c5970498 Mon Sep 17 00:00:00 2001 From: rbygrave Date: Fri, 6 Aug 2021 11:52:38 +1200 Subject: [PATCH] Change query metric hash from MD5 of sql + name + loc to Checksum of sql --- .../java/io/ebean/meta/MetaQueryMetric.java | 2 +- .../java/io/ebean/meta/MetaQueryPlan.java | 2 +- .../main/java/io/ebean/meta/MetricData.java | 6 ++-- .../java/io/ebean/meta/QueryPlanInit.java | 10 +++--- .../io/ebeaninternal/api/SpiQueryPlan.java | 2 +- .../server/core/DumpMetricsJson.java | 19 ++++++------ .../server/profile/DQueryPlanMeta.java | 16 +++------- .../server/profile/DQueryPlanMetric.java | 2 +- .../server/query/CQueryPlan.java | 25 +++------------ .../server/query/CQueryPlanStats.java | 2 +- .../server/query/DQueryPlanOutput.java | 7 ++--- .../io/ebeaninternal/server/util/Md5.java | 31 ------------------- .../io/ebeaninternal/server/util/Md5Test.java | 18 ----------- .../query/finder/TestCustomerFinder.java | 2 +- 14 files changed, 36 insertions(+), 108 deletions(-) delete mode 100644 ebean-core/src/main/java/io/ebeaninternal/server/util/Md5.java delete mode 100644 ebean-core/src/test/java/io/ebeaninternal/server/util/Md5Test.java diff --git a/ebean-api/src/main/java/io/ebean/meta/MetaQueryMetric.java b/ebean-api/src/main/java/io/ebean/meta/MetaQueryMetric.java index 514b700dc..edfe978e5 100644 --- a/ebean-api/src/main/java/io/ebean/meta/MetaQueryMetric.java +++ b/ebean-api/src/main/java/io/ebean/meta/MetaQueryMetric.java @@ -23,6 +23,6 @@ public interface MetaQueryMetric extends MetaTimedMetric { /** * Return the hash of the plan. */ - String getHash(); + long getHash(); } diff --git a/ebean-api/src/main/java/io/ebean/meta/MetaQueryPlan.java b/ebean-api/src/main/java/io/ebean/meta/MetaQueryPlan.java index fb2b6d095..4f9667bd0 100644 --- a/ebean-api/src/main/java/io/ebean/meta/MetaQueryPlan.java +++ b/ebean-api/src/main/java/io/ebean/meta/MetaQueryPlan.java @@ -30,7 +30,7 @@ public interface MetaQueryPlan { /** * Return the hash of the plan. */ - String getHash(); + long getHash(); /** * Return a description of the bind values. diff --git a/ebean-api/src/main/java/io/ebean/meta/MetricData.java b/ebean-api/src/main/java/io/ebean/meta/MetricData.java index b4606b1d7..9a24aa9f2 100644 --- a/ebean-api/src/main/java/io/ebean/meta/MetricData.java +++ b/ebean-api/src/main/java/io/ebean/meta/MetricData.java @@ -6,7 +6,7 @@ package io.ebean.meta; public class MetricData { private String name; - private String hash; + private long hash; private String loc; private String sql; @@ -30,11 +30,11 @@ public class MetricData { this.name = name; } - public String getHash() { + public long getHash() { return hash; } - public void setHash(String hash) { + public void setHash(long hash) { this.hash = hash; } diff --git a/ebean-api/src/main/java/io/ebean/meta/QueryPlanInit.java b/ebean-api/src/main/java/io/ebean/meta/QueryPlanInit.java index 3c19caa39..9b785ec62 100644 --- a/ebean-api/src/main/java/io/ebean/meta/QueryPlanInit.java +++ b/ebean-api/src/main/java/io/ebean/meta/QueryPlanInit.java @@ -10,7 +10,7 @@ public class QueryPlanInit { private boolean all; - private Set hashes = new HashSet<>(); + private Set hashes = new HashSet<>(); private long thresholdMicros; @@ -47,21 +47,21 @@ public class QueryPlanInit { /** * Return true if the query plan should be initiated based on it's hash. */ - public boolean includeHash(String hash) { - return all || hashes.contains(hash); + public boolean includeHash(long sqlHash) { + return all || hashes.contains(sqlHash); } /** * Return the specific hashes that we want to collect query plans on. */ - public Set getHashes() { + public Set getHashes() { return hashes; } /** * Set the specific hashes that we want to collect query plans on. */ - public void setHashes(Set hashes) { + public void setHashes(Set hashes) { this.hashes = hashes; } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryPlan.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryPlan.java index 96a1eade7..cd6c83114 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryPlan.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryPlan.java @@ -20,7 +20,7 @@ public interface SpiQueryPlan { /** * The hash for the query plan. */ - String getHash(); + long getHash(); /** * The SQL for the query plan. diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DumpMetricsJson.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DumpMetricsJson.java index ed13151c4..240d050f3 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DumpMetricsJson.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DumpMetricsJson.java @@ -197,7 +197,7 @@ class DumpMetricsJson implements ServerMetricsAsJson { metricStart(metric); appendTiming(metric); if (withHash) { - appendExtra("hash", metric.getHash()); + keyVal("hash", metric.getHash()); } if (isIncludeDetail(metric)) { appendExtra("loc", metric.getLocation()); @@ -218,13 +218,14 @@ class DumpMetricsJson implements ServerMetricsAsJson { } private void appendTiming(MetaTimedMetric timedMetric) throws IOException { - key("count"); - val(timedMetric.getCount()); - key("total"); - val(timedMetric.getTotal()); - key("mean"); - val(timedMetric.getMean()); - key("max"); - val(timedMetric.getMax()); + keyVal("count", timedMetric.getCount()); + keyVal("total", timedMetric.getTotal()); + keyVal("mean", timedMetric.getMean()); + keyVal("max", timedMetric.getMax()); + } + + private void keyVal(String key, long value) throws IOException { + key(key); + val(value); } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMeta.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMeta.java index 0690b5fdd..0f25f0d9c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMeta.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMeta.java @@ -1,7 +1,7 @@ package io.ebeaninternal.server.profile; import io.ebean.ProfileLocation; -import io.ebeaninternal.server.util.Md5; +import io.ebeaninternal.server.util.Checksum; class DQueryPlanMeta { @@ -10,7 +10,7 @@ class DQueryPlanMeta { private final ProfileLocation profileLocation; private final String name; private final String sql; - private final String hash; + private final long hash; DQueryPlanMeta(Class type, String label, ProfileLocation profileLocation, String sql) { this.type = type; @@ -22,22 +22,14 @@ class DQueryPlanMeta { name += "_" + label; } this.name = name; - this.hash = initHash(); - } - - private String initHash() { - StringBuilder sb = new StringBuilder(sql).append("|").append(name); - if (profileLocation != null) { - sb.append("|").append(profileLocation.location()); - } - return Md5.hash(sb.toString()); + this.hash = Checksum.checksum(sql); } public Class getType() { return type; } - public String getHash() { + public long getHash() { return hash; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java index 21559a152..765679404 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java @@ -59,7 +59,7 @@ class DQueryPlanMetric implements QueryPlanMetric { } @Override - public String getHash() { + public long getHash() { return meta.getHash(); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java index 295cbe7c2..3c322190f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java @@ -13,12 +13,12 @@ import io.ebeaninternal.api.SpiQueryBindCapture; import io.ebeaninternal.api.SpiQueryPlan; import io.ebeaninternal.server.core.OrmQueryRequest; import io.ebeaninternal.server.core.timezone.DataTimeZone; +import io.ebeaninternal.server.util.Checksum; import io.ebeaninternal.server.util.Str; import io.ebeaninternal.server.query.CQueryPlanStats.Snapshot; import io.ebeaninternal.server.type.DataBind; import io.ebeaninternal.server.type.DataBindCapture; import io.ebeaninternal.server.type.RsetDataReader; -import io.ebeaninternal.server.util.Md5; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -68,7 +68,7 @@ public class CQueryPlan implements SpiQueryPlan { private final boolean rawSql; private final String sql; - private final String hash; + private final long hash; private final String logWhereSql; @@ -118,7 +118,7 @@ public class CQueryPlan implements SpiQueryPlan { this.stats = new CQueryPlanStats(this); this.dependentTables = sqlTree.dependentTables(); this.bindCapture = initBindCapture(query); - this.hash = md5Hash(); + this.hash = Checksum.checksum(sql); } /** @@ -143,7 +143,7 @@ public class CQueryPlan implements SpiQueryPlan { this.stats = new CQueryPlanStats(this); this.dependentTables = sqlTree.dependentTables(); this.bindCapture = initBindCaptureRaw(sql, query); - this.hash = md5Hash(); + this.hash = Checksum.checksum(sql); } private String deriveName(String label, SpiQuery.Type type, String simpleName) { @@ -193,7 +193,7 @@ public class CQueryPlan implements SpiQueryPlan { } @Override - public String getHash() { + public long getHash() { return hash; } @@ -276,21 +276,6 @@ public class CQueryPlan implements SpiQueryPlan { return rawSql ? planKey.getPartialKey() + "_" + hash : planKey.getPartialKey(); } - /** - * Return the MD5 hash of the sql. - */ - private String md5Hash() { - StringBuilder sb = new StringBuilder(sql) - .append("|").append(name) - .append("|").append(location); - try { - return Md5.hash(sb.toString()); - } catch (Exception e) { - logger.error("Failed to MD5 hash the query", e); - return "error"; - } - } - SqlTree getSqlTree() { return sqlTree; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlanStats.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlanStats.java index 23de6de5c..3f7f61070 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlanStats.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryPlanStats.java @@ -127,7 +127,7 @@ public final class CQueryPlanStats { } @Override - public String getHash() { + public long getHash() { return queryPlan.getHash(); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/DQueryPlanOutput.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/DQueryPlanOutput.java index 8065a62dc..3d8c7aaac 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/DQueryPlanOutput.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/DQueryPlanOutput.java @@ -16,12 +16,11 @@ class DQueryPlanOutput implements MetaQueryPlan, SpiDbQueryPlan { private final String sql; private final String bind; private final String plan; - - private String hash; + private final long hash; private long queryTimeMicros; private long captureCount; - DQueryPlanOutput(Class beanType, String label, String hash, String sql, ProfileLocation profileLocation, String bind, String plan) { + DQueryPlanOutput(Class beanType, String label, long hash, String sql, ProfileLocation profileLocation, String bind, String plan) { this.beanType = beanType; this.label = label; this.hash = hash; @@ -32,7 +31,7 @@ class DQueryPlanOutput implements MetaQueryPlan, SpiDbQueryPlan { } @Override - public String getHash() { + public long getHash() { return hash; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/util/Md5.java b/ebean-core/src/main/java/io/ebeaninternal/server/util/Md5.java deleted file mode 100644 index 7329d3f25..000000000 --- a/ebean-core/src/main/java/io/ebeaninternal/server/util/Md5.java +++ /dev/null @@ -1,31 +0,0 @@ -package io.ebeaninternal.server.util; - -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; - -public final class Md5 { - - /** - * Return the MD5 hash of the underlying sql. - */ - public static String hash(String content) { - try { - MessageDigest md = MessageDigest.getInstance("MD5"); - return digestToHex(md.digest(content.getBytes(StandardCharsets.UTF_8))); - } catch (Exception e) { - throw new RuntimeException("MD5 hashing failed", e); - } - } - - /** - * Convert the digest into a hex value. - */ - private static String digestToHex(byte[] digest) { - StringBuilder sb = new StringBuilder(); - for (byte aDigest : digest) { - sb.append(Integer.toString((aDigest & 0xff) + 0x100, 16).substring(1)); - } - return sb.toString(); - } - -} diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/util/Md5Test.java b/ebean-core/src/test/java/io/ebeaninternal/server/util/Md5Test.java deleted file mode 100644 index d9db27d3d..000000000 --- a/ebean-core/src/test/java/io/ebeaninternal/server/util/Md5Test.java +++ /dev/null @@ -1,18 +0,0 @@ -package io.ebeaninternal.server.util; - -import org.junit.Test; - -import static org.junit.Assert.assertEquals; - -public class Md5Test { - - @Test - public void hash() throws Exception { - - String content = "some random content we wish to hash"; - String hash1 = Md5.hash(content); - String hash2 = Md5.hash(content); - assertEquals(hash1, hash2); - } - -} diff --git a/ebean-core/src/test/java/org/tests/query/finder/TestCustomerFinder.java b/ebean-core/src/test/java/org/tests/query/finder/TestCustomerFinder.java index eeefe42c2..8e8c5a288 100644 --- a/ebean-core/src/test/java/org/tests/query/finder/TestCustomerFinder.java +++ b/ebean-core/src/test/java/org/tests/query/finder/TestCustomerFinder.java @@ -244,7 +244,7 @@ public class TestCustomerFinder extends BaseTestCase { assertThat(metricsJson).contains("\"name\":\"orm.Customer.findList\""); assertThat(metricsJson).contains("\"loc\":\"CustomerFinder.byNameStatus(CustomerFinder.java:44)\""); if (isH2() || isPostgres()) { - assertThat(metricsJson).contains("\"hash\":\"cc20eb930403cfd418db2d0475c6e26a\""); + assertThat(metricsJson).contains("\"hash\":3634991469"); assertThat(metricsJson).contains("\"sql\":\"select t0.id, t0.status,"); } }