Skip to content

Commit 4a190b9

Browse files
mridul111998claude
andauthored
perf(ebean-dao): move SharedSchemaCache pre-warm to constructor (#618)
PR #615 introduced SharedSchemaCache with pre-warm calls inside EbeanLocalAccess.ensureSchemaUpToDate(). In deploys where schema migrations run as a separate job (LinkedIn's prod pattern, and likely others), ensureSchemaUpToDate() is gated off, so pre-warm never runs and the first request after JVM startup pays the inline information_schema query cost (the "Inline schema cache miss" log line). Moves the pre-warm calls to the EbeanLocalAccess constructor so they always run, regardless of whether schema evolution is enabled. refreshTable() already has its own try/catch, so a transient DB hiccup during construction logs a warning rather than failing boot. Also drops the unused tableColumns field (dead code, replaced by the shared cache in #615). Two supporting changes: 1. SharedSchemaCache.clearRegistry() now also calls clearCaches() on each held instance before wiping the REGISTRY map. Previously clearing only the static map left stale entries readable through any reference (e.g. EbeanLocalAccess.validator) that callers were already holding -- a footgun that the @VisibleForTesting name did not suggest. 2. EbeanLocalDAOTest.addIndex() helper now calls clearRegistry() after its dynamic ALTER TABLE ADD COLUMN. With pre-warm now running in the constructor, the cache snapshots the schema before addIndex modifies it; the explicit invalidation keeps subsequent columnExists()/indexExists() lookups in sync with the actual table state. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 56780a5 commit 4a190b9

3 files changed

Lines changed: 39 additions & 20 deletions

File tree

dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import java.util.List;
4343
import java.util.Map;
4444
import java.util.Set;
45-
import java.util.concurrent.ConcurrentHashMap;
4645
import java.util.stream.Collectors;
4746
import javax.annotation.Nonnull;
4847
import javax.annotation.Nullable;
@@ -83,9 +82,6 @@ public class EbeanLocalAccess<URN extends Urn> implements IEbeanLocalAccess<URN>
8382
private static final String DEFAULT_ACTOR = "urn:li:principal:UNKNOWN";
8483
private static final String EBEAN_SERVER_CONFIG = "EbeanServerConfig";
8584

86-
// key: table_name,
87-
// value: Set(column1, column2, column3 ...)
88-
private final Map<String, Set<String>> tableColumns = new ConcurrentHashMap<>();
8985
private final SchemaValidatorUtil validator;
9086

9187
public EbeanLocalAccess(EbeanServer server, ServerConfig serverConfig, @Nonnull Class<URN> urnClass,
@@ -97,6 +93,13 @@ public EbeanLocalAccess(EbeanServer server, ServerConfig serverConfig, @Nonnull
9793
_schemaEvolutionManager = createSchemaEvolutionManager(serverConfig);
9894
_nonDollarVirtualColumnsEnabled = nonDollarVirtualColumnsEnabled;
9995
validator = buildValidator(server, serverConfig);
96+
// Pre-warm the shared cache for both the prod and test tables so the first request after
97+
// JVM start never pays the inline information_schema query cost. Done here rather than in
98+
// ensureSchemaUpToDate() because consumers that run schema migrations as a separate job
99+
// (e.g., LinkedIn's prod deploy pattern) gate ensureSchemaUpToDate() off, so a pre-warm
100+
// there would never run and the cold-start cache miss would still hit the request path.
101+
validator.registerAndPreWarm(getTableName(_entityType));
102+
validator.registerAndPreWarm(getTestTableName(_entityType));
100103
}
101104

102105
private static SchemaValidatorUtil buildValidator(EbeanServer server, ServerConfig serverConfig) {
@@ -135,9 +138,7 @@ public void configureOptionalForceIndex(@Nullable String indexName,
135138

136139
public void ensureSchemaUpToDate() {
137140
_schemaEvolutionManager.ensureSchemaUpToDate();
138-
// Pre-warm the shared cache for both the prod and test tables so the first request is never slow.
139-
validator.registerAndPreWarm(getTableName(_entityType));
140-
validator.registerAndPreWarm(getTestTableName(_entityType));
141+
// Cache pre-warm moved to constructor — see comment there for rationale.
141142
validateForceIndex();
142143
}
143144

@@ -254,7 +255,7 @@ public <ASPECT_UNION extends RecordTemplate> int create(
254255
String onDuplicateKeyClause = buildOnDuplicateKeyForCreate(urn, aspectCreateLambdas);
255256

256257
// Use comprehensive helper to prepare SqlUpdate with all common logic
257-
SqlUpdate sqlUpdate = prepareMultiColumnInsert(urn, aspectValues, aspectCreateLambdas,
258+
SqlUpdate sqlUpdate = prepareMultiColumnInsert(urn, aspectValues, aspectCreateLambdas,
258259
auditStamp, ingestionTrackingContext, onDuplicateKeyClause);
259260

260261
return sqlUpdate.execute();
@@ -282,7 +283,7 @@ public <ASPECT_UNION extends RecordTemplate> int batchUpsert(
282283
// Extract parallel lists from contexts for prepareMultiColumnInsert
283284
List<RecordTemplate> aspectValues = new ArrayList<>();
284285
List<BaseLocalDAO.AspectUpdateLambda<? extends RecordTemplate>> aspectUpdateLambdas = new ArrayList<>();
285-
286+
286287
for (BaseLocalDAO.AspectUpdateContext<RecordTemplate> ctx : updateContexts) {
287288
aspectValues.add(ctx.getNewValue());
288289
aspectUpdateLambdas.add(ctx.getLambda());
@@ -292,7 +293,7 @@ public <ASPECT_UNION extends RecordTemplate> int batchUpsert(
292293
String onDuplicateKeyClause = buildOnDuplicateKeyForUpsert(urn, aspectUpdateLambdas);
293294

294295
// Use comprehensive helper to prepare SqlUpdate with all common logic
295-
SqlUpdate sqlUpdate = prepareMultiColumnInsert(urn, aspectValues, aspectUpdateLambdas,
296+
SqlUpdate sqlUpdate = prepareMultiColumnInsert(urn, aspectValues, aspectUpdateLambdas,
296297
auditStamp, ingestionTrackingContext, onDuplicateKeyClause);
297298

298299
return sqlUpdate.execute();
@@ -889,21 +890,21 @@ private SqlUpdate prepareMultiColumnInsert(
889890
@Nonnull AuditStamp auditStamp,
890891
@Nullable IngestionTrackingContext ingestionTrackingContext,
891892
@Nonnull String onDuplicateKeyClause) {
892-
893+
893894
// Validate that aspectValues and aspectLambdas have the same size
894895
if (aspectValues.size() != aspectLambdas.size()) {
895896
throw new IllegalArgumentException(
896897
String.format("Aspect values size (%d) must match aspect lambdas size (%d)",
897898
aspectValues.size(), aspectLambdas.size()));
898899
}
899-
900+
900901
// Validate that no aspect values are null
901902
aspectValues.forEach(aspectValue -> {
902903
if (aspectValue == null) {
903904
throw new IllegalArgumentException("Aspect value cannot be null");
904905
}
905906
});
906-
907+
907908
// Extract audit information
908909
final long timestamp = auditStamp.hasTime() ? auditStamp.getTime() : System.currentTimeMillis();
909910
final String actor = auditStamp.hasActor() ? auditStamp.getActor().toString() : DEFAULT_ACTOR;
@@ -969,7 +970,7 @@ private SqlUpdate prepareMultiColumnInsert(
969970
if (urnExtraction) {
970971
sqlUpdate.setParameter("a_urn", toJsonString(urn));
971972
}
972-
973+
973974
// Set common parameters
974975
sqlUpdate.setParameter("urn", urn.toString())
975976
.setParameter("lastmodifiedon", utcTimestamp)
@@ -987,13 +988,13 @@ private SqlUpdate prepareMultiColumnInsert(
987988
* @return the ON DUPLICATE KEY UPDATE clause string
988989
*/
989990
private String buildOnDuplicateKeyForCreate(
990-
@Nonnull URN urn,
991+
@Nonnull URN urn,
991992
@Nonnull List<? extends BaseLocalDAO.AspectUpdateLambda<? extends RecordTemplate>> aspectLambdas) {
992-
993+
993994
List<String> classNames = aspectLambdas.stream()
994995
.map(lambda -> lambda.getAspectClass().getCanonicalName())
995996
.collect(Collectors.toList());
996-
997+
997998
StringBuilder onDuplicateKey = new StringBuilder();
998999
onDuplicateKey.append(ON_DUPLICATE_KEY_UPDATE);
9991000
for (int i = 0; i < classNames.size(); i++) {
@@ -1016,13 +1017,13 @@ private String buildOnDuplicateKeyForCreate(
10161017
* @return the ON DUPLICATE KEY UPDATE clause string
10171018
*/
10181019
private String buildOnDuplicateKeyForUpsert(
1019-
@Nonnull URN urn,
1020+
@Nonnull URN urn,
10201021
@Nonnull List<? extends BaseLocalDAO.AspectUpdateLambda<? extends RecordTemplate>> aspectLambdas) {
1021-
1022+
10221023
List<String> classNames = aspectLambdas.stream()
10231024
.map(lambda -> lambda.getAspectClass().getCanonicalName())
10241025
.collect(Collectors.toList());
1025-
1026+
10261027
StringBuilder onDuplicateKey = new StringBuilder();
10271028
onDuplicateKey.append(ON_DUPLICATE_KEY_UPDATE);
10281029
for (int i = 0; i < classNames.size(); i++) {

dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SharedSchemaCache.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,13 @@ void clearCaches() {
210210

211211
@VisibleForTesting
212212
public static void clearRegistry() {
213+
// Invalidate the in-memory caches on every held instance before dropping references from
214+
// the registry. Callers that already hold a reference to a SharedSchemaCache (e.g. via
215+
// EbeanLocalAccess.validator) will then see the cache miss on their next read instead of
216+
// a stale entry.
217+
for (SharedSchemaCache cache : REGISTRY.values()) {
218+
cache.clearCaches();
219+
}
213220
REGISTRY.clear();
214221
}
215222
}

dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4133,9 +4133,11 @@ private void addIndex(Urn urn, String aspectName, String pathName, Object val) {
41334133
String checkColumnExistance = String.format("SELECT * FROM information_schema.COLUMNS WHERE TABLE_SCHEMA = '%s' AND"
41344134
+ " TABLE_NAME = '%s' AND COLUMN_NAME = '%s'", getDatabaseName(), getTableName(urn), fullIndexColumnName);
41354135

4136+
boolean schemaChanged = false;
41364137
if (_server.createSqlQuery(checkColumnExistance).findList().isEmpty()) {
41374138
String sqlUpdate = String.format("ALTER TABLE %s ADD COLUMN %s VARCHAR(255);", getTableName(urn), fullIndexColumnName);
41384139
_server.execute(Ebean.createSqlUpdate(sqlUpdate));
4140+
schemaChanged = true;
41394141
}
41404142

41414143
checkColumnExistance = String.format("SELECT * FROM information_schema.COLUMNS WHERE TABLE_SCHEMA = '%s' AND"
@@ -4144,6 +4146,15 @@ private void addIndex(Urn urn, String aspectName, String pathName, Object val) {
41444146
if (aspectColumnName != null && _server.createSqlQuery(checkColumnExistance).findList().isEmpty()) {
41454147
String sqlUpdate = String.format("ALTER TABLE %s ADD COLUMN %s VARCHAR(255);", getTableName(urn), aspectColumnName);
41464148
_server.execute(Ebean.createSqlUpdate(sqlUpdate));
4149+
schemaChanged = true;
4150+
}
4151+
// EbeanLocalAccess pre-warms the SharedSchemaCache in its constructor (capturing the schema
4152+
// at DAO construction time). The ALTER TABLE above adds a new virtual/aspect column AFTER
4153+
// construction, so the held cache is now stale. Clear the registry to invalidate every
4154+
// held instance's caches; subsequent columnExists()/indexExists() lookups then re-query
4155+
// information_schema and see the new column.
4156+
if (schemaChanged) {
4157+
SharedSchemaCache.clearRegistry();
41474158
}
41484159

41494160
// finally, we need to update the newly added column with the passed-in value.

0 commit comments

Comments
 (0)