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

JdbcHelper urlTofactory Map replacement #1114

Merged
merged 3 commits into from
Feb 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions agent-bridge-datastore/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ dependencies {
implementation(project(":newrelic-weaver-api"))
implementation(project(":agent-bridge"))

testImplementation('org.awaitility:awaitility:4.2.0')
testImplementation(fileTree(dir: 'src/test/resources/com/newrelic/agent/bridge', include: '*.jar'))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package com.newrelic.agent.bridge.datastore;

import com.newrelic.agent.bridge.AgentBridge;

import java.time.Instant;
import java.util.AbstractMap;
import java.util.Set;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;

/**
* A cache implementation that uses a composition pattern to wrap a {@link ConcurrentHashMap}
* populated with {@link TimestampedMapValue} instances. An instance of this class is constructed
* with function that contains the logic of when to evict items from the cache.
*
* @param <K> the class type of the cache key
* @param <V> the class type of the value object
*/
public class ExpiringValueCache<K, V> {
jtduffy marked this conversation as resolved.
Show resolved Hide resolved

private final ConcurrentHashMap<K, TimestampedMapValue<V>> wrappedMap;

/**
* Create a new ExpiringValueCache with the specified timer interval and expiration
* logic function that will determine if an entry should be removed from the cache.
*
* <pre>
* // Example: This is our threshold for expiring an entry - anything not accessed within
* // 10 seconds ago will be evicted.
* Instant expireThreshold = Instant.now().minusMillis(10000)
* ExpiringValueCache.ExpiringValueLogicFunction<Integer> func =
* (createdTime, timeLastAccessed) -> timeLastAccessed.isBefore(Instant.now().minusMillis(expireThreshold));
* </pre>
* @param timerThreadName an identifier given to the instance; used in log messages
* @param timerIntervalMilli the interval time, in milliseconds, of how often the timer fires to check for items
* to be evicted
* @param expireLogicFunction the {@link ExpiringValueLogicFunction} lambda that contains the
* logic to determine if an entry should be removed from the map
*/
public ExpiringValueCache(String timerThreadName, long timerIntervalMilli,
ExpiringValueLogicFunction expireLogicFunction) {
wrappedMap = new ConcurrentHashMap<>();

TimerTask task = new TimerTask() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a static ScheduledThreadPoolExecutor?
Or else we'd be creating and holding one Thread per cache.

public void run() {
AgentBridge.getAgent().getLogger().log(Level.FINE, "ExpiringValueCache timer [{0}] firing", timerThreadName);

wrappedMap.forEach((key, val) -> {
if (expireLogicFunction.shouldExpireValue(val.getTimeCreated(), val.getTimeLastAccessed())) {
AgentBridge.getAgent().getLogger().log(Level.FINEST, "Removing key [{0}] from cache [{}]", key, timerThreadName);
wrappedMap.remove(key);
}
});
}
};

//The initial fire delay will just be the supplied interval for simplicity
Timer timer = new Timer(timerThreadName, true);
timer.scheduleAtFixedRate(task, timerIntervalMilli, timerIntervalMilli);

AgentBridge.getAgent().getLogger().log(Level.INFO, "ExpiringValueCache instance with timer: [{0}], " +
"timer interval: {1}ms created", timerThreadName, timerIntervalMilli);
}

/**
* Return the value mapped by the supplied key.
*
* @param key the key for the target value
* @return the target value
*/
public V get(K key) {
TimestampedMapValue<V> testValue = wrappedMap.get(key);
return testValue == null ? null : testValue.getValue();
}

/**
* Add a new value to the map.
*
* @param key the key for the supplied value
* @param value the value to add to the map, wrapped in a TimestampedMapValue instance
*/
public V put(K key, V value) {
if (value == null) {
throw new NullPointerException();
}
wrappedMap.put(key, new TimestampedMapValue<>(value));
return value;
}

public boolean containsKey(K key) {
return wrappedMap.containsKey(key);
}

public int size() {
return wrappedMap.size();
}

/**
* A functional interface for implementing the logic to determine if an entry in the map should
* be removed based on the supplied {@link TimestampedMapValue} instance.
*/
@FunctionalInterface
public interface ExpiringValueLogicFunction {
boolean shouldExpireValue(Instant createdTime, Instant timeLastAccessed);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.Statement;
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Map;
Expand All @@ -38,12 +40,24 @@ protected Boolean initialValue() {
}
};

/**
* Any values in the urlToFactory or urlToDatabaseName Maps older than this value will
* be evicted from the cache when the {@link ExpiringValueCache} timer fires. This
* will also be used as the timer interval value.
*/
private static final long CACHE_EXPIRATION_AGE_MILLI = Duration.ofHours(8).toMillis();

private static final ExpiringValueCache.ExpiringValueLogicFunction EXPIRE_FUNC =
(timeCreated, timeLastAccessed) -> timeLastAccessed.isBefore(Instant.now().minusMillis(CACHE_EXPIRATION_AGE_MILLI));

// This will contain every vendor type that we detected on the client system
private static final Map<String, DatabaseVendor> typeToVendorLookup = new ConcurrentHashMap<>(10);
private static final Map<Class<?>, DatabaseVendor> classToVendorLookup = AgentBridge.collectionFactory.createConcurrentWeakKeyedMap();
private static final Map<String, ConnectionFactory> urlToFactory = new ConcurrentHashMap<>(5);
private static final Map<String, String> urlToDatabaseName = new ConcurrentHashMap<>(5);
private static final ExpiringValueCache<String, ConnectionFactory> urlToFactory =
new ExpiringValueCache<>("urlToFactoryCache", CACHE_EXPIRATION_AGE_MILLI, EXPIRE_FUNC);

private static final ExpiringValueCache<String, String> urlToDatabaseName =
new ExpiringValueCache<>("urlToDatabaseNameCache", CACHE_EXPIRATION_AGE_MILLI, EXPIRE_FUNC);
private static final Map<Statement, String> statementToSql = AgentBridge.collectionFactory.createConcurrentWeakKeyedMap();
private static final Map<Connection, String> connectionToIdentifier = AgentBridge.collectionFactory.createConcurrentWeakKeyedMap();
private static final Map<Connection, String> connectionToURL = AgentBridge.collectionFactory.createConcurrentWeakKeyedMap();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.newrelic.agent.bridge.datastore;

import java.time.Instant;

/**
* A class designed to wrap a map value in a facade that contains the creation time
* and the time the value was last accessed. This wrapper object can then be used in
* other Map/cache classes that can utilize these timestamps for specific
* use cases (expiring map entries based on value age, for example).
*
* @param <V> the class type of the underlying value
*/
public class TimestampedMapValue<V> {

private final V wrappedValue;

private final Instant timeCreated;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can these be longs?
it would save some allocation from the heap.

private Instant timeLastAccessed;

/**
* Construct a new TimestampedMapValue, wrapping the supplied value of type V.
* @param wrappedValue the value to be wrapped
*/
public TimestampedMapValue(V wrappedValue) {
this.wrappedValue = wrappedValue;

Instant currentTime = Instant.now();
this.timeCreated = currentTime;
this.timeLastAccessed = currentTime;
}

/**
* Return the value wrapped by this TimestampedMapValue.
*
* @return the underlying, wrapped value
*/
public V getValue() {
timeLastAccessed = Instant.now();
return this.wrappedValue;
}

/**
* Return the {@link Instant} of when this TimestampedMapValue was created.
*
* @return the creation timestamp as an {@link Instant}
*/
public Instant getTimeCreated() {
return this.timeCreated;
}

/**
* Return the {@link Instant} of when the underlying value was last accessed.
*
* @return @return the value's last accessed timestamp as an {@link Instant}
*/
public Instant getTimeLastAccessed() {
return this.timeLastAccessed;
}

/**
* Return true if the underlying value was last accessed after the supplied time.
*
* @param targetTime the {@link Instant} to compare the last accessed time with
*
* @return true if the underlying value was last accessed after the supplied time,
* false otherwise
*/
public boolean lastAccessedAfter(Instant targetTime) {
return this.timeLastAccessed.isAfter(targetTime);
}

/**
* Return true if the underlying value was last accessed before the supplied time.
*
* @param targetTime the {@link Instant} to compare the last accessed time with
*
* @return if the underlying value was last accessed before the supplied time,
* false otherwise
*/
public boolean lastAccessedPriorTo(Instant targetTime) {
return this.timeLastAccessed.isBefore(targetTime);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package com.newrelic.agent.bridge.datastore;

import static java.util.concurrent.TimeUnit.SECONDS;
import static org.awaitility.Awaitility.*;
import static org.junit.Assert.*;

import org.junit.Test;

import java.time.Duration;
import java.time.Instant;

public class ExpiringValueCacheTest {

private static final String VALUE_PREFIX = "val";

@Test
public void constructor_WithNoOpLambda_RemovesNoItems() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
int mapSize = testMap.size();

//Let the timer fire a few times and make sure the map doesn't change during that time
await()
.during(Duration.ofSeconds(3))
.atMost(Duration.ofSeconds(4))
.until(() -> testMap.size() == mapSize);
}

@Test
public void timer_WithValidLambda_ExpiresTargetRecords() {
ExpiringValueCache.ExpiringValueLogicFunction func =
(createdTime, timeLastAccessed) -> timeLastAccessed.isBefore(Instant.now().plusMillis(10000));;

ExpiringValueCache<String, String> testMap = generateNewHashMap(1000, func);

await()
.atMost(3, SECONDS)
.until(() -> testMap.size() == 0);
}

@Test
public void timer_WithValidLambda_LeavesMapUnmodified() {
ExpiringValueCache.ExpiringValueLogicFunction func =
(createdTime, timeLastAccessed) -> timeLastAccessed.isBefore(Instant.now().minusMillis(10000));;
ExpiringValueCache<String, String> testMap = generateNewHashMap(1000, func);
int mapSize = testMap.size();

await()
.during(Duration.ofSeconds(3))
.atMost(Duration.ofSeconds(4))
.until(() -> testMap.size() == mapSize);
}

@Test
public void containsKey_WithValidKey_ReturnsTrue() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
assertTrue(testMap.containsKey("1"));
}

@Test
public void size_ReturnsProperValue() {
ExpiringValueCache<String, String> testMap = new ExpiringValueCache<>("testTimer", 1000, noOpLogicFunction());
assertEquals(0, testMap.size());

testMap.put("1", "Borderlands");
testMap.put("2", "Wonderlands");
assertEquals(2, testMap.size());
}

@Test
public void getValue_WithValidKey_ReturnsProperValue() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
assertEquals(VALUE_PREFIX + "1", testMap.get("1"));
}

@Test
public void getValue_WithInvalidKey_ReturnsNull() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
assertNull(testMap.get("zzzzzzz"));
}

@Test
public void putValue_WithValidKeyAndValue_AddsSuccessfully() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
int beforeMapSize = testMap.size();
testMap.put("222", "Borderlands");

assertEquals(beforeMapSize + 1, testMap.size());
}

@Test(expected = NullPointerException.class)
public void putValue_WithNullKey_ThrowsException() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
testMap.put(null, "Borderlands");
}

@Test(expected = NullPointerException.class)
public void putValue_WithNullValue_ThrowsException() {
ExpiringValueCache<String, String> testMap = generateNewHashMap(500, noOpLogicFunction());
testMap.put("222", null);
}

private ExpiringValueCache<String, String> generateNewHashMap(
long timerInterval, ExpiringValueCache.ExpiringValueLogicFunction func) {

ExpiringValueCache<String, String> testMap = new ExpiringValueCache<>("testTimer", timerInterval, func);
for(int i=0; i< 20; i++) {
testMap.put(Integer.toString(i), VALUE_PREFIX + i);
}

return testMap;
}

private ExpiringValueCache.ExpiringValueLogicFunction noOpLogicFunction() {
return (created, accessed) -> false;
}

}
Loading