Merge pull request #2294 from newrelic/db-parser-optimization

Prevent cache lock for long DB statement parsing

Author: jtduffy

newrelic-agent/src/main/java/com/newrelic/agent/database/CachingDatabaseStatementParser.java

@@ -59,9 +59,14 @@ public ParsedDatabaseStatement getParsedDatabaseStatement(final DatabaseVendor d
                 return UNPARSEABLE_STATEMENT;
             }
 
-            return getOrCreateCache().get(
-                    statement,
-                    s -> databaseStatementParser.getParsedDatabaseStatement(databaseVendor, statement, resultSetMetaData));
+            Cache<String, ParsedDatabaseStatement> cache = getOrCreateCache();
+            ParsedDatabaseStatement parsedStatement = cache.getIfPresent(statement);
+            if (parsedStatement == null) {
+                parsedStatement = databaseStatementParser.getParsedDatabaseStatement(databaseVendor, statement, resultSetMetaData);
+                cache.put(statement, parsedStatement);
+            }
+
+            return parsedStatement;
         } catch (RuntimeException ex) {
             toLog = ex;
         }

newrelic-agent/src/test/java/com/newrelic/agent/database/CachingDatabaseStatementParserTest.java

@@ -5,12 +5,8 @@
 
 import java.sql.ResultSetMetaData;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -24,8 +20,15 @@ public void getParsedDatabaseStatement_cachesSqlStatement() {
         DatabaseStatementParser mockParser = mock(DatabaseStatementParser.class);
         CachingDatabaseStatementParser cache = new CachingDatabaseStatementParser(mockParser);
 
+        when(mockParser.getParsedDatabaseStatement(any(), eq("sql"), any())).thenReturn(new ParsedDatabaseStatement("tablename", "select", true));
+
+        // Initial call will cache the result
         ParsedDatabaseStatement statement = cache.getParsedDatabaseStatement(mock(DatabaseVendor.class), "sql", mock(ResultSetMetaData.class));
         verify(mockParser, times(1)).getParsedDatabaseStatement(any(), eq("sql"), any());
+
+        // Subsequent calls will retrieve the value from the cache and not call the databaseStatementParser again
+        assertEquals(statement, cache.getParsedDatabaseStatement(mock(DatabaseVendor.class), "sql", mock(ResultSetMetaData.class)));
+        verify(mockParser, times(1)).getParsedDatabaseStatement(any(), eq("sql"), any());
     }
 
     @Test
@@ -34,4 +37,4 @@ public void getParsedDatabaseStatement_withNullStatement_returnsUnparseableState
         CachingDatabaseStatementParser cache = new CachingDatabaseStatementParser(mockParser);
         assertEquals(DatabaseStatementParser.UNPARSEABLE_STATEMENT, cache.getParsedDatabaseStatement(mock(DatabaseVendor.class), null, null));
     }
-}
+}