Skip to content

Commit

Permalink
[#11166] Bypass cache for long SQL queries
Browse files Browse the repository at this point in the history
  • Loading branch information
kojandy committed Jul 1, 2024
1 parent 268987d commit 221ced9
Show file tree
Hide file tree
Showing 12 changed files with 180 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public class DefaultProfilerConfig implements ProfilerConfig {
private boolean traceSqlBindValue = false;
@Value("${profiler.jdbc.maxsqlbindvaluesize}")
private int maxSqlBindValueSize = 1024;
@Value("${profiler.jdbc.sqlcachelengthlimit}")
private int maxSqlLength = 2048;

@Value("${profiler.transport.grpc.stats.logging.period}")
private String grpcStatLoggingPeriod = "PT1M";
Expand Down Expand Up @@ -134,6 +136,11 @@ public int getMaxSqlBindValueSize() {
return maxSqlBindValueSize;
}

@Override
public int getMaxSqlLength() {
return maxSqlLength;
}

@Override
public String getGrpcStatLoggingPeriod() {
return grpcStatLoggingPeriod;
Expand Down Expand Up @@ -273,16 +280,22 @@ public Map<String, String> readPattern(String propertyNamePatternRegex) {

@Override
public String toString() {
return "DefaultProfilerConfig{" + "pinpointDisable='" + pinpointDisable + '\'' +
", activeProfile=" + activeProfile +
return "DefaultProfilerConfig{" +
"properties=" + properties +
", pinpointDisable='" + pinpointDisable + '\'' +
", logDirMaxBackupSize=" + logDirMaxBackupSize +
", activeProfile='" + activeProfile + '\'' +
", staticResourceCleanup=" + staticResourceCleanup +
", transportModule=" + transportModule +
", jdbcSqlCacheSize=" + jdbcSqlCacheSize +
", traceSqlBindValue=" + traceSqlBindValue +
", maxSqlBindValueSize=" + maxSqlBindValueSize +
", maxSqlLength=" + maxSqlLength +
", grpcStatLoggingPeriod='" + grpcStatLoggingPeriod + '\'' +
", httpStatusCodeErrors=" + httpStatusCodeErrors +
", injectionModuleFactoryClazzName='" + injectionModuleFactoryClazzName + '\'' +
", applicationNamespace='" + applicationNamespace + '\'' +
", agentClassloaderAdditionalLibs='" + agentClassloaderAdditionalLibs + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public interface ProfilerConfig {

int getMaxSqlBindValueSize();

int getMaxSqlLength();

String getGrpcStatLoggingPeriod();

@InterfaceAudience.Private
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
package com.navercorp.pinpoint.profiler.cache;

import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.hash.HashFunction;
import com.google.common.hash.Hashing;

import java.nio.charset.StandardCharsets;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;

public class UidCache implements Cache<String, Result<byte[]>> {

// zero means not exist.
private final ConcurrentMap<String, Result<byte[]>> cache;

private final HashFunction hashFunction = Hashing.murmur3_128();
private final Function<String, byte[]> uidFunction;

public UidCache(int cacheSize) {
public UidCache(int cacheSize, Function<String, byte[]> uidFunction) {
this.cache = createCache(cacheSize);
this.uidFunction = uidFunction;
}

private ConcurrentMap<String, Result<byte[]>> createCache(int maxCacheSize) {
Expand All @@ -33,18 +31,12 @@ public Result<byte[]> put(String value) {
return find;
}

final byte[] uid = calculateUid(value);
final byte[] uid = uidFunction.apply(value);
final Result<byte[]> result = new Result<>(false, uid);
final Result<byte[]> before = this.cache.putIfAbsent(value, result);
if (before != null) {
return before;
}
return new Result<>(true, uid);
}

private byte[] calculateUid(String value) {
return hashFunction
.hashString(value, StandardCharsets.UTF_8)
.asBytes();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,15 @@
import com.navercorp.pinpoint.common.profiler.message.EnhancedDataSender;
import com.navercorp.pinpoint.io.ResponseMessage;
import com.navercorp.pinpoint.profiler.cache.SimpleCache;
import com.navercorp.pinpoint.profiler.cache.UidCache;
import com.navercorp.pinpoint.profiler.context.module.MetadataDataSender;
import com.navercorp.pinpoint.profiler.context.monitor.config.MonitorConfig;
import com.navercorp.pinpoint.profiler.metadata.CachingSqlNormalizer;
import com.navercorp.pinpoint.profiler.metadata.DefaultCachingSqlNormalizer;
import com.navercorp.pinpoint.profiler.metadata.DefaultSqlMetaDataService;
import com.navercorp.pinpoint.profiler.metadata.MetaDataType;
import com.navercorp.pinpoint.profiler.metadata.ParsingResultInternal;
import com.navercorp.pinpoint.profiler.metadata.SimpleCachingSqlNormalizer;
import com.navercorp.pinpoint.profiler.metadata.SqlCacheService;
import com.navercorp.pinpoint.profiler.metadata.SqlMetaDataService;
import com.navercorp.pinpoint.profiler.metadata.SqlUidMetaDataService;
import com.navercorp.pinpoint.profiler.metadata.UidCachingSqlNormalizer;

import java.util.Objects;

Expand Down Expand Up @@ -61,13 +59,14 @@ public SqlMetaDataService get() {
final int jdbcSqlCacheSize = profilerConfig.getJdbcSqlCacheSize();

if (monitorConfig.isSqlStatEnable()) {
final UidCache stringCache = new UidCache(jdbcSqlCacheSize);
CachingSqlNormalizer<ParsingResultInternal<byte[]>> simpleCachingSqlNormalizer = new DefaultCachingSqlNormalizer<>(stringCache);
final int maxSqlLength = profilerConfig.getMaxSqlLength();

UidCachingSqlNormalizer simpleCachingSqlNormalizer = new UidCachingSqlNormalizer(jdbcSqlCacheSize, maxSqlLength);
SqlCacheService<byte[]> sqlCacheService = new SqlCacheService<>(enhancedDataSender, simpleCachingSqlNormalizer);
return new SqlUidMetaDataService(sqlCacheService);
} else {
final SimpleCache<String> stringCache = simpleCacheFactory.newSimpleCache(jdbcSqlCacheSize);
CachingSqlNormalizer<ParsingResultInternal<Integer>> simpleCachingSqlNormalizer = new DefaultCachingSqlNormalizer<>(stringCache);
SimpleCachingSqlNormalizer simpleCachingSqlNormalizer = new SimpleCachingSqlNormalizer(stringCache);
SqlCacheService<Integer> sqlCacheService = new SqlCacheService<>(enhancedDataSender, simpleCachingSqlNormalizer);
return new DefaultSqlMetaDataService(sqlCacheService);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.navercorp.pinpoint.profiler.metadata;

import com.navercorp.pinpoint.bootstrap.context.ParsingResult;
import com.navercorp.pinpoint.common.annotations.VisibleForTesting;
import com.navercorp.pinpoint.common.trace.AnnotationKey;
import com.navercorp.pinpoint.common.util.IntStringStringValue;
import com.navercorp.pinpoint.common.util.StringUtils;
Expand Down Expand Up @@ -65,6 +66,7 @@ public Annotation<?> newSqlAnnotation(ParsingResult parsingResult, String bindVa
return Annotations.of(AnnotationKey.SQL_ID.getCode(), sqlValue);
}

@VisibleForTesting
static MetaDataType newSqlMetaData(ParsingResultInternal<Integer> parsingResult) {
return new SqlMetaData(parsingResult.getId(), parsingResult.getSql());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,19 @@
/**
* @author emeroad
*/
public class DefaultCachingSqlNormalizer<ID> implements CachingSqlNormalizer<ParsingResultInternal<ID>> {
public class SimpleCachingSqlNormalizer implements CachingSqlNormalizer<ParsingResultInternal<Integer>> {
private final Logger logger = LogManager.getLogger(this.getClass());

protected final Logger logger = LogManager.getLogger(this.getClass());

private final Cache<String, Result<ID>> sqlCache;
private final Cache<String, Result<Integer>> sqlCache;
private final SqlNormalizer sqlNormalizer;

public DefaultCachingSqlNormalizer(Cache<String, Result<ID>> sqlCache) {
public SimpleCachingSqlNormalizer(Cache<String, Result<Integer>> sqlCache) {
this.sqlCache = Objects.requireNonNull(sqlCache, "sqlCache");
this.sqlNormalizer = new DefaultSqlNormalizer();
}


@Override
public boolean normalizedSql(ParsingResultInternal<ID> parsingResult) {
public boolean normalizedSql(ParsingResultInternal<Integer> parsingResult) {
if (parsingResult == null) {
return false;
}
Expand All @@ -55,7 +53,7 @@ public boolean normalizedSql(ParsingResultInternal<ID> parsingResult) {
final String originalSql = parsingResult.getOriginalSql();
final NormalizedSql normalizedSql = this.sqlNormalizer.normalizeSql(originalSql);

final Result<ID> cachingResult = this.sqlCache.put(normalizedSql.getNormalizedSql());
final Result<Integer> cachingResult = this.sqlCache.put(normalizedSql.getNormalizedSql());

boolean success = parsingResult.setId(cachingResult.getId());
if (!success) {
Expand All @@ -68,5 +66,4 @@ public boolean normalizedSql(ParsingResultInternal<ID> parsingResult) {

return cachingResult.isNewValue();
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.navercorp.pinpoint.profiler.metadata;

import com.navercorp.pinpoint.bootstrap.context.ParsingResult;
import com.navercorp.pinpoint.common.annotations.VisibleForTesting;
import com.navercorp.pinpoint.common.trace.AnnotationKey;
import com.navercorp.pinpoint.common.util.BytesStringStringValue;
import com.navercorp.pinpoint.common.util.StringUtils;
Expand Down Expand Up @@ -45,6 +46,7 @@ public Annotation<?> newSqlAnnotation(ParsingResult parsingResult, String bindVa
return Annotations.of(AnnotationKey.SQL_UID.getCode(), sqlValue);
}

@VisibleForTesting
static MetaDataType newSqlUidMetaData(ParsingResultInternal<byte[]> parsingResult) {
return new SqlUidMetaData(parsingResult.getId(), parsingResult.getSql());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.navercorp.pinpoint.profiler.metadata;

import com.google.common.hash.Hashing;
import com.navercorp.pinpoint.common.profiler.sql.DefaultSqlNormalizer;
import com.navercorp.pinpoint.common.profiler.sql.NormalizedSql;
import com.navercorp.pinpoint.common.profiler.sql.SqlNormalizer;
import com.navercorp.pinpoint.profiler.cache.Cache;
import com.navercorp.pinpoint.profiler.cache.Result;
import com.navercorp.pinpoint.profiler.cache.UidCache;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.nio.charset.StandardCharsets;
import java.util.function.Function;

public class UidCachingSqlNormalizer implements CachingSqlNormalizer<ParsingResultInternal<byte[]>> {
private final Logger logger = LogManager.getLogger(this.getClass());
private final Function<String, byte[]> hashFunction = x -> Hashing.murmur3_128().hashString(x, StandardCharsets.UTF_8).asBytes();

private final Cache<String, Result<byte[]>> sqlCache;
private final SqlNormalizer sqlNormalizer;
private final int lengthLimit;

public UidCachingSqlNormalizer(int cacheSize, int lengthLimit) {
this.sqlCache = new UidCache(cacheSize, hashFunction);
this.sqlNormalizer = new DefaultSqlNormalizer();
this.lengthLimit = lengthLimit;
}

@Override
public boolean normalizedSql(ParsingResultInternal<byte[]> parsingResult) {
if (parsingResult == null) {
return false;
}
if (parsingResult.getId() != null) {
// already cached
return false;
}

final String originalSql = parsingResult.getOriginalSql();
final NormalizedSql normalizedSql = this.sqlNormalizer.normalizeSql(originalSql);

byte[] uid;
boolean isNewValue;
if (lengthLimit == -1 || normalizedSql.getNormalizedSql().length() <= lengthLimit) {
final Result<byte[]> cachingResult = this.sqlCache.put(normalizedSql.getNormalizedSql());
uid = cachingResult.getId();
isNewValue = cachingResult.isNewValue();
} else {
// bypass cache
uid = hashFunction.apply(normalizedSql.getNormalizedSql());
isNewValue = true;
}

setParsingResult(parsingResult, uid, normalizedSql);
return isNewValue;
}

private void setParsingResult(ParsingResultInternal<byte[]> parsingResult, byte[] uid, NormalizedSql normalizedSql) {
boolean success = parsingResult.setId(uid);
if (!success) {
if (logger.isWarnEnabled()) {
logger.warn("invalid state. setSqlUid fail setUid:{}, ParsingResultInternal:{}", uid, parsingResult);
}
}
parsingResult.setSql(normalizedSql.getNormalizedSql());
parsingResult.setOutput(normalizedSql.getParseParameter());
}
}
Original file line number Diff line number Diff line change
@@ -1,40 +1,55 @@
package com.navercorp.pinpoint.profiler.cache;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import java.util.Arrays;
import java.util.function.Function;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class UidCacheTest {
@Test
public void sameValue() {
UidCache cache1 = newCache();
UidCache cache2 = newCache();
@ExtendWith(MockitoExtension.class)
class UidCacheTest {
UidCache sut;

Result<byte[]> result1 = cache1.put("test");
Result<byte[]> result2 = cache2.put("test");
@Mock
Function<String, byte[]> uidFunction;

assertTrue(result1.isNewValue());
assertTrue(result2.isNewValue());
assertArrayEquals(result1.getId(), result2.getId());
@BeforeEach
void setUp() {
sut = new UidCache(1024, uidFunction);

when(uidFunction.apply(any()))
.thenReturn(new byte[]{});
}

@Test
public void differentValue() {
UidCache cache = newCache();
void sameValue() {
Result<byte[]> result1 = sut.put("test");
Result<byte[]> result2 = sut.put("test");

assertTrue(result1.isNewValue());
assertFalse(result2.isNewValue());

Result<byte[]> result1 = cache.put("test");
Result<byte[]> result2 = cache.put("different");
verify(uidFunction, times(1)).apply("test");
}

@Test
void differentValue() {
Result<byte[]> result1 = sut.put("test");
Result<byte[]> result2 = sut.put("different");

assertTrue(result1.isNewValue());
assertTrue(result2.isNewValue());
assertFalse(Arrays.equals(result1.getId(), result2.getId()));
}

private UidCache newCache() {
return new UidCache(1024);
verify(uidFunction, times(1)).apply("test");
verify(uidFunction, times(1)).apply("different");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class SimpleCachingSqlNormalizerTest {
@Test
public void testNormalizedSql() {
SimpleCache<String> cache = newCache(1);
CachingSqlNormalizer<ParsingResultInternal<Integer>> normalizer = new DefaultCachingSqlNormalizer<>(cache);
SimpleCachingSqlNormalizer normalizer = new SimpleCachingSqlNormalizer(cache);
ParsingResultInternal<Integer> parsingResult = new DefaultParsingResult("select * from dual");

boolean newCache = normalizer.normalizedSql(parsingResult);
Expand All @@ -47,7 +47,7 @@ public void testNormalizedSql() {
@Test
public void testNormalizedSql_cache_expire() {
SimpleCache<String> cache = newCache(1);
CachingSqlNormalizer<ParsingResultInternal<Integer>> normalizer = new DefaultCachingSqlNormalizer<>(cache);
SimpleCachingSqlNormalizer normalizer = new SimpleCachingSqlNormalizer(cache);
ParsingResultInternal<Integer> parsingResult = new DefaultParsingResult("select * from table1");
boolean newCache = normalizer.normalizedSql(parsingResult);
Assertions.assertTrue(newCache, "newCacheState");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class SqlCacheServiceTest {
public void cacheSql() {
final EnhancedDataSender<MetaDataType, ResponseMessage> dataSender = mock(EnhancedDataSender.class);
SimpleCache<String> sqlCache = new SimpleCache<>(new IdAllocator.ZigZagAllocator(), 100);
CachingSqlNormalizer<ParsingResultInternal<Integer>> simpleCachingSqlNormalizer = new DefaultCachingSqlNormalizer<>(sqlCache);
SimpleCachingSqlNormalizer simpleCachingSqlNormalizer = new SimpleCachingSqlNormalizer(sqlCache);
final SqlCacheService<Integer> sqlMetaDataService = new SqlCacheService<>(dataSender, simpleCachingSqlNormalizer);

final String sql = "select * from A";
Expand Down
Loading

0 comments on commit 221ced9

Please sign in to comment.