Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#11166] Bypass cache for long SQL queries #11177

Merged
merged 1 commit into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
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 @@
return maxSqlBindValueSize;
}

@Override
public int getMaxSqlLength() {
return maxSqlLength;

Check warning on line 141 in agent-module/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java

View check run for this annotation

Codecov / codecov/patch

agent-module/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java#L141

Added line #L141 was not covered by tests
}

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

@Override
public String toString() {
return "DefaultProfilerConfig{" + "pinpointDisable='" + pinpointDisable + '\'' +
", activeProfile=" + activeProfile +
return "DefaultProfilerConfig{" +

Check warning on line 283 in agent-module/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java

View check run for this annotation

Codecov / codecov/patch

agent-module/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/DefaultProfilerConfig.java#L283

Added line #L283 was not covered by tests
"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 @@
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();

Check warning on line 62 in agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/metadata/SqlMetadataServiceProvider.java

View check run for this annotation

Codecov / codecov/patch

agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/metadata/SqlMetadataServiceProvider.java#L62

Added line #L62 was not covered by tests

UidCachingSqlNormalizer simpleCachingSqlNormalizer = new UidCachingSqlNormalizer(jdbcSqlCacheSize, maxSqlLength);

Check warning on line 64 in agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/metadata/SqlMetadataServiceProvider.java

View check run for this annotation

Codecov / codecov/patch

agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/metadata/SqlMetadataServiceProvider.java#L64

Added line #L64 was not covered by tests
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;

Check warning on line 33 in agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/metadata/UidCachingSqlNormalizer.java

View check run for this annotation

Codecov / codecov/patch

agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/metadata/UidCachingSqlNormalizer.java#L33

Added line #L33 was not covered by tests
}
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);

Check warning on line 63 in agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/metadata/UidCachingSqlNormalizer.java

View check run for this annotation

Codecov / codecov/patch

agent-module/profiler/src/main/java/com/navercorp/pinpoint/profiler/metadata/UidCachingSqlNormalizer.java#L63

Added line #L63 was not covered by tests
}
}
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