From 552381d126ca30055f67b71d7ed4a60608a0bc8a Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Wed, 1 Apr 2026 01:29:29 -0700 Subject: [PATCH 01/15] Reduce redundant HDFS refreshes during table commits Reuse request-scoped metadata and toggle lookups so repeated refreshes in a single request avoid extra HDFS reads and duplicate HTS calls without weakening commit concurrency checks. Made-with: Cursor --- .../catalog/OpenHouseInternalCatalog.java | 7 +- .../OpenHouseInternalTableOperations.java | 21 +- .../OpenHouseInternalTableOperationsTest.java | 184 +++++++++++++++++- .../common/cache/RequestScopedCache.java | 60 ++++++ .../ToggleStatusesRepositoryImpl.java | 27 ++- .../RepositoryTestWithSettableComponents.java | 25 ++- .../ToggleStatusesRepositoryImplTest.java | 117 +++++++++++ 7 files changed, 422 insertions(+), 19 deletions(-) create mode 100644 services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java create mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java index d1fae0ad5..df7a742b0 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java @@ -7,6 +7,7 @@ import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.cluster.storage.selector.StorageSelector; import com.linkedin.openhouse.common.api.spec.TableUri; +import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.common.exception.AlreadyExistsException; import com.linkedin.openhouse.common.exception.NoSuchSoftDeletedUserTableException; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; @@ -63,6 +64,9 @@ public class OpenHouseInternalCatalog extends BaseMetastoreCatalog { @Autowired MeterRegistry meterRegistry; + @Autowired(required = false) + RequestScopedCache requestScopedCache = new RequestScopedCache(); + @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { FileIO fileIO = resolveFileIO(tableIdentifier); @@ -74,7 +78,8 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) { houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); } @Override diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java index 13791048c..656bec9ce 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java @@ -12,6 +12,7 @@ import com.linkedin.openhouse.cluster.storage.StorageClient; import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorageClient; import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; +import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.internal.catalog.exception.InvalidIcebergSnapshotException; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; @@ -83,7 +84,11 @@ public class OpenHouseInternalTableOperations extends BaseMetastoreTableOperatio FileIOManager fileIOManager; + RequestScopedCache requestScopedCache; + private static final Gson GSON = new Gson(); + private static final String TABLE_METADATA_CACHE_NAMESPACE = + OpenHouseInternalTableOperations.class.getName() + ".tableMetadata"; private static final Cache CACHE = CacheBuilder.newBuilder().expireAfterWrite(5, TimeUnit.MINUTES).maximumSize(1000).build(); @@ -131,7 +136,10 @@ protected void doRefresh() { protected void refreshMetadata(final String metadataLoc) { long startTime = System.currentTimeMillis(); boolean needToReload = !Objects.equal(currentMetadataLocation(), metadataLoc); - Runnable r = () -> super.refreshFromMetadataLocation(metadataLoc); + Runnable r = + () -> + super.refreshFromMetadataLocation( + metadataLoc, null, 20, this::loadTableMetadataWithRequestCache); try { if (needToReload) { metricsReporter.executeWithStats( @@ -337,6 +345,8 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { updatedMtDataRef, io().newOutputFile(newMetadataLocation)), InternalCatalogMetricsConstant.METADATA_UPDATE_LATENCY, getCatalogMetricTags()); + requestScopedCache.put( + TABLE_METADATA_CACHE_NAMESPACE, newMetadataLocation, updatedMtDataRef); log.info( "updateMetadata to location {} succeeded, took {} ms", newMetadataLocation, @@ -387,7 +397,7 @@ updatedMtDataRef, io().newOutputFile(newMetadataLocation)), * "forced refresh" in {@link OpenHouseInternalTableOperations#commit(TableMetadata, * TableMetadata)} */ - refreshFromMetadataLocation(newMetadataLocation); + refreshMetadata(newMetadataLocation); } if (isReplicatedTableCreate(properties)) { updateMetadataFieldForTable(metadata, newMetadataLocation); @@ -768,4 +778,11 @@ private List getIntermediateSchemasFromProps(TableMetadata metadata) { .create() .fromJson(serializedNewIntermediateSchemas, new TypeToken>() {}.getType()); } + + private TableMetadata loadTableMetadataWithRequestCache(String metadataLocation) { + return requestScopedCache.getOrLoad( + TABLE_METADATA_CACHE_NAMESPACE, + metadataLocation, + () -> TableMetadataParser.read(io(), metadataLocation)); + } } diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java index cad25eb12..2553bcb15 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java @@ -8,6 +8,7 @@ import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.cluster.storage.local.LocalStorage; import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; +import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTable; @@ -34,6 +35,7 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.SneakyThrows; @@ -72,6 +74,8 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; public class OpenHouseInternalTableOperationsTest { private static final String TEST_LOCATION = "test_location"; @@ -96,6 +100,7 @@ public class OpenHouseInternalTableOperationsTest { @Mock private FSDataInputStream mockFSDataInputStream; @Mock private FSDataOutputStream mockFSDataOutputStream; + private RequestScopedCache requestScopedCache; private OpenHouseInternalTableOperations openHouseInternalTableOperations; private OpenHouseInternalTableOperations openHouseInternalTableOperationsWithMockMetrics; @@ -107,6 +112,7 @@ private static String getTempLocation() { @BeforeEach void setup() { MockitoAnnotations.openMocks(this); + requestScopedCache = new RequestScopedCache(); Mockito.when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any())) .thenReturn(mockHouseTable); HadoopFileIO fileIO = new HadoopFileIO(new Configuration()); @@ -119,7 +125,8 @@ void setup() { mockHouseTableMapper, TEST_TABLE_IDENTIFIER, metricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); // Create a separate instance with mock metrics reporter for testing metrics openHouseInternalTableOperationsWithMockMetrics = @@ -129,7 +136,8 @@ void setup() { mockHouseTableMapper, TEST_TABLE_IDENTIFIER, mockMetricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); LocalStorage localStorage = mock(LocalStorage.class); when(fileIOManager.getStorage(fileIO)).thenReturn(localStorage); @@ -1068,6 +1076,124 @@ void testCommitMetadataUpdateLatencyHasHistogramBuckets() { this::executeCommitMetadata); } + @Test + void testRefreshReusesCachedMetadataWithinSingleRequest() { + RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + try { + HouseTablePrimaryKey primaryKey = + HouseTablePrimaryKey.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .build(); + when(mockHouseTableRepository.findById(primaryKey)).thenReturn(Optional.of(mockHouseTable)); + when(mockHouseTable.getTableLocation()).thenReturn("test_metadata_location"); + + OpenHouseInternalTableOperations secondOperations = + new OpenHouseInternalTableOperations( + mockHouseTableRepository, + new HadoopFileIO(new Configuration()), + mockHouseTableMapper, + TEST_TABLE_IDENTIFIER, + new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), + fileIOManager, + requestScopedCache); + + try (MockedStatic parserMock = + Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { + parserMock + .when( + () -> + TableMetadataParser.read( + Mockito.any(FileIO.class), Mockito.eq("test_metadata_location"))) + .thenReturn(BASE_TABLE_METADATA); + + openHouseInternalTableOperations.refresh(); + secondOperations.refresh(); + + parserMock.verify( + () -> + TableMetadataParser.read( + Mockito.any(FileIO.class), Mockito.eq("test_metadata_location")), + times(1)); + } + } finally { + RequestContextHolder.resetRequestAttributes(); + } + } + + @Test + void testCommitSeedsRequestCacheForSubsequentRefresh() { + RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + try { + AtomicReference savedHouseTable = new AtomicReference<>(); + HouseTablePrimaryKey primaryKey = + HouseTablePrimaryKey.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .build(); + when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any())) + .thenAnswer( + invocation -> { + TableMetadata tableMetadata = invocation.getArgument(0); + HouseTable mappedHouseTable = + HouseTable.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .tableLocation( + tableMetadata.properties().get(getCanonicalFieldName("tableLocation"))) + .build(); + savedHouseTable.set(mappedHouseTable); + return mappedHouseTable; + }); + when(mockHouseTableRepository.save(Mockito.any(HouseTable.class))) + .thenAnswer( + invocation -> { + HouseTable houseTable = invocation.getArgument(0); + savedHouseTable.set(houseTable); + return houseTable; + }); + when(mockHouseTableRepository.findById(primaryKey)) + .thenAnswer(invocation -> Optional.ofNullable(savedHouseTable.get())); + + OpenHouseInternalTableOperations refreshedOperations = + new OpenHouseInternalTableOperations( + mockHouseTableRepository, + new HadoopFileIO(new Configuration()), + mockHouseTableMapper, + TEST_TABLE_IDENTIFIER, + new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), + fileIOManager, + requestScopedCache); + + Map properties = new HashMap<>(BASE_TABLE_METADATA.properties()); + properties.put(getCanonicalFieldName("tableLocation"), TEST_LOCATION); + TableMetadata metadata = BASE_TABLE_METADATA.replaceProperties(properties); + + try (MockedStatic parserMock = + Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { + parserMock + .when( + () -> + TableMetadataParser.write( + Mockito.any(TableMetadata.class), + Mockito.any(org.apache.iceberg.io.OutputFile.class))) + .thenAnswer(invocation -> null); + + openHouseInternalTableOperations.doCommit(BASE_TABLE_METADATA, metadata); + refreshedOperations.refresh(); + + String committedLocation = savedHouseTable.get().getTableLocation(); + Assertions.assertEquals(committedLocation, refreshedOperations.currentMetadataLocation()); + parserMock.verify( + () -> + TableMetadataParser.read(Mockito.any(FileIO.class), Mockito.eq(committedLocation)), + never()); + } + } finally { + RequestContextHolder.resetRequestAttributes(); + } + } + /** * Common test method for verifying metrics exclude both database and table tags. * @@ -1095,7 +1221,8 @@ private void testMetricExcludesDatabaseTag( mockHouseTableMapper, TEST_TABLE_IDENTIFIER, realMetricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); // Setup test-specific mocks setupFunction.accept(operationsWithRealMetrics); @@ -1157,7 +1284,8 @@ private void testMetricHasHistogramBuckets( mockHouseTableMapper, TEST_TABLE_IDENTIFIER, realMetricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); // Setup test-specific mocks setupFunction.accept(operationsWithRealMetrics); @@ -1861,4 +1989,52 @@ void testDoCommitCreatesOtelSpans() { tracerProvider.close(); } } + + private static final class MapBackedRequestAttributes implements RequestAttributes { + private final Map requestScope = new HashMap<>(); + + @Override + public Object getAttribute(String name, int scope) { + return scope == RequestAttributes.SCOPE_REQUEST ? requestScope.get(name) : null; + } + + @Override + public void setAttribute(String name, Object value, int scope) { + if (scope == RequestAttributes.SCOPE_REQUEST) { + requestScope.put(name, value); + } + } + + @Override + public void removeAttribute(String name, int scope) { + if (scope == RequestAttributes.SCOPE_REQUEST) { + requestScope.remove(name); + } + } + + @Override + public String[] getAttributeNames(int scope) { + return scope == RequestAttributes.SCOPE_REQUEST + ? requestScope.keySet().toArray(new String[0]) + : new String[0]; + } + + @Override + public void registerDestructionCallback(String name, Runnable callback, int scope) {} + + @Override + public Object resolveReference(String key) { + return null; + } + + @Override + public String getSessionId() { + return "test-session"; + } + + @Override + public Object getSessionMutex() { + return this; + } + } } diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java new file mode 100644 index 000000000..563387131 --- /dev/null +++ b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java @@ -0,0 +1,60 @@ +package com.linkedin.openhouse.common.cache; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; +import org.springframework.stereotype.Component; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; + +/** + * Small request-scoped cache backed by {@link RequestAttributes}. When there is no active request, + * cache lookups fall back to direct loading and writes are ignored. + */ +@Component +public class RequestScopedCache { + + private static final String ATTRIBUTE_NAME = RequestScopedCache.class.getName() + ".CACHE"; + + public T getOrLoad(String namespace, Object key, Supplier loader) { + RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); + if (requestAttributes == null) { + return loader.get(); + } + + return cast( + namespaceCache(requestAttributes, namespace).computeIfAbsent(key, unused -> loader.get())); + } + + public void put(String namespace, Object key, Object value) { + RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); + if (requestAttributes == null) { + return; + } + + namespaceCache(requestAttributes, namespace).put(key, value); + } + + @SuppressWarnings("unchecked") + private Map> requestCache(RequestAttributes requestAttributes) { + Map> cache = + (Map>) + requestAttributes.getAttribute(ATTRIBUTE_NAME, RequestAttributes.SCOPE_REQUEST); + if (cache == null) { + cache = new ConcurrentHashMap<>(); + requestAttributes.setAttribute(ATTRIBUTE_NAME, cache, RequestAttributes.SCOPE_REQUEST); + } + return cache; + } + + private Map namespaceCache( + RequestAttributes requestAttributes, String namespace) { + return requestCache(requestAttributes) + .computeIfAbsent(namespace, unused -> new ConcurrentHashMap<>()); + } + + @SuppressWarnings("unchecked") + private static T cast(Object value) { + return (T) value; + } +} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java index cf639477e..5d8ae3229 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.toggle.repository; +import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.housetables.client.api.ToggleStatusApi; import com.linkedin.openhouse.housetables.client.model.EntityResponseBodyToggleStatus; import com.linkedin.openhouse.tables.toggle.ToggleStatusMapper; @@ -17,20 +18,30 @@ @Repository @Slf4j public class ToggleStatusesRepositoryImpl implements ToggleStatusesRepository { + private static final String TOGGLE_STATUS_CACHE_NAMESPACE = + ToggleStatusesRepositoryImpl.class.getName() + ".toggleStatus"; + @Autowired private ToggleStatusApi apiInstance; @Autowired private ToggleStatusMapper toggleStatusMapper; + @Autowired(required = false) + private RequestScopedCache requestScopedCache = new RequestScopedCache(); + @Override public Optional findById(ToggleStatusKey toggleStatusKey) { - return apiInstance - .getTableToggleStatus( - toggleStatusKey.getDatabaseId(), - toggleStatusKey.getTableId(), - toggleStatusKey.getFeatureId()) - .map(EntityResponseBodyToggleStatus::getEntity) - .map(s -> toggleStatusMapper.toTableToggleStatus(toggleStatusKey, s)) - .blockOptional(); + return requestScopedCache.getOrLoad( + TOGGLE_STATUS_CACHE_NAMESPACE, + toggleStatusKey, + () -> + apiInstance + .getTableToggleStatus( + toggleStatusKey.getDatabaseId(), + toggleStatusKey.getTableId(), + toggleStatusKey.getFeatureId()) + .map(EntityResponseBodyToggleStatus::getEntity) + .map(s -> toggleStatusMapper.toTableToggleStatus(toggleStatusKey, s)) + .blockOptional()); } @Override diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java index 1ce1cae1d..7438ec3aa 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java @@ -6,6 +6,7 @@ import com.linkedin.openhouse.cluster.metrics.micrometer.MetricsReporter; import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.internal.catalog.OpenHouseInternalTableOperations; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; @@ -64,6 +65,8 @@ public class RepositoryTestWithSettableComponents { @Autowired MeterRegistry meterRegistry; + private final RequestScopedCache requestScopedCache = new RequestScopedCache(); + FileIO fileIO; @PostConstruct @@ -104,7 +107,8 @@ void testNoRetryInternalRepo() throws Exception { houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); creationDTO = openHouseInternalRepository.save(creationDTO); @@ -120,7 +124,13 @@ void testNoRetryInternalRepo() throws Exception { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter2, fileIOManager); + htsRepo, + fileIO, + houseTableMapper, + tableIdentifier, + metricsReporter2, + fileIOManager, + requestScopedCache); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy(new BaseTable(spyOperations, realTable.name())); @@ -192,7 +202,8 @@ void testSaveClearsTransientCommitPropertiesDuringTransaction() throws Exception houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager); + fileIOManager, + requestScopedCache); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); @@ -251,7 +262,13 @@ void testFailedHtsRepoWhenGet() { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter, fileIOManager); + htsRepo, + fileIO, + houseTableMapper, + tableIdentifier, + metricsReporter, + fileIOManager, + requestScopedCache); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy( diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java new file mode 100644 index 000000000..7ca78fe74 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java @@ -0,0 +1,117 @@ +package com.linkedin.openhouse.tables.toggle.repository; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.linkedin.openhouse.common.cache.RequestScopedCache; +import com.linkedin.openhouse.housetables.client.api.ToggleStatusApi; +import com.linkedin.openhouse.housetables.client.model.EntityResponseBodyToggleStatus; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; +import com.linkedin.openhouse.tables.toggle.ToggleStatusMapper; +import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; +import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; +import java.util.HashMap; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; +import reactor.core.publisher.Mono; + +@ExtendWith(MockitoExtension.class) +class ToggleStatusesRepositoryImplTest { + + @Mock private ToggleStatusApi apiInstance; + @Mock private ToggleStatusMapper toggleStatusMapper; + @Mock private EntityResponseBodyToggleStatus entityResponseBodyToggleStatus; + @Mock private ToggleStatus toggleStatus; + + @Spy private RequestScopedCache requestScopedCache = new RequestScopedCache(); + + @InjectMocks private ToggleStatusesRepositoryImpl toggleStatusesRepository; + + @Test + void testFindByIdUsesRequestCacheWithinSingleRequest() { + ToggleStatusKey toggleStatusKey = + ToggleStatusKey.builder().databaseId("db").tableId("table").featureId("feature").build(); + TableToggleStatus expected = + TableToggleStatus.builder() + .databaseId("db") + .tableId("table") + .featureId("feature") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) + .build(); + + when(apiInstance.getTableToggleStatus("db", "table", "feature")) + .thenReturn(Mono.just(entityResponseBodyToggleStatus)); + when(entityResponseBodyToggleStatus.getEntity()).thenReturn(toggleStatus); + when(toggleStatusMapper.toTableToggleStatus(toggleStatusKey, toggleStatus)) + .thenReturn(expected); + + RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + try { + TableToggleStatus first = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); + TableToggleStatus second = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); + + assertSame(expected, first); + assertSame(expected, second); + } finally { + RequestContextHolder.resetRequestAttributes(); + } + + verify(apiInstance, times(1)).getTableToggleStatus("db", "table", "feature"); + } + + private static final class MapBackedRequestAttributes implements RequestAttributes { + private final HashMap requestScope = new HashMap<>(); + + @Override + public Object getAttribute(String name, int scope) { + return scope == RequestAttributes.SCOPE_REQUEST ? requestScope.get(name) : null; + } + + @Override + public void setAttribute(String name, Object value, int scope) { + if (scope == RequestAttributes.SCOPE_REQUEST) { + requestScope.put(name, value); + } + } + + @Override + public void removeAttribute(String name, int scope) { + if (scope == RequestAttributes.SCOPE_REQUEST) { + requestScope.remove(name); + } + } + + @Override + public String[] getAttributeNames(int scope) { + return scope == RequestAttributes.SCOPE_REQUEST + ? requestScope.keySet().toArray(new String[0]) + : new String[0]; + } + + @Override + public void registerDestructionCallback(String name, Runnable callback, int scope) {} + + @Override + public Object resolveReference(String key) { + return null; + } + + @Override + public String getSessionId() { + return "test-session"; + } + + @Override + public Object getSessionMutex() { + return this; + } + } +} From 9b92ce1160b92e2a267d2cf03e7a167b6678129a Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Wed, 1 Apr 2026 02:02:01 -0700 Subject: [PATCH 02/15] Restore required request cache injection Register the request cache from internalcatalog configuration so table service contexts can inject it consistently without optional autowiring fallbacks. Made-with: Cursor --- .../internal/catalog/InternalCatalogConfig.java | 14 ++++++++++++++ .../internal/catalog/OpenHouseInternalCatalog.java | 3 +-- .../openhouse/common/cache/RequestScopedCache.java | 2 -- .../repository/ToggleStatusesRepositoryImpl.java | 3 +-- .../h2/RepositoryTestWithSettableComponents.java | 2 +- 5 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java new file mode 100644 index 000000000..81be601ea --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java @@ -0,0 +1,14 @@ +package com.linkedin.openhouse.internal.catalog; + +import com.linkedin.openhouse.common.cache.RequestScopedCache; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class InternalCatalogConfig { + + @Bean + public RequestScopedCache provideRequestScopedCache() { + return new RequestScopedCache(); + } +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java index df7a742b0..e75791eb7 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java @@ -64,8 +64,7 @@ public class OpenHouseInternalCatalog extends BaseMetastoreCatalog { @Autowired MeterRegistry meterRegistry; - @Autowired(required = false) - RequestScopedCache requestScopedCache = new RequestScopedCache(); + @Autowired RequestScopedCache requestScopedCache; @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java index 563387131..95e049f57 100644 --- a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java +++ b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java @@ -3,7 +3,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; -import org.springframework.stereotype.Component; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.RequestContextHolder; @@ -11,7 +10,6 @@ * Small request-scoped cache backed by {@link RequestAttributes}. When there is no active request, * cache lookups fall back to direct loading and writes are ignored. */ -@Component public class RequestScopedCache { private static final String ATTRIBUTE_NAME = RequestScopedCache.class.getName() + ".CACHE"; diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java index 5d8ae3229..cbc6548ff 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java @@ -25,8 +25,7 @@ public class ToggleStatusesRepositoryImpl implements ToggleStatusesRepository { @Autowired private ToggleStatusMapper toggleStatusMapper; - @Autowired(required = false) - private RequestScopedCache requestScopedCache = new RequestScopedCache(); + @Autowired private RequestScopedCache requestScopedCache; @Override public Optional findById(ToggleStatusKey toggleStatusKey) { diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java index 7438ec3aa..803b064a2 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java @@ -65,7 +65,7 @@ public class RepositoryTestWithSettableComponents { @Autowired MeterRegistry meterRegistry; - private final RequestScopedCache requestScopedCache = new RequestScopedCache(); + @Autowired RequestScopedCache requestScopedCache; FileIO fileIO; From a7c4ab9831ad5a749fa014785b974d6d7fa6c6f5 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Thu, 2 Apr 2026 22:18:01 -0700 Subject: [PATCH 03/15] Document request cache wiring and simplify request-scoped tests Clarify why RequestScopedCache is provided from internal catalog config and replace bespoke RequestAttributes test helpers with Spring's servlet-backed request attributes. Made-with: Cursor --- .../catalog/InternalCatalogConfig.java | 8 +++ .../OpenHouseInternalTableOperationsTest.java | 57 ++----------------- .../common/cache/RequestScopedCache.java | 6 ++ .../ToggleStatusesRepositoryImplTest.java | 55 ++---------------- 4 files changed, 24 insertions(+), 102 deletions(-) diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java index 81be601ea..11750db3c 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java @@ -4,6 +4,14 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +/** + * Registers the default {@link RequestScopedCache} bean from the {@code internal.catalog} package + * because OpenHouse applications explicitly scan this package but do not consistently scan {@code + * com.linkedin.openhouse.common.cache}. Keeping bean registration here avoids widening every + * application/test scan list while preserving the cache implementation as a shared utility in + * {@code services/common}. This default can still be replaced by another bean, for example via + * {@code @Primary} in a downstream repo. + */ @Configuration public class InternalCatalogConfig { diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java index 2553bcb15..19ba2908d 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java @@ -74,8 +74,9 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.springframework.web.context.request.RequestAttributes; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; public class OpenHouseInternalTableOperationsTest { private static final String TEST_LOCATION = "test_location"; @@ -1078,7 +1079,8 @@ void testCommitMetadataUpdateLatencyHasHistogramBuckets() { @Test void testRefreshReusesCachedMetadataWithinSingleRequest() { - RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + RequestContextHolder.setRequestAttributes( + new ServletRequestAttributes(new MockHttpServletRequest())); try { HouseTablePrimaryKey primaryKey = HouseTablePrimaryKey.builder() @@ -1123,7 +1125,8 @@ void testRefreshReusesCachedMetadataWithinSingleRequest() { @Test void testCommitSeedsRequestCacheForSubsequentRefresh() { - RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + RequestContextHolder.setRequestAttributes( + new ServletRequestAttributes(new MockHttpServletRequest())); try { AtomicReference savedHouseTable = new AtomicReference<>(); HouseTablePrimaryKey primaryKey = @@ -1989,52 +1992,4 @@ void testDoCommitCreatesOtelSpans() { tracerProvider.close(); } } - - private static final class MapBackedRequestAttributes implements RequestAttributes { - private final Map requestScope = new HashMap<>(); - - @Override - public Object getAttribute(String name, int scope) { - return scope == RequestAttributes.SCOPE_REQUEST ? requestScope.get(name) : null; - } - - @Override - public void setAttribute(String name, Object value, int scope) { - if (scope == RequestAttributes.SCOPE_REQUEST) { - requestScope.put(name, value); - } - } - - @Override - public void removeAttribute(String name, int scope) { - if (scope == RequestAttributes.SCOPE_REQUEST) { - requestScope.remove(name); - } - } - - @Override - public String[] getAttributeNames(int scope) { - return scope == RequestAttributes.SCOPE_REQUEST - ? requestScope.keySet().toArray(new String[0]) - : new String[0]; - } - - @Override - public void registerDestructionCallback(String name, Runnable callback, int scope) {} - - @Override - public Object resolveReference(String key) { - return null; - } - - @Override - public String getSessionId() { - return "test-session"; - } - - @Override - public Object getSessionMutex() { - return this; - } - } } diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java index 95e049f57..56acfb962 100644 --- a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java +++ b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java @@ -9,6 +9,12 @@ /** * Small request-scoped cache backed by {@link RequestAttributes}. When there is no active request, * cache lookups fall back to direct loading and writes are ignored. + * + *

This class intentionally lives as a plain shared utility instead of a scanned Spring + * component. OpenHouse applications use explicit package scan allowlists, and this package is not + * included consistently across all application and test contexts. The default bean is therefore + * provided from a scanned configuration class, which keeps the utility in {@code services/common} + * while still allowing downstream repos to override the bean if needed. */ public class RequestScopedCache { diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java index 7ca78fe74..4d8abc968 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java @@ -12,15 +12,15 @@ import com.linkedin.openhouse.tables.toggle.ToggleStatusMapper; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; -import java.util.HashMap; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.web.context.request.RequestAttributes; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; import reactor.core.publisher.Mono; @ExtendWith(MockitoExtension.class) @@ -53,7 +53,8 @@ void testFindByIdUsesRequestCacheWithinSingleRequest() { when(toggleStatusMapper.toTableToggleStatus(toggleStatusKey, toggleStatus)) .thenReturn(expected); - RequestContextHolder.setRequestAttributes(new MapBackedRequestAttributes()); + RequestContextHolder.setRequestAttributes( + new ServletRequestAttributes(new MockHttpServletRequest())); try { TableToggleStatus first = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); TableToggleStatus second = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); @@ -66,52 +67,4 @@ void testFindByIdUsesRequestCacheWithinSingleRequest() { verify(apiInstance, times(1)).getTableToggleStatus("db", "table", "feature"); } - - private static final class MapBackedRequestAttributes implements RequestAttributes { - private final HashMap requestScope = new HashMap<>(); - - @Override - public Object getAttribute(String name, int scope) { - return scope == RequestAttributes.SCOPE_REQUEST ? requestScope.get(name) : null; - } - - @Override - public void setAttribute(String name, Object value, int scope) { - if (scope == RequestAttributes.SCOPE_REQUEST) { - requestScope.put(name, value); - } - } - - @Override - public void removeAttribute(String name, int scope) { - if (scope == RequestAttributes.SCOPE_REQUEST) { - requestScope.remove(name); - } - } - - @Override - public String[] getAttributeNames(int scope) { - return scope == RequestAttributes.SCOPE_REQUEST - ? requestScope.keySet().toArray(new String[0]) - : new String[0]; - } - - @Override - public void registerDestructionCallback(String name, Runnable callback, int scope) {} - - @Override - public Object resolveReference(String key) { - return null; - } - - @Override - public String getSessionId() { - return "test-session"; - } - - @Override - public Object getSessionMutex() { - return this; - } - } } From ba759a14d707a7d68349d6ba9c9f786b6ccfb76e Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Fri, 3 Apr 2026 20:15:42 -0700 Subject: [PATCH 04/15] Revert table toggle changes from services/tables/src/ --- .../ToggleStatusesRepositoryImpl.java | 26 +++---- .../RepositoryTestWithSettableComponents.java | 25 ++----- .../ToggleStatusesRepositoryImplTest.java | 70 ------------------- 3 files changed, 12 insertions(+), 109 deletions(-) delete mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java index cbc6548ff..cf639477e 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImpl.java @@ -1,6 +1,5 @@ package com.linkedin.openhouse.tables.toggle.repository; -import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.housetables.client.api.ToggleStatusApi; import com.linkedin.openhouse.housetables.client.model.EntityResponseBodyToggleStatus; import com.linkedin.openhouse.tables.toggle.ToggleStatusMapper; @@ -18,29 +17,20 @@ @Repository @Slf4j public class ToggleStatusesRepositoryImpl implements ToggleStatusesRepository { - private static final String TOGGLE_STATUS_CACHE_NAMESPACE = - ToggleStatusesRepositoryImpl.class.getName() + ".toggleStatus"; - @Autowired private ToggleStatusApi apiInstance; @Autowired private ToggleStatusMapper toggleStatusMapper; - @Autowired private RequestScopedCache requestScopedCache; - @Override public Optional findById(ToggleStatusKey toggleStatusKey) { - return requestScopedCache.getOrLoad( - TOGGLE_STATUS_CACHE_NAMESPACE, - toggleStatusKey, - () -> - apiInstance - .getTableToggleStatus( - toggleStatusKey.getDatabaseId(), - toggleStatusKey.getTableId(), - toggleStatusKey.getFeatureId()) - .map(EntityResponseBodyToggleStatus::getEntity) - .map(s -> toggleStatusMapper.toTableToggleStatus(toggleStatusKey, s)) - .blockOptional()); + return apiInstance + .getTableToggleStatus( + toggleStatusKey.getDatabaseId(), + toggleStatusKey.getTableId(), + toggleStatusKey.getFeatureId()) + .map(EntityResponseBodyToggleStatus::getEntity) + .map(s -> toggleStatusMapper.toTableToggleStatus(toggleStatusKey, s)) + .blockOptional(); } @Override diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java index 803b064a2..1ce1cae1d 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java @@ -6,7 +6,6 @@ import com.linkedin.openhouse.cluster.metrics.micrometer.MetricsReporter; import com.linkedin.openhouse.cluster.storage.StorageManager; -import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.internal.catalog.OpenHouseInternalTableOperations; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; @@ -65,8 +64,6 @@ public class RepositoryTestWithSettableComponents { @Autowired MeterRegistry meterRegistry; - @Autowired RequestScopedCache requestScopedCache; - FileIO fileIO; @PostConstruct @@ -107,8 +104,7 @@ void testNoRetryInternalRepo() throws Exception { houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager, - requestScopedCache); + fileIOManager); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); creationDTO = openHouseInternalRepository.save(creationDTO); @@ -124,13 +120,7 @@ void testNoRetryInternalRepo() throws Exception { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, - fileIO, - houseTableMapper, - tableIdentifier, - metricsReporter2, - fileIOManager, - requestScopedCache); + htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter2, fileIOManager); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy(new BaseTable(spyOperations, realTable.name())); @@ -202,8 +192,7 @@ void testSaveClearsTransientCommitPropertiesDuringTransaction() throws Exception houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager, - requestScopedCache); + fileIOManager); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); @@ -262,13 +251,7 @@ void testFailedHtsRepoWhenGet() { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, - fileIO, - houseTableMapper, - tableIdentifier, - metricsReporter, - fileIOManager, - requestScopedCache); + htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter, fileIOManager); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy( diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java deleted file mode 100644 index 4d8abc968..000000000 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/repository/ToggleStatusesRepositoryImplTest.java +++ /dev/null @@ -1,70 +0,0 @@ -package com.linkedin.openhouse.tables.toggle.repository; - -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.linkedin.openhouse.common.cache.RequestScopedCache; -import com.linkedin.openhouse.housetables.client.api.ToggleStatusApi; -import com.linkedin.openhouse.housetables.client.model.EntityResponseBodyToggleStatus; -import com.linkedin.openhouse.housetables.client.model.ToggleStatus; -import com.linkedin.openhouse.tables.toggle.ToggleStatusMapper; -import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; -import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Spy; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.web.context.request.RequestContextHolder; -import org.springframework.web.context.request.ServletRequestAttributes; -import reactor.core.publisher.Mono; - -@ExtendWith(MockitoExtension.class) -class ToggleStatusesRepositoryImplTest { - - @Mock private ToggleStatusApi apiInstance; - @Mock private ToggleStatusMapper toggleStatusMapper; - @Mock private EntityResponseBodyToggleStatus entityResponseBodyToggleStatus; - @Mock private ToggleStatus toggleStatus; - - @Spy private RequestScopedCache requestScopedCache = new RequestScopedCache(); - - @InjectMocks private ToggleStatusesRepositoryImpl toggleStatusesRepository; - - @Test - void testFindByIdUsesRequestCacheWithinSingleRequest() { - ToggleStatusKey toggleStatusKey = - ToggleStatusKey.builder().databaseId("db").tableId("table").featureId("feature").build(); - TableToggleStatus expected = - TableToggleStatus.builder() - .databaseId("db") - .tableId("table") - .featureId("feature") - .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) - .build(); - - when(apiInstance.getTableToggleStatus("db", "table", "feature")) - .thenReturn(Mono.just(entityResponseBodyToggleStatus)); - when(entityResponseBodyToggleStatus.getEntity()).thenReturn(toggleStatus); - when(toggleStatusMapper.toTableToggleStatus(toggleStatusKey, toggleStatus)) - .thenReturn(expected); - - RequestContextHolder.setRequestAttributes( - new ServletRequestAttributes(new MockHttpServletRequest())); - try { - TableToggleStatus first = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); - TableToggleStatus second = toggleStatusesRepository.findById(toggleStatusKey).orElseThrow(); - - assertSame(expected, first); - assertSame(expected, second); - } finally { - RequestContextHolder.resetRequestAttributes(); - } - - verify(apiInstance, times(1)).getTableToggleStatus("db", "table", "feature"); - } -} From 58179c723ca052a519f20649c14d7c101b34525e Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Fri, 3 Apr 2026 20:32:09 -0700 Subject: [PATCH 05/15] Use Spring Caffeine for request metadata caching Replace the hand-rolled request cache with a Spring CacheManager backed by Caffeine so request-local metadata reuse is easier to extend and observe later. Made-with: Cursor --- services/common/build.gradle | 2 + .../common/cache/RequestScopedCache.java | 83 ++++++++++++------- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/services/common/build.gradle b/services/common/build.gradle index 83c54bfde..ca8bf81d6 100644 --- a/services/common/build.gradle +++ b/services/common/build.gradle @@ -36,6 +36,8 @@ dependencies { implementation 'io.opentelemetry:opentelemetry-sdk:1.47.0' implementation 'io.opentelemetry:opentelemetry-semconv:1.14.0-alpha' implementation 'org.apache.commons:commons-lang3:3.12.0' + implementation 'org.springframework:spring-context-support:5.3.18' + implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' // version chosen to be consistent with the transitive dependency // from the springboot framework's version in other modules. diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java index 56acfb962..57f9f6a29 100644 --- a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java +++ b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java @@ -1,64 +1,85 @@ package com.linkedin.openhouse.common.cache; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import com.github.benmanes.caffeine.cache.Caffeine; +import java.util.Objects; import java.util.function.Supplier; +import org.springframework.cache.Cache; +import org.springframework.cache.CacheManager; +import org.springframework.cache.caffeine.CaffeineCacheManager; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.RequestContextHolder; /** - * Small request-scoped cache backed by {@link RequestAttributes}. When there is no active request, - * cache lookups fall back to direct loading and writes are ignored. + * Small request-scoped cache backed by Spring's cache abstraction. A {@link CaffeineCacheManager} + * instance is bound to the active {@link RequestAttributes}, so each request gets isolated caches + * while the public API stays simple. When there is no active request, cache lookups fall back to + * direct loading and writes are ignored. * *

This class intentionally lives as a plain shared utility instead of a scanned Spring * component. OpenHouse applications use explicit package scan allowlists, and this package is not * included consistently across all application and test contexts. The default bean is therefore * provided from a scanned configuration class, which keeps the utility in {@code services/common} - * while still allowing downstream repos to override the bean if needed. + * while still allowing downstream repos to override the bean if needed. The cache manager is + * created lazily per request so future wiring can swap in a differently configured Spring {@link + * CacheManager} without changing call sites. */ public class RequestScopedCache { - private static final String ATTRIBUTE_NAME = RequestScopedCache.class.getName() + ".CACHE"; + private static final String ATTRIBUTE_NAME = + RequestScopedCache.class.getName() + ".CACHE_MANAGER"; + private final Supplier cacheManagerFactory; + + public RequestScopedCache() { + this(RequestScopedCache::defaultCacheManager); + } + + public RequestScopedCache(Supplier cacheManagerFactory) { + this.cacheManagerFactory = Objects.requireNonNull(cacheManagerFactory); + } public T getOrLoad(String namespace, Object key, Supplier loader) { - RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); - if (requestAttributes == null) { + Cache cache = currentCache(namespace); + if (cache == null) { return loader.get(); } - - return cast( - namespaceCache(requestAttributes, namespace).computeIfAbsent(key, unused -> loader.get())); + return cache.get(key, loader::get); } public void put(String namespace, Object key, Object value) { - RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); - if (requestAttributes == null) { + Cache cache = currentCache(namespace); + if (cache == null) { return; } - - namespaceCache(requestAttributes, namespace).put(key, value); + cache.put(key, value); } - @SuppressWarnings("unchecked") - private Map> requestCache(RequestAttributes requestAttributes) { - Map> cache = - (Map>) - requestAttributes.getAttribute(ATTRIBUTE_NAME, RequestAttributes.SCOPE_REQUEST); - if (cache == null) { - cache = new ConcurrentHashMap<>(); - requestAttributes.setAttribute(ATTRIBUTE_NAME, cache, RequestAttributes.SCOPE_REQUEST); + private Cache currentCache(String namespace) { + RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); + if (requestAttributes == null) { + return null; + } + CacheManager cacheManager = requestCacheManager(requestAttributes); + if (cacheManager == null) { + return null; } - return cache; + return cacheManager.getCache(namespace); } - private Map namespaceCache( - RequestAttributes requestAttributes, String namespace) { - return requestCache(requestAttributes) - .computeIfAbsent(namespace, unused -> new ConcurrentHashMap<>()); + private CacheManager requestCacheManager(RequestAttributes requestAttributes) { + CacheManager cacheManager = + (CacheManager) + requestAttributes.getAttribute(ATTRIBUTE_NAME, RequestAttributes.SCOPE_REQUEST); + if (cacheManager == null) { + cacheManager = cacheManagerFactory.get(); + requestAttributes.setAttribute(ATTRIBUTE_NAME, cacheManager, RequestAttributes.SCOPE_REQUEST); + } + return cacheManager; } - @SuppressWarnings("unchecked") - private static T cast(Object value) { - return (T) value; + private static CacheManager defaultCacheManager() { + CaffeineCacheManager cacheManager = new CaffeineCacheManager(); + cacheManager.setAllowNullValues(false); + cacheManager.setCaffeine(Caffeine.newBuilder().recordStats()); + return cacheManager; } } From f5aa788e9125ba0b96a02d8ea963efff40039ce9 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 12:28:03 -0700 Subject: [PATCH 06/15] Cache table metadata by location with Spring Caffeine Made-with: Cursor --- .../cluster/configs/ClusterProperties.java | 6 + .../openhouse/internalcatalog/build.gradle | 2 + .../catalog/InternalCatalogConfig.java | 46 +++- .../catalog/OpenHouseInternalCatalog.java | 6 +- .../OpenHouseInternalTableOperations.java | 22 +- .../cache/SpringTableMetadataCache.java | 27 ++ .../catalog/cache/TableMetadataCache.java | 11 + .../catalog/cache/TableMetadataCaches.java | 9 + .../OpenHouseInternalTableOperationsTest.java | 239 +++++++++--------- services/common/build.gradle | 3 - .../common/cache/RequestScopedCache.java | 85 ------- .../RepositoryTestWithSettableComponents.java | 42 ++- 12 files changed, 259 insertions(+), 239 deletions(-) create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCache.java create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java delete mode 100644 services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java diff --git a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java index 9d0c81876..7e919b02b 100644 --- a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java +++ b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java @@ -49,6 +49,12 @@ public class ClusterProperties { @Value("${cluster.iceberg.write.metadata.previous-versions-max:100}") private int clusterIcebergWriteMetadataPreviousVersionsMax; + @Value("${cluster.iceberg.metadata-cache.ttl:5m}") + private String clusterIcebergMetadataCacheTtl; + + @Value("${cluster.iceberg.metadata-cache.max-size:1000}") + private int clusterIcebergMetadataCacheMaxSize; + @Value("${cluster.housetables.base-uri:#{null}}") private String clusterHouseTablesBaseUri; diff --git a/iceberg/openhouse/internalcatalog/build.gradle b/iceberg/openhouse/internalcatalog/build.gradle index b1e5c44c4..f71baf319 100644 --- a/iceberg/openhouse/internalcatalog/build.gradle +++ b/iceberg/openhouse/internalcatalog/build.gradle @@ -19,6 +19,8 @@ dependencies { api project(':cluster:metrics') api project(':iceberg:azure') api project(':services:common') + implementation 'org.springframework:spring-context-support:5.3.18' + implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' // Needed for testing against manually bootstrapped HTS server through bootRun task. testImplementation "io.netty:netty-resolver-dns-native-macos:4.1.70.Final:osx-x86_64" // Needed for testing against mock HTS server along with other constructs in /tables module. diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java index 11750db3c..84ffeb08f 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java @@ -1,22 +1,44 @@ package com.linkedin.openhouse.internal.catalog; -import com.linkedin.openhouse.common.cache.RequestScopedCache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.linkedin.openhouse.cluster.configs.ClusterProperties; +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; +import java.util.List; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.convert.DurationStyle; +import org.springframework.cache.CacheManager; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.caffeine.CaffeineCacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -/** - * Registers the default {@link RequestScopedCache} bean from the {@code internal.catalog} package - * because OpenHouse applications explicitly scan this package but do not consistently scan {@code - * com.linkedin.openhouse.common.cache}. Keeping bean registration here avoids widening every - * application/test scan list while preserving the cache implementation as a shared utility in - * {@code services/common}. This default can still be replaced by another bean, for example via - * {@code @Primary} in a downstream repo. - */ @Configuration +@EnableCaching public class InternalCatalogConfig { - @Bean - public RequestScopedCache provideRequestScopedCache() { - return new RequestScopedCache(); + private static final String DEFAULT_METADATA_CACHE_TTL = "5m"; + private static final int DEFAULT_METADATA_CACHE_MAX_SIZE = 1000; + + @Bean(name = TableMetadataCaches.CACHE_MANAGER) + public CacheManager internalCatalogCacheManager( + ObjectProvider clusterPropertiesProvider) { + ClusterProperties clusterProperties = clusterPropertiesProvider.getIfAvailable(); + String metadataCacheTtl = + clusterProperties == null + ? DEFAULT_METADATA_CACHE_TTL + : clusterProperties.getClusterIcebergMetadataCacheTtl(); + int metadataCacheMaxSize = + clusterProperties == null + ? DEFAULT_METADATA_CACHE_MAX_SIZE + : clusterProperties.getClusterIcebergMetadataCacheMaxSize(); + CaffeineCacheManager cacheManager = new CaffeineCacheManager(); + cacheManager.setAllowNullValues(false); + cacheManager.setCacheNames(List.of(TableMetadataCaches.TABLE_METADATA)); + cacheManager.setCaffeine( + Caffeine.newBuilder() + .expireAfterWrite(DurationStyle.detectAndParse(metadataCacheTtl)) + .maximumSize(metadataCacheMaxSize) + .recordStats()); + return cacheManager; } } diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java index e75791eb7..f73b2d71e 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java @@ -7,9 +7,9 @@ import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.cluster.storage.selector.StorageSelector; import com.linkedin.openhouse.common.api.spec.TableUri; -import com.linkedin.openhouse.common.cache.RequestScopedCache; import com.linkedin.openhouse.common.exception.AlreadyExistsException; import com.linkedin.openhouse.common.exception.NoSuchSoftDeletedUserTableException; +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCache; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTable; @@ -64,7 +64,7 @@ public class OpenHouseInternalCatalog extends BaseMetastoreCatalog { @Autowired MeterRegistry meterRegistry; - @Autowired RequestScopedCache requestScopedCache; + @Autowired TableMetadataCache tableMetadataCache; @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { @@ -78,7 +78,7 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) { tableIdentifier, metricsReporter, fileIOManager, - requestScopedCache); + tableMetadataCache); } @Override diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java index 656bec9ce..913a9cbb8 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java @@ -12,7 +12,7 @@ import com.linkedin.openhouse.cluster.storage.StorageClient; import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorageClient; import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; -import com.linkedin.openhouse.common.cache.RequestScopedCache; +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCache; import com.linkedin.openhouse.internal.catalog.exception.InvalidIcebergSnapshotException; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; @@ -84,11 +84,9 @@ public class OpenHouseInternalTableOperations extends BaseMetastoreTableOperatio FileIOManager fileIOManager; - RequestScopedCache requestScopedCache; + TableMetadataCache tableMetadataCache; private static final Gson GSON = new Gson(); - private static final String TABLE_METADATA_CACHE_NAMESPACE = - OpenHouseInternalTableOperations.class.getName() + ".tableMetadata"; private static final Cache CACHE = CacheBuilder.newBuilder().expireAfterWrite(5, TimeUnit.MINUTES).maximumSize(1000).build(); @@ -139,7 +137,7 @@ protected void refreshMetadata(final String metadataLoc) { Runnable r = () -> super.refreshFromMetadataLocation( - metadataLoc, null, 20, this::loadTableMetadataWithRequestCache); + metadataLoc, null, 20, this::loadTableMetadataWithCache); try { if (needToReload) { metricsReporter.executeWithStats( @@ -335,6 +333,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { } final TableMetadata updatedMtDataRef = metadataToCommit; + TableMetadata cachedTableMetadata = updatedMtDataRef; Tracer tracer = GlobalOpenTelemetry.getTracer("openhouse-tables"); Span writeSpan = tracer.spanBuilder("IcebergTableOps.writeMetadata").startSpan(); long metadataUpdateStartTime = System.currentTimeMillis(); @@ -345,8 +344,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { updatedMtDataRef, io().newOutputFile(newMetadataLocation)), InternalCatalogMetricsConstant.METADATA_UPDATE_LATENCY, getCatalogMetricTags()); - requestScopedCache.put( - TABLE_METADATA_CACHE_NAMESPACE, newMetadataLocation, updatedMtDataRef); + cachedTableMetadata = tableMetadataCache.seed(newMetadataLocation, updatedMtDataRef); log.info( "updateMetadata to location {} succeeded, took {} ms", newMetadataLocation, @@ -364,7 +362,7 @@ updatedMtDataRef, io().newOutputFile(newMetadataLocation)), writeSpan.end(); } - houseTable = houseTableMapper.toHouseTable(metadataToCommit, fileIO); + houseTable = houseTableMapper.toHouseTable(cachedTableMetadata, fileIO); if (base != null && (properties.containsKey(CatalogConstants.OPENHOUSE_TABLEID_KEY) && !properties @@ -779,10 +777,8 @@ private List getIntermediateSchemasFromProps(TableMetadata metadata) { .fromJson(serializedNewIntermediateSchemas, new TypeToken>() {}.getType()); } - private TableMetadata loadTableMetadataWithRequestCache(String metadataLocation) { - return requestScopedCache.getOrLoad( - TABLE_METADATA_CACHE_NAMESPACE, - metadataLocation, - () -> TableMetadataParser.read(io(), metadataLocation)); + private TableMetadata loadTableMetadataWithCache(String metadataLocation) { + return tableMetadataCache.load( + metadataLocation, () -> TableMetadataParser.read(io(), metadataLocation)); } } diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java new file mode 100644 index 000000000..f2eb027e3 --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java @@ -0,0 +1,27 @@ +package com.linkedin.openhouse.internal.catalog.cache; + +import java.util.function.Supplier; +import org.apache.iceberg.TableMetadata; +import org.springframework.cache.annotation.CacheConfig; +import org.springframework.cache.annotation.CachePut; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.stereotype.Component; + +@Component +@CacheConfig( + cacheManager = TableMetadataCaches.CACHE_MANAGER, + cacheNames = TableMetadataCaches.TABLE_METADATA) +public class SpringTableMetadataCache implements TableMetadataCache { + + @Override + @Cacheable(key = "#metadataLocation") + public TableMetadata load(String metadataLocation, Supplier metadataLoader) { + return metadataLoader.get(); + } + + @Override + @CachePut(key = "#metadataLocation") + public TableMetadata seed(String metadataLocation, TableMetadata tableMetadata) { + return tableMetadata; + } +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCache.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCache.java new file mode 100644 index 000000000..1f10aefda --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCache.java @@ -0,0 +1,11 @@ +package com.linkedin.openhouse.internal.catalog.cache; + +import java.util.function.Supplier; +import org.apache.iceberg.TableMetadata; + +public interface TableMetadataCache { + + TableMetadata load(String metadataLocation, Supplier metadataLoader); + + TableMetadata seed(String metadataLocation, TableMetadata tableMetadata); +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java new file mode 100644 index 000000000..35312d5ea --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java @@ -0,0 +1,9 @@ +package com.linkedin.openhouse.internal.catalog.cache; + +public final class TableMetadataCaches { + + public static final String CACHE_MANAGER = "internalCatalogCacheManager"; + public static final String TABLE_METADATA = "tableMetadata"; + + private TableMetadataCaches() {} +} diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java index 19ba2908d..98587a2a3 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java @@ -8,7 +8,7 @@ import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.cluster.storage.local.LocalStorage; import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; -import com.linkedin.openhouse.common.cache.RequestScopedCache; +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCache; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTable; @@ -35,8 +35,10 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.stream.Collectors; import lombok.SneakyThrows; import org.apache.commons.compress.utils.Lists; @@ -74,9 +76,6 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.web.context.request.RequestContextHolder; -import org.springframework.web.context.request.ServletRequestAttributes; public class OpenHouseInternalTableOperationsTest { private static final String TEST_LOCATION = "test_location"; @@ -101,7 +100,7 @@ public class OpenHouseInternalTableOperationsTest { @Mock private FSDataInputStream mockFSDataInputStream; @Mock private FSDataOutputStream mockFSDataOutputStream; - private RequestScopedCache requestScopedCache; + private TableMetadataCache tableMetadataCache; private OpenHouseInternalTableOperations openHouseInternalTableOperations; private OpenHouseInternalTableOperations openHouseInternalTableOperationsWithMockMetrics; @@ -113,7 +112,7 @@ private static String getTempLocation() { @BeforeEach void setup() { MockitoAnnotations.openMocks(this); - requestScopedCache = new RequestScopedCache(); + tableMetadataCache = new InMemoryTableMetadataCache(); Mockito.when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any())) .thenReturn(mockHouseTable); HadoopFileIO fileIO = new HadoopFileIO(new Configuration()); @@ -127,7 +126,7 @@ void setup() { TEST_TABLE_IDENTIFIER, metricsReporter, fileIOManager, - requestScopedCache); + tableMetadataCache); // Create a separate instance with mock metrics reporter for testing metrics openHouseInternalTableOperationsWithMockMetrics = @@ -138,7 +137,7 @@ void setup() { TEST_TABLE_IDENTIFIER, mockMetricsReporter, fileIOManager, - requestScopedCache); + tableMetadataCache); LocalStorage localStorage = mock(LocalStorage.class); when(fileIOManager.getStorage(fileIO)).thenReturn(localStorage); @@ -1078,122 +1077,109 @@ void testCommitMetadataUpdateLatencyHasHistogramBuckets() { } @Test - void testRefreshReusesCachedMetadataWithinSingleRequest() { - RequestContextHolder.setRequestAttributes( - new ServletRequestAttributes(new MockHttpServletRequest())); - try { - HouseTablePrimaryKey primaryKey = - HouseTablePrimaryKey.builder() - .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) - .tableId(TEST_TABLE_IDENTIFIER.name()) - .build(); - when(mockHouseTableRepository.findById(primaryKey)).thenReturn(Optional.of(mockHouseTable)); - when(mockHouseTable.getTableLocation()).thenReturn("test_metadata_location"); - - OpenHouseInternalTableOperations secondOperations = - new OpenHouseInternalTableOperations( - mockHouseTableRepository, - new HadoopFileIO(new Configuration()), - mockHouseTableMapper, - TEST_TABLE_IDENTIFIER, - new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), - fileIOManager, - requestScopedCache); - - try (MockedStatic parserMock = - Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { - parserMock - .when( - () -> - TableMetadataParser.read( - Mockito.any(FileIO.class), Mockito.eq("test_metadata_location"))) - .thenReturn(BASE_TABLE_METADATA); - - openHouseInternalTableOperations.refresh(); - secondOperations.refresh(); - - parserMock.verify( - () -> - TableMetadataParser.read( - Mockito.any(FileIO.class), Mockito.eq("test_metadata_location")), - times(1)); - } - } finally { - RequestContextHolder.resetRequestAttributes(); + void testRefreshReusesCachedMetadataAcrossOperations() { + HouseTablePrimaryKey primaryKey = + HouseTablePrimaryKey.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .build(); + when(mockHouseTableRepository.findById(primaryKey)).thenReturn(Optional.of(mockHouseTable)); + when(mockHouseTable.getTableLocation()).thenReturn("test_metadata_location"); + + OpenHouseInternalTableOperations secondOperations = + new OpenHouseInternalTableOperations( + mockHouseTableRepository, + new HadoopFileIO(new Configuration()), + mockHouseTableMapper, + TEST_TABLE_IDENTIFIER, + new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), + fileIOManager, + tableMetadataCache); + + try (MockedStatic parserMock = + Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { + parserMock + .when( + () -> + TableMetadataParser.read( + Mockito.any(FileIO.class), Mockito.eq("test_metadata_location"))) + .thenReturn(BASE_TABLE_METADATA); + + openHouseInternalTableOperations.refresh(); + secondOperations.refresh(); + + parserMock.verify( + () -> + TableMetadataParser.read( + Mockito.any(FileIO.class), Mockito.eq("test_metadata_location")), + times(1)); } } @Test - void testCommitSeedsRequestCacheForSubsequentRefresh() { - RequestContextHolder.setRequestAttributes( - new ServletRequestAttributes(new MockHttpServletRequest())); - try { - AtomicReference savedHouseTable = new AtomicReference<>(); - HouseTablePrimaryKey primaryKey = - HouseTablePrimaryKey.builder() - .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) - .tableId(TEST_TABLE_IDENTIFIER.name()) - .build(); - when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any())) - .thenAnswer( - invocation -> { - TableMetadata tableMetadata = invocation.getArgument(0); - HouseTable mappedHouseTable = - HouseTable.builder() - .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) - .tableId(TEST_TABLE_IDENTIFIER.name()) - .tableLocation( - tableMetadata.properties().get(getCanonicalFieldName("tableLocation"))) - .build(); - savedHouseTable.set(mappedHouseTable); - return mappedHouseTable; - }); - when(mockHouseTableRepository.save(Mockito.any(HouseTable.class))) - .thenAnswer( - invocation -> { - HouseTable houseTable = invocation.getArgument(0); - savedHouseTable.set(houseTable); - return houseTable; - }); - when(mockHouseTableRepository.findById(primaryKey)) - .thenAnswer(invocation -> Optional.ofNullable(savedHouseTable.get())); - - OpenHouseInternalTableOperations refreshedOperations = - new OpenHouseInternalTableOperations( - mockHouseTableRepository, - new HadoopFileIO(new Configuration()), - mockHouseTableMapper, - TEST_TABLE_IDENTIFIER, - new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), - fileIOManager, - requestScopedCache); - - Map properties = new HashMap<>(BASE_TABLE_METADATA.properties()); - properties.put(getCanonicalFieldName("tableLocation"), TEST_LOCATION); - TableMetadata metadata = BASE_TABLE_METADATA.replaceProperties(properties); + void testCommitSeedsCacheForSubsequentRefresh() { + AtomicReference savedHouseTable = new AtomicReference<>(); + HouseTablePrimaryKey primaryKey = + HouseTablePrimaryKey.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .build(); + when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any())) + .thenAnswer( + invocation -> { + TableMetadata tableMetadata = invocation.getArgument(0); + HouseTable mappedHouseTable = + HouseTable.builder() + .databaseId(TEST_TABLE_IDENTIFIER.namespace().toString()) + .tableId(TEST_TABLE_IDENTIFIER.name()) + .tableLocation( + tableMetadata.properties().get(getCanonicalFieldName("tableLocation"))) + .build(); + savedHouseTable.set(mappedHouseTable); + return mappedHouseTable; + }); + when(mockHouseTableRepository.save(Mockito.any(HouseTable.class))) + .thenAnswer( + invocation -> { + HouseTable houseTable = invocation.getArgument(0); + savedHouseTable.set(houseTable); + return houseTable; + }); + when(mockHouseTableRepository.findById(primaryKey)) + .thenAnswer(invocation -> Optional.ofNullable(savedHouseTable.get())); - try (MockedStatic parserMock = - Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { - parserMock - .when( - () -> - TableMetadataParser.write( - Mockito.any(TableMetadata.class), - Mockito.any(org.apache.iceberg.io.OutputFile.class))) - .thenAnswer(invocation -> null); + OpenHouseInternalTableOperations refreshedOperations = + new OpenHouseInternalTableOperations( + mockHouseTableRepository, + new HadoopFileIO(new Configuration()), + mockHouseTableMapper, + TEST_TABLE_IDENTIFIER, + new MetricsReporter(new SimpleMeterRegistry(), "TEST_CATALOG", Lists.newArrayList()), + fileIOManager, + tableMetadataCache); - openHouseInternalTableOperations.doCommit(BASE_TABLE_METADATA, metadata); - refreshedOperations.refresh(); + Map properties = new HashMap<>(BASE_TABLE_METADATA.properties()); + properties.put(getCanonicalFieldName("tableLocation"), TEST_LOCATION); + TableMetadata metadata = BASE_TABLE_METADATA.replaceProperties(properties); - String committedLocation = savedHouseTable.get().getTableLocation(); - Assertions.assertEquals(committedLocation, refreshedOperations.currentMetadataLocation()); - parserMock.verify( - () -> - TableMetadataParser.read(Mockito.any(FileIO.class), Mockito.eq(committedLocation)), - never()); - } - } finally { - RequestContextHolder.resetRequestAttributes(); + try (MockedStatic parserMock = + Mockito.mockStatic(TableMetadataParser.class, Mockito.CALLS_REAL_METHODS)) { + parserMock + .when( + () -> + TableMetadataParser.write( + Mockito.any(TableMetadata.class), + Mockito.any(org.apache.iceberg.io.OutputFile.class))) + .thenAnswer(invocation -> null); + + openHouseInternalTableOperations.doCommit(BASE_TABLE_METADATA, metadata); + refreshedOperations.refresh(); + + String committedLocation = savedHouseTable.get().getTableLocation(); + Assertions.assertEquals(committedLocation, refreshedOperations.currentMetadataLocation()); + parserMock.verify( + () -> TableMetadataParser.read(Mockito.any(FileIO.class), Mockito.eq(committedLocation)), + never()); } } @@ -1225,7 +1211,7 @@ private void testMetricExcludesDatabaseTag( TEST_TABLE_IDENTIFIER, realMetricsReporter, fileIOManager, - requestScopedCache); + tableMetadataCache); // Setup test-specific mocks setupFunction.accept(operationsWithRealMetrics); @@ -1288,7 +1274,7 @@ private void testMetricHasHistogramBuckets( TEST_TABLE_IDENTIFIER, realMetricsReporter, fileIOManager, - requestScopedCache); + tableMetadataCache); // Setup test-specific mocks setupFunction.accept(operationsWithRealMetrics); @@ -1992,4 +1978,19 @@ void testDoCommitCreatesOtelSpans() { tracerProvider.close(); } } + + private static final class InMemoryTableMetadataCache implements TableMetadataCache { + private final Map cache = new ConcurrentHashMap<>(); + + @Override + public TableMetadata load(String metadataLocation, Supplier metadataLoader) { + return cache.computeIfAbsent(metadataLocation, ignored -> metadataLoader.get()); + } + + @Override + public TableMetadata seed(String metadataLocation, TableMetadata tableMetadata) { + cache.put(metadataLocation, tableMetadata); + return tableMetadata; + } + } } diff --git a/services/common/build.gradle b/services/common/build.gradle index ca8bf81d6..f005a8c6c 100644 --- a/services/common/build.gradle +++ b/services/common/build.gradle @@ -36,9 +36,6 @@ dependencies { implementation 'io.opentelemetry:opentelemetry-sdk:1.47.0' implementation 'io.opentelemetry:opentelemetry-semconv:1.14.0-alpha' implementation 'org.apache.commons:commons-lang3:3.12.0' - implementation 'org.springframework:spring-context-support:5.3.18' - implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' - // version chosen to be consistent with the transitive dependency // from the springboot framework's version in other modules. testImplementation 'commons-io:commons-io:2.4' diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java b/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java deleted file mode 100644 index 57f9f6a29..000000000 --- a/services/common/src/main/java/com/linkedin/openhouse/common/cache/RequestScopedCache.java +++ /dev/null @@ -1,85 +0,0 @@ -package com.linkedin.openhouse.common.cache; - -import com.github.benmanes.caffeine.cache.Caffeine; -import java.util.Objects; -import java.util.function.Supplier; -import org.springframework.cache.Cache; -import org.springframework.cache.CacheManager; -import org.springframework.cache.caffeine.CaffeineCacheManager; -import org.springframework.web.context.request.RequestAttributes; -import org.springframework.web.context.request.RequestContextHolder; - -/** - * Small request-scoped cache backed by Spring's cache abstraction. A {@link CaffeineCacheManager} - * instance is bound to the active {@link RequestAttributes}, so each request gets isolated caches - * while the public API stays simple. When there is no active request, cache lookups fall back to - * direct loading and writes are ignored. - * - *

This class intentionally lives as a plain shared utility instead of a scanned Spring - * component. OpenHouse applications use explicit package scan allowlists, and this package is not - * included consistently across all application and test contexts. The default bean is therefore - * provided from a scanned configuration class, which keeps the utility in {@code services/common} - * while still allowing downstream repos to override the bean if needed. The cache manager is - * created lazily per request so future wiring can swap in a differently configured Spring {@link - * CacheManager} without changing call sites. - */ -public class RequestScopedCache { - - private static final String ATTRIBUTE_NAME = - RequestScopedCache.class.getName() + ".CACHE_MANAGER"; - private final Supplier cacheManagerFactory; - - public RequestScopedCache() { - this(RequestScopedCache::defaultCacheManager); - } - - public RequestScopedCache(Supplier cacheManagerFactory) { - this.cacheManagerFactory = Objects.requireNonNull(cacheManagerFactory); - } - - public T getOrLoad(String namespace, Object key, Supplier loader) { - Cache cache = currentCache(namespace); - if (cache == null) { - return loader.get(); - } - return cache.get(key, loader::get); - } - - public void put(String namespace, Object key, Object value) { - Cache cache = currentCache(namespace); - if (cache == null) { - return; - } - cache.put(key, value); - } - - private Cache currentCache(String namespace) { - RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); - if (requestAttributes == null) { - return null; - } - CacheManager cacheManager = requestCacheManager(requestAttributes); - if (cacheManager == null) { - return null; - } - return cacheManager.getCache(namespace); - } - - private CacheManager requestCacheManager(RequestAttributes requestAttributes) { - CacheManager cacheManager = - (CacheManager) - requestAttributes.getAttribute(ATTRIBUTE_NAME, RequestAttributes.SCOPE_REQUEST); - if (cacheManager == null) { - cacheManager = cacheManagerFactory.get(); - requestAttributes.setAttribute(ATTRIBUTE_NAME, cacheManager, RequestAttributes.SCOPE_REQUEST); - } - return cacheManager; - } - - private static CacheManager defaultCacheManager() { - CaffeineCacheManager cacheManager = new CaffeineCacheManager(); - cacheManager.setAllowNullValues(false); - cacheManager.setCaffeine(Caffeine.newBuilder().recordStats()); - return cacheManager; - } -} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java index 1ce1cae1d..dcc5b4b04 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java @@ -8,6 +8,7 @@ import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.internal.catalog.OpenHouseInternalTableOperations; +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCache; import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTable; @@ -26,9 +27,12 @@ import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; import javax.annotation.PostConstruct; import org.apache.iceberg.BaseTable; import org.apache.iceberg.Table; +import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.TableIdentifier; @@ -88,6 +92,22 @@ private HouseTableRepository provideFailedHtsRepoWhenSave(String tableLocation) return htsRepo; } + private TableMetadataCache newTableMetadataCache() { + Map cache = new ConcurrentHashMap<>(); + return new TableMetadataCache() { + @Override + public TableMetadata load(String metadataLocation, Supplier metadataLoader) { + return cache.computeIfAbsent(metadataLocation, ignored -> metadataLoader.get()); + } + + @Override + public TableMetadata seed(String metadataLocation, TableMetadata tableMetadata) { + cache.put(metadataLocation, tableMetadata); + return tableMetadata; + } + }; + } + @Test void testNoRetryInternalRepo() throws Exception { TableIdentifier tableIdentifier = @@ -104,7 +124,8 @@ void testNoRetryInternalRepo() throws Exception { houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager); + fileIOManager, + newTableMetadataCache()); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); creationDTO = openHouseInternalRepository.save(creationDTO); @@ -120,7 +141,13 @@ void testNoRetryInternalRepo() throws Exception { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter2, fileIOManager); + htsRepo, + fileIO, + houseTableMapper, + tableIdentifier, + metricsReporter2, + fileIOManager, + newTableMetadataCache()); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy(new BaseTable(spyOperations, realTable.name())); @@ -192,7 +219,8 @@ void testSaveClearsTransientCommitPropertiesDuringTransaction() throws Exception houseTableMapper, tableIdentifier, metricsReporter, - fileIOManager); + fileIOManager, + newTableMetadataCache()); ((SettableCatalogForTest) catalog).setOperation(actualOps); TableDto creationDTO = TABLE_DTO.toBuilder().tableVersion(INITIAL_TABLE_VERSION).build(); @@ -251,7 +279,13 @@ void testFailedHtsRepoWhenGet() { new MetricsReporter(this.meterRegistry, "test", Lists.newArrayList()); OpenHouseInternalTableOperations mockOps = new OpenHouseInternalTableOperations( - htsRepo, fileIO, houseTableMapper, tableIdentifier, metricsReporter, fileIOManager); + htsRepo, + fileIO, + houseTableMapper, + tableIdentifier, + metricsReporter, + fileIOManager, + newTableMetadataCache()); OpenHouseInternalTableOperations spyOperations = Mockito.spy(mockOps); BaseTable spyOptsMockedTable = Mockito.spy( From 836c68f28c3eba8faf11ea1bc35ddd397da61eb3 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 13:22:17 -0700 Subject: [PATCH 07/15] Remove unused cached metadata fallback The metadata write path already rethrows cache seeding failures before house table mapping runs, so the extra variable never changes behavior. Made-with: Cursor --- .../internal/catalog/OpenHouseInternalTableOperations.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java index 913a9cbb8..17497f98e 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java @@ -333,7 +333,6 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { } final TableMetadata updatedMtDataRef = metadataToCommit; - TableMetadata cachedTableMetadata = updatedMtDataRef; Tracer tracer = GlobalOpenTelemetry.getTracer("openhouse-tables"); Span writeSpan = tracer.spanBuilder("IcebergTableOps.writeMetadata").startSpan(); long metadataUpdateStartTime = System.currentTimeMillis(); @@ -344,7 +343,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { updatedMtDataRef, io().newOutputFile(newMetadataLocation)), InternalCatalogMetricsConstant.METADATA_UPDATE_LATENCY, getCatalogMetricTags()); - cachedTableMetadata = tableMetadataCache.seed(newMetadataLocation, updatedMtDataRef); + tableMetadataCache.seed(newMetadataLocation, updatedMtDataRef); log.info( "updateMetadata to location {} succeeded, took {} ms", newMetadataLocation, @@ -362,7 +361,7 @@ updatedMtDataRef, io().newOutputFile(newMetadataLocation)), writeSpan.end(); } - houseTable = houseTableMapper.toHouseTable(cachedTableMetadata, fileIO); + houseTable = houseTableMapper.toHouseTable(updatedMtDataRef, fileIO); if (base != null && (properties.containsKey(CatalogConstants.OPENHOUSE_TABLEID_KEY) && !properties From 226ec0aff843bb045df20eef4720546700b1c94e Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 13:40:50 -0700 Subject: [PATCH 08/15] Bind internal catalog metadata cache properties natively Use a dedicated ConfigurationProperties bean so Spring applies defaults and binds cache overrides from cluster YAML without manual parsing in InternalCatalogConfig. Made-with: Cursor --- .../catalog/InternalCatalogConfig.java | 30 ++++----- .../catalog/InternalCatalogProperties.java | 16 +++++ .../catalog/InternalCatalogConfigTest.java | 63 +++++++++++++++++++ 3 files changed, 91 insertions(+), 18 deletions(-) create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java create mode 100644 iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java index 84ffeb08f..59e53bd00 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java @@ -1,43 +1,37 @@ package com.linkedin.openhouse.internal.catalog; import com.github.benmanes.caffeine.cache.Caffeine; -import com.linkedin.openhouse.cluster.configs.ClusterProperties; +import com.linkedin.openhouse.cluster.configs.YamlPropertySourceFactory; import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; import java.util.List; -import org.springframework.beans.factory.ObjectProvider; -import org.springframework.boot.convert.DurationStyle; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.EnableCaching; import org.springframework.cache.caffeine.CaffeineCacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.PropertySource; @Configuration @EnableCaching +@EnableConfigurationProperties(InternalCatalogProperties.class) +@PropertySource( + name = "internalCatalogCluster", + value = "file:${OPENHOUSE_CLUSTER_CONFIG_PATH:/var/config/cluster.yaml}", + factory = YamlPropertySourceFactory.class, + ignoreResourceNotFound = true) public class InternalCatalogConfig { - private static final String DEFAULT_METADATA_CACHE_TTL = "5m"; - private static final int DEFAULT_METADATA_CACHE_MAX_SIZE = 1000; - @Bean(name = TableMetadataCaches.CACHE_MANAGER) public CacheManager internalCatalogCacheManager( - ObjectProvider clusterPropertiesProvider) { - ClusterProperties clusterProperties = clusterPropertiesProvider.getIfAvailable(); - String metadataCacheTtl = - clusterProperties == null - ? DEFAULT_METADATA_CACHE_TTL - : clusterProperties.getClusterIcebergMetadataCacheTtl(); - int metadataCacheMaxSize = - clusterProperties == null - ? DEFAULT_METADATA_CACHE_MAX_SIZE - : clusterProperties.getClusterIcebergMetadataCacheMaxSize(); + InternalCatalogProperties internalCatalogProperties) { CaffeineCacheManager cacheManager = new CaffeineCacheManager(); cacheManager.setAllowNullValues(false); cacheManager.setCacheNames(List.of(TableMetadataCaches.TABLE_METADATA)); cacheManager.setCaffeine( Caffeine.newBuilder() - .expireAfterWrite(DurationStyle.detectAndParse(metadataCacheTtl)) - .maximumSize(metadataCacheMaxSize) + .expireAfterWrite(internalCatalogProperties.getTtl()) + .maximumSize(internalCatalogProperties.getMaxSize()) .recordStats()); return cacheManager; } diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java new file mode 100644 index 000000000..e094a2d75 --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java @@ -0,0 +1,16 @@ +package com.linkedin.openhouse.internal.catalog; + +import java.time.Duration; +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** Typed configuration for internal catalog metadata caching. */ +@Getter +@Setter +@ConfigurationProperties(prefix = "cluster.iceberg.metadata-cache") +public class InternalCatalogProperties { + + private Duration ttl = Duration.ofMinutes(5); + private long maxSize = 1000; +} diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java new file mode 100644 index 000000000..c7d95ea1f --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java @@ -0,0 +1,63 @@ +package com.linkedin.openhouse.internal.catalog; + +import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cache.caffeine.CaffeineCache; +import org.springframework.cache.caffeine.CaffeineCacheManager; + +public class InternalCatalogConfigTest { + + private final ApplicationContextRunner contextRunner = + new ApplicationContextRunner().withUserConfiguration(InternalCatalogConfig.class); + + @Test + public void testDefaultMetadataCacheProperties() { + contextRunner.run( + context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(5), 1000)); + } + + @Test + public void testOverriddenMetadataCacheProperties() { + contextRunner + .withPropertyValues( + "cluster.iceberg.metadata-cache.ttl=7m", "cluster.iceberg.metadata-cache.max-size=42") + .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42)); + } + + private void assertMetadataCacheConfiguration( + AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { + Assertions.assertNull(context.getStartupFailure()); + + InternalCatalogProperties internalCatalogProperties = + context.getBean(InternalCatalogProperties.class); + Assertions.assertEquals(expectedTtl, internalCatalogProperties.getTtl()); + Assertions.assertEquals(expectedMaxSize, internalCatalogProperties.getMaxSize()); + + CaffeineCacheManager cacheManager = + context.getBean(TableMetadataCaches.CACHE_MANAGER, CaffeineCacheManager.class); + Assertions.assertEquals( + List.of(TableMetadataCaches.TABLE_METADATA), List.copyOf(cacheManager.getCacheNames())); + + CaffeineCache tableMetadataCache = + (CaffeineCache) cacheManager.getCache(TableMetadataCaches.TABLE_METADATA); + Assertions.assertNotNull(tableMetadataCache); + + com.github.benmanes.caffeine.cache.Cache nativeCache = + tableMetadataCache.getNativeCache(); + Assertions.assertEquals( + expectedTtl.toNanos(), + nativeCache + .policy() + .expireAfterWrite() + .orElseThrow() + .getExpiresAfter(TimeUnit.NANOSECONDS)); + Assertions.assertEquals( + expectedMaxSize, nativeCache.policy().eviction().orElseThrow().getMaximum()); + } +} From 37ab1d05800c863629c2e68299c8a0ed346462e4 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 15:05:08 -0700 Subject: [PATCH 09/15] Move internal catalog cache loading into tables service This keeps the cache manager owned by the tables app so metadata cache settings bind through the shared Spring environment without creating the cache in other services. Made-with: Cursor --- iceberg/openhouse/internalcatalog/build.gradle | 2 -- services/tables/build.gradle | 2 ++ .../tables/config/InternalCatalogCacheConfig.java | 12 +++--------- .../config/InternalCatalogCacheConfigTest.java | 7 ++++--- 4 files changed, 9 insertions(+), 14 deletions(-) rename iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java => services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java (73%) rename iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java => services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java (92%) diff --git a/iceberg/openhouse/internalcatalog/build.gradle b/iceberg/openhouse/internalcatalog/build.gradle index f71baf319..b1e5c44c4 100644 --- a/iceberg/openhouse/internalcatalog/build.gradle +++ b/iceberg/openhouse/internalcatalog/build.gradle @@ -19,8 +19,6 @@ dependencies { api project(':cluster:metrics') api project(':iceberg:azure') api project(':services:common') - implementation 'org.springframework:spring-context-support:5.3.18' - implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' // Needed for testing against manually bootstrapped HTS server through bootRun task. testImplementation "io.netty:netty-resolver-dns-native-macos:4.1.70.Final:osx-x86_64" // Needed for testing against mock HTS server along with other constructs in /tables module. diff --git a/services/tables/build.gradle b/services/tables/build.gradle index daadd8297..896b661a1 100644 --- a/services/tables/build.gradle +++ b/services/tables/build.gradle @@ -40,6 +40,8 @@ dependencies { api project(':cluster:metrics') api 'org.springframework.security:spring-security-config:5.7.2' api 'org.springframework.boot:spring-boot-starter-webflux:2.7.8' + implementation 'org.springframework:spring-context-support:5.3.18' + implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' implementation 'com.cronutils:cron-utils:9.2.0' testImplementation 'org.junit.jupiter:junit-jupiter-engine:' + junit_version testImplementation 'org.springframework.security:spring-security-test:5.7.3' diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java similarity index 73% rename from iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java rename to services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java index 59e53bd00..6e1d93484 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfig.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java @@ -1,7 +1,7 @@ -package com.linkedin.openhouse.internal.catalog; +package com.linkedin.openhouse.tables.config; import com.github.benmanes.caffeine.cache.Caffeine; -import com.linkedin.openhouse.cluster.configs.YamlPropertySourceFactory; +import com.linkedin.openhouse.internal.catalog.InternalCatalogProperties; import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; import java.util.List; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -10,17 +10,11 @@ import org.springframework.cache.caffeine.CaffeineCacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.PropertySource; @Configuration @EnableCaching @EnableConfigurationProperties(InternalCatalogProperties.class) -@PropertySource( - name = "internalCatalogCluster", - value = "file:${OPENHOUSE_CLUSTER_CONFIG_PATH:/var/config/cluster.yaml}", - factory = YamlPropertySourceFactory.class, - ignoreResourceNotFound = true) -public class InternalCatalogConfig { +public class InternalCatalogCacheConfig { @Bean(name = TableMetadataCaches.CACHE_MANAGER) public CacheManager internalCatalogCacheManager( diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java similarity index 92% rename from iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java rename to services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java index c7d95ea1f..e658850cf 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/InternalCatalogConfigTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java @@ -1,5 +1,6 @@ -package com.linkedin.openhouse.internal.catalog; +package com.linkedin.openhouse.tables.config; +import com.linkedin.openhouse.internal.catalog.InternalCatalogProperties; import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; import java.time.Duration; import java.util.List; @@ -11,10 +12,10 @@ import org.springframework.cache.caffeine.CaffeineCache; import org.springframework.cache.caffeine.CaffeineCacheManager; -public class InternalCatalogConfigTest { +class InternalCatalogCacheConfigTest { private final ApplicationContextRunner contextRunner = - new ApplicationContextRunner().withUserConfiguration(InternalCatalogConfig.class); + new ApplicationContextRunner().withUserConfiguration(InternalCatalogCacheConfig.class); @Test public void testDefaultMetadataCacheProperties() { From 498b1d7ce6542163399436fa53c578c885c64e73 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 18:24:32 -0700 Subject: [PATCH 10/15] Simplify internal catalog cache config Keep the metadata cache properties and cache manager in the tables service so Spring Boot can bind them directly without redundant internal catalog config types. Made-with: Cursor --- .../cluster/configs/ClusterProperties.java | 6 --- .../catalog/InternalCatalogProperties.java | 16 ------- .../cache/SpringTableMetadataCache.java | 14 +++--- .../catalog/cache/TableMetadataCaches.java | 9 ---- .../openhouse/tables/config/Config.java | 45 +++++++++++++++++++ .../config/InternalCatalogCacheConfig.java | 32 ------------- ...ogCacheConfigTest.java => ConfigTest.java} | 24 +++++----- 7 files changed, 63 insertions(+), 83 deletions(-) delete mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java delete mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java create mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java delete mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java rename services/tables/src/test/java/com/linkedin/openhouse/tables/config/{InternalCatalogCacheConfigTest.java => ConfigTest.java} (62%) diff --git a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java index 7e919b02b..9d0c81876 100644 --- a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java +++ b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java @@ -49,12 +49,6 @@ public class ClusterProperties { @Value("${cluster.iceberg.write.metadata.previous-versions-max:100}") private int clusterIcebergWriteMetadataPreviousVersionsMax; - @Value("${cluster.iceberg.metadata-cache.ttl:5m}") - private String clusterIcebergMetadataCacheTtl; - - @Value("${cluster.iceberg.metadata-cache.max-size:1000}") - private int clusterIcebergMetadataCacheMaxSize; - @Value("${cluster.housetables.base-uri:#{null}}") private String clusterHouseTablesBaseUri; diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java deleted file mode 100644 index e094a2d75..000000000 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/InternalCatalogProperties.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.linkedin.openhouse.internal.catalog; - -import java.time.Duration; -import lombok.Getter; -import lombok.Setter; -import org.springframework.boot.context.properties.ConfigurationProperties; - -/** Typed configuration for internal catalog metadata caching. */ -@Getter -@Setter -@ConfigurationProperties(prefix = "cluster.iceberg.metadata-cache") -public class InternalCatalogProperties { - - private Duration ttl = Duration.ofMinutes(5); - private long maxSize = 1000; -} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java index f2eb027e3..f29504d32 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/SpringTableMetadataCache.java @@ -2,25 +2,27 @@ import java.util.function.Supplier; import org.apache.iceberg.TableMetadata; -import org.springframework.cache.annotation.CacheConfig; import org.springframework.cache.annotation.CachePut; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Component; @Component -@CacheConfig( - cacheManager = TableMetadataCaches.CACHE_MANAGER, - cacheNames = TableMetadataCaches.TABLE_METADATA) public class SpringTableMetadataCache implements TableMetadataCache { @Override - @Cacheable(key = "#metadataLocation") + @Cacheable( + cacheManager = "internalCatalogCacheManager", + cacheNames = "tableMetadata", + key = "#metadataLocation") public TableMetadata load(String metadataLocation, Supplier metadataLoader) { return metadataLoader.get(); } @Override - @CachePut(key = "#metadataLocation") + @CachePut( + cacheManager = "internalCatalogCacheManager", + cacheNames = "tableMetadata", + key = "#metadataLocation") public TableMetadata seed(String metadataLocation, TableMetadata tableMetadata) { return tableMetadata; } diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java deleted file mode 100644 index 35312d5ea..000000000 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/TableMetadataCaches.java +++ /dev/null @@ -1,9 +0,0 @@ -package com.linkedin.openhouse.internal.catalog.cache; - -public final class TableMetadataCaches { - - public static final String CACHE_MANAGER = "internalCatalogCacheManager"; - public static final String TABLE_METADATA = "tableMetadata"; - - private TableMetadataCaches() {} -} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java new file mode 100644 index 000000000..239045281 --- /dev/null +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java @@ -0,0 +1,45 @@ +package com.linkedin.openhouse.tables.config; + +import com.github.benmanes.caffeine.cache.Caffeine; +import java.time.Duration; +import java.util.List; +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cache.CacheManager; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.caffeine.CaffeineCacheManager; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +@EnableCaching +@EnableConfigurationProperties +@ConfigurationProperties(prefix = "cluster.iceberg.tables") +@Getter +@Setter +public class Config { + + private MetadataCacheProperties metadataCache = new MetadataCacheProperties(); + + @Bean(name = "internalCatalogCacheManager") + public CacheManager internalCatalogCacheManager() { + CaffeineCacheManager cacheManager = new CaffeineCacheManager(); + cacheManager.setAllowNullValues(false); + cacheManager.setCacheNames(List.of("tableMetadata")); + cacheManager.setCaffeine( + Caffeine.newBuilder() + .expireAfterWrite(metadataCache.getTtl()) + .maximumSize(metadataCache.getMaxSize()) + .recordStats()); + return cacheManager; + } + + @Getter + @Setter + public static class MetadataCacheProperties { + private Duration ttl = Duration.ofMinutes(5); + private long maxSize = 1000; + } +} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java deleted file mode 100644 index 6e1d93484..000000000 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfig.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.linkedin.openhouse.tables.config; - -import com.github.benmanes.caffeine.cache.Caffeine; -import com.linkedin.openhouse.internal.catalog.InternalCatalogProperties; -import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; -import java.util.List; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.cache.CacheManager; -import org.springframework.cache.annotation.EnableCaching; -import org.springframework.cache.caffeine.CaffeineCacheManager; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -@EnableCaching -@EnableConfigurationProperties(InternalCatalogProperties.class) -public class InternalCatalogCacheConfig { - - @Bean(name = TableMetadataCaches.CACHE_MANAGER) - public CacheManager internalCatalogCacheManager( - InternalCatalogProperties internalCatalogProperties) { - CaffeineCacheManager cacheManager = new CaffeineCacheManager(); - cacheManager.setAllowNullValues(false); - cacheManager.setCacheNames(List.of(TableMetadataCaches.TABLE_METADATA)); - cacheManager.setCaffeine( - Caffeine.newBuilder() - .expireAfterWrite(internalCatalogProperties.getTtl()) - .maximumSize(internalCatalogProperties.getMaxSize()) - .recordStats()); - return cacheManager; - } -} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java similarity index 62% rename from services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java rename to services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java index e658850cf..c106372a5 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCacheConfigTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java @@ -1,7 +1,5 @@ package com.linkedin.openhouse.tables.config; -import com.linkedin.openhouse.internal.catalog.InternalCatalogProperties; -import com.linkedin.openhouse.internal.catalog.cache.TableMetadataCaches; import java.time.Duration; import java.util.List; import java.util.concurrent.TimeUnit; @@ -12,10 +10,10 @@ import org.springframework.cache.caffeine.CaffeineCache; import org.springframework.cache.caffeine.CaffeineCacheManager; -class InternalCatalogCacheConfigTest { +class ConfigTest { private final ApplicationContextRunner contextRunner = - new ApplicationContextRunner().withUserConfiguration(InternalCatalogCacheConfig.class); + new ApplicationContextRunner().withUserConfiguration(Config.class); @Test public void testDefaultMetadataCacheProperties() { @@ -27,7 +25,8 @@ public void testDefaultMetadataCacheProperties() { public void testOverriddenMetadataCacheProperties() { contextRunner .withPropertyValues( - "cluster.iceberg.metadata-cache.ttl=7m", "cluster.iceberg.metadata-cache.max-size=42") + "cluster.iceberg.tables.metadata-cache.ttl=7m", + "cluster.iceberg.tables.metadata-cache.max-size=42") .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42)); } @@ -35,18 +34,15 @@ private void assertMetadataCacheConfiguration( AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { Assertions.assertNull(context.getStartupFailure()); - InternalCatalogProperties internalCatalogProperties = - context.getBean(InternalCatalogProperties.class); - Assertions.assertEquals(expectedTtl, internalCatalogProperties.getTtl()); - Assertions.assertEquals(expectedMaxSize, internalCatalogProperties.getMaxSize()); + Config config = context.getBean(Config.class); + Assertions.assertEquals(expectedTtl, config.getMetadataCache().getTtl()); + Assertions.assertEquals(expectedMaxSize, config.getMetadataCache().getMaxSize()); CaffeineCacheManager cacheManager = - context.getBean(TableMetadataCaches.CACHE_MANAGER, CaffeineCacheManager.class); - Assertions.assertEquals( - List.of(TableMetadataCaches.TABLE_METADATA), List.copyOf(cacheManager.getCacheNames())); + context.getBean("internalCatalogCacheManager", CaffeineCacheManager.class); + Assertions.assertEquals(List.of("tableMetadata"), List.copyOf(cacheManager.getCacheNames())); - CaffeineCache tableMetadataCache = - (CaffeineCache) cacheManager.getCache(TableMetadataCaches.TABLE_METADATA); + CaffeineCache tableMetadataCache = (CaffeineCache) cacheManager.getCache("tableMetadata"); Assertions.assertNotNull(tableMetadataCache); com.github.benmanes.caffeine.cache.Cache nativeCache = From 34bf955016654499897a682781064c6b5f1b9606 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 18:51:44 -0700 Subject: [PATCH 11/15] Decouple internal catalog cache binding from behavior Keep the tables metadata-cache namespace owned by the deployable while leaving cache configuration and tests in internalcatalog. Made-with: Cursor --- .../openhouse/internalcatalog/build.gradle | 2 + .../cache/InternalCatalogCacheConfig.java | 27 +++++++++++ .../cache/InternalCatalogCacheProperties.java | 13 ++++++ .../cache/InternalCatalogCacheConfigTest.java | 34 ++++++++------ services/tables/build.gradle | 2 - .../openhouse/tables/config/Config.java | 45 ------------------- .../InternalCatalogCachePropertiesConfig.java | 18 ++++++++ ...ernalCatalogCachePropertiesConfigTest.java | 41 +++++++++++++++++ 8 files changed, 122 insertions(+), 60 deletions(-) create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java rename services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java => iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java (58%) delete mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java create mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java create mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java diff --git a/iceberg/openhouse/internalcatalog/build.gradle b/iceberg/openhouse/internalcatalog/build.gradle index b1e5c44c4..0537754f2 100644 --- a/iceberg/openhouse/internalcatalog/build.gradle +++ b/iceberg/openhouse/internalcatalog/build.gradle @@ -11,7 +11,9 @@ plugins { dependencies { implementation 'com.github.spotbugs:spotbugs-annotations:4.8.1' + implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' api 'org.springframework.retry:spring-retry:1.3.3' + implementation 'org.springframework:spring-context-support:5.3.18' compileOnly "io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:${otel_annotations_version}" api 'io.opentelemetry:opentelemetry-api:1.47.0' api project(':client:hts') diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java new file mode 100644 index 000000000..2c26b38cf --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java @@ -0,0 +1,27 @@ +package com.linkedin.openhouse.internal.catalog.cache; + +import com.github.benmanes.caffeine.cache.Caffeine; +import java.util.List; +import org.springframework.cache.CacheManager; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.caffeine.CaffeineCacheManager; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +@EnableCaching +public class InternalCatalogCacheConfig { + + @Bean(name = "internalCatalogCacheManager") + public CacheManager internalCatalogCacheManager(InternalCatalogCacheProperties cacheProperties) { + CaffeineCacheManager cacheManager = new CaffeineCacheManager(); + cacheManager.setAllowNullValues(false); + cacheManager.setCacheNames(List.of("tableMetadata")); + cacheManager.setCaffeine( + Caffeine.newBuilder() + .expireAfterWrite(cacheProperties.getTtl()) + .maximumSize(cacheProperties.getMaxSize()) + .recordStats()); + return cacheManager; + } +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java new file mode 100644 index 000000000..3b649bab1 --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java @@ -0,0 +1,13 @@ +package com.linkedin.openhouse.internal.catalog.cache; + +import java.time.Duration; +import lombok.Getter; +import lombok.Setter; + +@Getter +@Setter +public class InternalCatalogCacheProperties { + + private Duration ttl = Duration.ofMinutes(5); + private long maxSize = 1000; +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java similarity index 58% rename from services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java rename to iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java index c106372a5..791dc0756 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/ConfigTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java @@ -1,4 +1,4 @@ -package com.linkedin.openhouse.tables.config; +package com.linkedin.openhouse.internal.catalog.cache; import java.time.Duration; import java.util.List; @@ -10,23 +10,29 @@ import org.springframework.cache.caffeine.CaffeineCache; import org.springframework.cache.caffeine.CaffeineCacheManager; -class ConfigTest { +class InternalCatalogCacheConfigTest { private final ApplicationContextRunner contextRunner = - new ApplicationContextRunner().withUserConfiguration(Config.class); + new ApplicationContextRunner().withUserConfiguration(InternalCatalogCacheConfig.class); @Test - public void testDefaultMetadataCacheProperties() { - contextRunner.run( - context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(5), 1000)); + public void testDefaultMetadataCacheConfiguration() { + contextRunner + .withBean(InternalCatalogCacheProperties.class, InternalCatalogCacheProperties::new) + .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(5), 1000)); } @Test - public void testOverriddenMetadataCacheProperties() { + public void testConfiguredMetadataCacheConfiguration() { contextRunner - .withPropertyValues( - "cluster.iceberg.tables.metadata-cache.ttl=7m", - "cluster.iceberg.tables.metadata-cache.max-size=42") + .withBean( + InternalCatalogCacheProperties.class, + () -> { + InternalCatalogCacheProperties cacheProperties = new InternalCatalogCacheProperties(); + cacheProperties.setTtl(Duration.ofMinutes(7)); + cacheProperties.setMaxSize(42); + return cacheProperties; + }) .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42)); } @@ -34,12 +40,14 @@ private void assertMetadataCacheConfiguration( AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { Assertions.assertNull(context.getStartupFailure()); - Config config = context.getBean(Config.class); - Assertions.assertEquals(expectedTtl, config.getMetadataCache().getTtl()); - Assertions.assertEquals(expectedMaxSize, config.getMetadataCache().getMaxSize()); + InternalCatalogCacheProperties cacheProperties = + context.getBean(InternalCatalogCacheProperties.class); + Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); + Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); CaffeineCacheManager cacheManager = context.getBean("internalCatalogCacheManager", CaffeineCacheManager.class); + Assertions.assertFalse(cacheManager.isAllowNullValues()); Assertions.assertEquals(List.of("tableMetadata"), List.copyOf(cacheManager.getCacheNames())); CaffeineCache tableMetadataCache = (CaffeineCache) cacheManager.getCache("tableMetadata"); diff --git a/services/tables/build.gradle b/services/tables/build.gradle index 896b661a1..daadd8297 100644 --- a/services/tables/build.gradle +++ b/services/tables/build.gradle @@ -40,8 +40,6 @@ dependencies { api project(':cluster:metrics') api 'org.springframework.security:spring-security-config:5.7.2' api 'org.springframework.boot:spring-boot-starter-webflux:2.7.8' - implementation 'org.springframework:spring-context-support:5.3.18' - implementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' implementation 'com.cronutils:cron-utils:9.2.0' testImplementation 'org.junit.jupiter:junit-jupiter-engine:' + junit_version testImplementation 'org.springframework.security:spring-security-test:5.7.3' diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java deleted file mode 100644 index 239045281..000000000 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/Config.java +++ /dev/null @@ -1,45 +0,0 @@ -package com.linkedin.openhouse.tables.config; - -import com.github.benmanes.caffeine.cache.Caffeine; -import java.time.Duration; -import java.util.List; -import lombok.Getter; -import lombok.Setter; -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.cache.CacheManager; -import org.springframework.cache.annotation.EnableCaching; -import org.springframework.cache.caffeine.CaffeineCacheManager; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -@EnableCaching -@EnableConfigurationProperties -@ConfigurationProperties(prefix = "cluster.iceberg.tables") -@Getter -@Setter -public class Config { - - private MetadataCacheProperties metadataCache = new MetadataCacheProperties(); - - @Bean(name = "internalCatalogCacheManager") - public CacheManager internalCatalogCacheManager() { - CaffeineCacheManager cacheManager = new CaffeineCacheManager(); - cacheManager.setAllowNullValues(false); - cacheManager.setCacheNames(List.of("tableMetadata")); - cacheManager.setCaffeine( - Caffeine.newBuilder() - .expireAfterWrite(metadataCache.getTtl()) - .maximumSize(metadataCache.getMaxSize()) - .recordStats()); - return cacheManager; - } - - @Getter - @Setter - public static class MetadataCacheProperties { - private Duration ttl = Duration.ofMinutes(5); - private long maxSize = 1000; - } -} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java new file mode 100644 index 000000000..745f4e361 --- /dev/null +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java @@ -0,0 +1,18 @@ +package com.linkedin.openhouse.tables.config; + +import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +@EnableConfigurationProperties +public class InternalCatalogCachePropertiesConfig { + + @Bean + @ConfigurationProperties(prefix = "cluster.iceberg.tables.metadata-cache") + public InternalCatalogCacheProperties internalCatalogCacheProperties() { + return new InternalCatalogCacheProperties(); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java new file mode 100644 index 000000000..cae0ae92b --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java @@ -0,0 +1,41 @@ +package com.linkedin.openhouse.tables.config; + +import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; +import java.time.Duration; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +class InternalCatalogCachePropertiesConfigTest { + + private final ApplicationContextRunner contextRunner = + new ApplicationContextRunner() + .withUserConfiguration(InternalCatalogCachePropertiesConfig.class); + + @Test + public void testDefaultMetadataCacheProperties() { + contextRunner.run( + context -> assertMetadataCacheProperties(context, Duration.ofMinutes(5), 1000)); + } + + @Test + public void testOverriddenMetadataCacheProperties() { + contextRunner + .withPropertyValues( + "cluster.iceberg.tables.metadata-cache.ttl=7m", + "cluster.iceberg.tables.metadata-cache.max-size=42") + .run(context -> assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42)); + } + + private void assertMetadataCacheProperties( + AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { + Assertions.assertNull(context.getStartupFailure()); + + InternalCatalogCacheProperties cacheProperties = + context.getBean(InternalCatalogCacheProperties.class); + Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); + Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); + Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + } +} From 303470a8668c6212059a87fc157e6328cdcede32 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sat, 4 Apr 2026 22:02:44 -0700 Subject: [PATCH 12/15] Add cross-module metadata cache propagation test Verify the tables-service metadata cache properties configure the internal catalog Caffeine cache end to end, and add the test-only dependencies needed to assert that wiring directly. Made-with: Cursor --- services/tables/build.gradle | 2 + ...ernalCatalogCachePropertiesConfigTest.java | 58 ++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/services/tables/build.gradle b/services/tables/build.gradle index daadd8297..c1b91e175 100644 --- a/services/tables/build.gradle +++ b/services/tables/build.gradle @@ -43,6 +43,8 @@ dependencies { implementation 'com.cronutils:cron-utils:9.2.0' testImplementation 'org.junit.jupiter:junit-jupiter-engine:' + junit_version testImplementation 'org.springframework.security:spring-security-test:5.7.3' + testImplementation 'org.springframework:spring-context-support:5.3.18' + testImplementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' testImplementation(testFixtures(project(':services:common'))) testImplementation (project(':tables-test-fixtures:tables-test-fixtures_2.12')) { exclude group: 'com.linkedin.iceberg' diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java index cae0ae92b..0f80b1889 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java @@ -1,11 +1,16 @@ package com.linkedin.openhouse.tables.config; +import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheConfig; import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cache.caffeine.CaffeineCache; +import org.springframework.cache.caffeine.CaffeineCacheManager; class InternalCatalogCachePropertiesConfigTest { @@ -13,10 +18,18 @@ class InternalCatalogCachePropertiesConfigTest { new ApplicationContextRunner() .withUserConfiguration(InternalCatalogCachePropertiesConfig.class); + private final ApplicationContextRunner crossModuleContextRunner = + new ApplicationContextRunner() + .withUserConfiguration( + InternalCatalogCachePropertiesConfig.class, InternalCatalogCacheConfig.class); + @Test public void testDefaultMetadataCacheProperties() { contextRunner.run( - context -> assertMetadataCacheProperties(context, Duration.ofMinutes(5), 1000)); + context -> { + assertMetadataCacheProperties(context, Duration.ofMinutes(5), 1000); + Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + }); } @Test @@ -25,7 +38,24 @@ public void testOverriddenMetadataCacheProperties() { .withPropertyValues( "cluster.iceberg.tables.metadata-cache.ttl=7m", "cluster.iceberg.tables.metadata-cache.max-size=42") - .run(context -> assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42)); + .run( + context -> { + assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42); + Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + }); + } + + @Test + public void testMetadataCachePropertiesPropagateToInternalCatalogCacheConfiguration() { + crossModuleContextRunner + .withPropertyValues( + "cluster.iceberg.tables.metadata-cache.ttl=7m", + "cluster.iceberg.tables.metadata-cache.max-size=42") + .run( + context -> { + assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42); + assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42); + }); } private void assertMetadataCacheProperties( @@ -36,6 +66,28 @@ private void assertMetadataCacheProperties( context.getBean(InternalCatalogCacheProperties.class); Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); - Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + } + + private void assertMetadataCacheConfiguration( + AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { + CaffeineCacheManager cacheManager = + context.getBean("internalCatalogCacheManager", CaffeineCacheManager.class); + Assertions.assertFalse(cacheManager.isAllowNullValues()); + Assertions.assertEquals(List.of("tableMetadata"), List.copyOf(cacheManager.getCacheNames())); + + CaffeineCache tableMetadataCache = (CaffeineCache) cacheManager.getCache("tableMetadata"); + Assertions.assertNotNull(tableMetadataCache); + + com.github.benmanes.caffeine.cache.Cache nativeCache = + tableMetadataCache.getNativeCache(); + Assertions.assertEquals( + expectedTtl.toNanos(), + nativeCache + .policy() + .expireAfterWrite() + .orElseThrow() + .getExpiresAfter(TimeUnit.NANOSECONDS)); + Assertions.assertEquals( + expectedMaxSize, nativeCache.policy().eviction().orElseThrow().getMaximum()); } } From d7f1e80b59a489efa34c74305072504b53a229a5 Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sun, 5 Apr 2026 15:42:36 -0700 Subject: [PATCH 13/15] Tighten internal catalog cache test boundary Keep services/tables focused on generic cache property propagation and move Caffeine-specific verification into internalcatalog so backend changes stay local to that module. Made-with: Cursor --- .../cache/InternalCatalogCacheProperties.java | 4 ++ .../cache/InternalCatalogCacheConfigTest.java | 47 ++++++++++++++++++- services/tables/build.gradle | 1 - ...ernalCatalogCachePropertiesConfigTest.java | 30 +----------- 4 files changed, 52 insertions(+), 30 deletions(-) diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java index 3b649bab1..badcc12dd 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java @@ -4,6 +4,10 @@ import lombok.Getter; import lombok.Setter; +/** + * Stable cache-settings contract between deployable-owned configuration and internal catalog cache + * wiring. + */ @Getter @Setter public class InternalCatalogCacheProperties { diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java index 791dc0756..ad4292e05 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java @@ -3,8 +3,11 @@ import java.time.Duration; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.iceberg.TableMetadata; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.cache.caffeine.CaffeineCache; @@ -15,6 +18,11 @@ class InternalCatalogCacheConfigTest { private final ApplicationContextRunner contextRunner = new ApplicationContextRunner().withUserConfiguration(InternalCatalogCacheConfig.class); + private final ApplicationContextRunner tableMetadataCacheContextRunner = + new ApplicationContextRunner() + .withUserConfiguration(InternalCatalogCacheConfig.class) + .withBean(SpringTableMetadataCache.class, SpringTableMetadataCache::new); + @Test public void testDefaultMetadataCacheConfiguration() { contextRunner @@ -36,7 +44,43 @@ public void testConfiguredMetadataCacheConfiguration() { .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42)); } - private void assertMetadataCacheConfiguration( + @Test + public void testSpringTableMetadataCacheUsesConfiguredTableMetadataCache() { + tableMetadataCacheContextRunner + .withBean( + InternalCatalogCacheProperties.class, + () -> { + InternalCatalogCacheProperties cacheProperties = new InternalCatalogCacheProperties(); + cacheProperties.setTtl(Duration.ofMinutes(7)); + cacheProperties.setMaxSize(42); + return cacheProperties; + }) + .run( + context -> { + CaffeineCache tableMetadataCache = + assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42); + TableMetadataCache cache = context.getBean(TableMetadataCache.class); + String metadataLocation = "metadata-location"; + TableMetadata seededMetadata = Mockito.mock(TableMetadata.class); + AtomicInteger loadCount = new AtomicInteger(); + + cache.seed(metadataLocation, seededMetadata); + TableMetadata loadedMetadata = + cache.load( + metadataLocation, + () -> { + loadCount.incrementAndGet(); + return Mockito.mock(TableMetadata.class); + }); + + Assertions.assertSame(seededMetadata, loadedMetadata); + Assertions.assertEquals(0, loadCount.get()); + Assertions.assertSame( + seededMetadata, tableMetadataCache.get(metadataLocation, TableMetadata.class)); + }); + } + + private CaffeineCache assertMetadataCacheConfiguration( AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { Assertions.assertNull(context.getStartupFailure()); @@ -64,5 +108,6 @@ private void assertMetadataCacheConfiguration( .getExpiresAfter(TimeUnit.NANOSECONDS)); Assertions.assertEquals( expectedMaxSize, nativeCache.policy().eviction().orElseThrow().getMaximum()); + return tableMetadataCache; } } diff --git a/services/tables/build.gradle b/services/tables/build.gradle index c1b91e175..271d8951e 100644 --- a/services/tables/build.gradle +++ b/services/tables/build.gradle @@ -44,7 +44,6 @@ dependencies { testImplementation 'org.junit.jupiter:junit-jupiter-engine:' + junit_version testImplementation 'org.springframework.security:spring-security-test:5.7.3' testImplementation 'org.springframework:spring-context-support:5.3.18' - testImplementation 'com.github.ben-manes.caffeine:caffeine:2.8.8' testImplementation(testFixtures(project(':services:common'))) testImplementation (project(':tables-test-fixtures:tables-test-fixtures_2.12')) { exclude group: 'com.linkedin.iceberg' diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java index 0f80b1889..aa815191e 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java @@ -3,14 +3,11 @@ import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheConfig; import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; import java.time.Duration; -import java.util.List; -import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; -import org.springframework.cache.caffeine.CaffeineCache; -import org.springframework.cache.caffeine.CaffeineCacheManager; +import org.springframework.cache.CacheManager; class InternalCatalogCachePropertiesConfigTest { @@ -54,7 +51,7 @@ public void testMetadataCachePropertiesPropagateToInternalCatalogCacheConfigurat .run( context -> { assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42); - assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42); + Assertions.assertNotNull(context.getBean(CacheManager.class)); }); } @@ -67,27 +64,4 @@ private void assertMetadataCacheProperties( Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); } - - private void assertMetadataCacheConfiguration( - AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { - CaffeineCacheManager cacheManager = - context.getBean("internalCatalogCacheManager", CaffeineCacheManager.class); - Assertions.assertFalse(cacheManager.isAllowNullValues()); - Assertions.assertEquals(List.of("tableMetadata"), List.copyOf(cacheManager.getCacheNames())); - - CaffeineCache tableMetadataCache = (CaffeineCache) cacheManager.getCache("tableMetadata"); - Assertions.assertNotNull(tableMetadataCache); - - com.github.benmanes.caffeine.cache.Cache nativeCache = - tableMetadataCache.getNativeCache(); - Assertions.assertEquals( - expectedTtl.toNanos(), - nativeCache - .policy() - .expireAfterWrite() - .orElseThrow() - .getExpiresAfter(TimeUnit.NANOSECONDS)); - Assertions.assertEquals( - expectedMaxSize, nativeCache.policy().eviction().orElseThrow().getMaximum()); - } } From 622850b625a5b2492a42af795564e0f43711d65c Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sun, 5 Apr 2026 20:32:42 -0700 Subject: [PATCH 14/15] Clarify internal catalog config naming Differentiate raw Spring-bound properties, module settings, and cache bean wiring so the tables app boundary and internalcatalog runtime config are easier to follow. Made-with: Cursor --- ...cheConfig.java => CacheConfiguration.java} | 16 +++- .../cache/InternalCatalogCacheProperties.java | 17 ----- .../config/InternalCatalogSettings.java | 19 +++++ ...gTest.java => CacheConfigurationTest.java} | 41 +++++----- .../tables/config/InternalCatalogBeans.java | 28 +++++++ .../InternalCatalogCachePropertiesConfig.java | 18 ----- .../config/InternalCatalogProperties.java | 21 +++++ .../config/InternalCatalogBeansTest.java | 76 +++++++++++++++++++ ...ernalCatalogCachePropertiesConfigTest.java | 67 ---------------- 9 files changed, 173 insertions(+), 130 deletions(-) rename iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/{InternalCatalogCacheConfig.java => CacheConfiguration.java} (61%) delete mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java create mode 100644 iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/config/InternalCatalogSettings.java rename iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/{InternalCatalogCacheConfigTest.java => CacheConfigurationTest.java} (74%) create mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogBeans.java delete mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java create mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogProperties.java create mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogBeansTest.java delete mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java similarity index 61% rename from iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java rename to iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java index 2c26b38cf..9fa9ee0f7 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfig.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java @@ -1,7 +1,9 @@ package com.linkedin.openhouse.internal.catalog.cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.linkedin.openhouse.internal.catalog.config.InternalCatalogSettings; import java.util.List; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.EnableCaching; import org.springframework.cache.caffeine.CaffeineCacheManager; @@ -10,17 +12,23 @@ @Configuration @EnableCaching -public class InternalCatalogCacheConfig { +public class CacheConfiguration { + + @Bean + @ConditionalOnMissingBean(InternalCatalogSettings.class) + public InternalCatalogSettings internalCatalogSettings() { + return new InternalCatalogSettings(); + } @Bean(name = "internalCatalogCacheManager") - public CacheManager internalCatalogCacheManager(InternalCatalogCacheProperties cacheProperties) { + public CacheManager internalCatalogCacheManager(InternalCatalogSettings settings) { CaffeineCacheManager cacheManager = new CaffeineCacheManager(); cacheManager.setAllowNullValues(false); cacheManager.setCacheNames(List.of("tableMetadata")); cacheManager.setCaffeine( Caffeine.newBuilder() - .expireAfterWrite(cacheProperties.getTtl()) - .maximumSize(cacheProperties.getMaxSize()) + .expireAfterWrite(settings.getMetadataCache().getTtl()) + .maximumSize(settings.getMetadataCache().getMaxSize()) .recordStats()); return cacheManager; } diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java deleted file mode 100644 index badcc12dd..000000000 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheProperties.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.linkedin.openhouse.internal.catalog.cache; - -import java.time.Duration; -import lombok.Getter; -import lombok.Setter; - -/** - * Stable cache-settings contract between deployable-owned configuration and internal catalog cache - * wiring. - */ -@Getter -@Setter -public class InternalCatalogCacheProperties { - - private Duration ttl = Duration.ofMinutes(5); - private long maxSize = 1000; -} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/config/InternalCatalogSettings.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/config/InternalCatalogSettings.java new file mode 100644 index 000000000..078a778ac --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/config/InternalCatalogSettings.java @@ -0,0 +1,19 @@ +package com.linkedin.openhouse.internal.catalog.config; + +import java.time.Duration; +import lombok.Getter; +import lombok.Setter; + +@Getter +@Setter +public class InternalCatalogSettings { + + private MetadataCache metadataCache = new MetadataCache(); + + @Getter + @Setter + public static class MetadataCache { + private Duration ttl = Duration.ofMinutes(5); + private long maxSize = 1000; + } +} diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfigurationTest.java similarity index 74% rename from iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java rename to iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfigurationTest.java index ad4292e05..ff7f7d400 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/InternalCatalogCacheConfigTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfigurationTest.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.internal.catalog.cache; +import com.linkedin.openhouse.internal.catalog.config.InternalCatalogSettings; import java.time.Duration; import java.util.List; import java.util.concurrent.TimeUnit; @@ -13,48 +14,34 @@ import org.springframework.cache.caffeine.CaffeineCache; import org.springframework.cache.caffeine.CaffeineCacheManager; -class InternalCatalogCacheConfigTest { +class CacheConfigurationTest { private final ApplicationContextRunner contextRunner = - new ApplicationContextRunner().withUserConfiguration(InternalCatalogCacheConfig.class); + new ApplicationContextRunner().withUserConfiguration(CacheConfiguration.class); private final ApplicationContextRunner tableMetadataCacheContextRunner = new ApplicationContextRunner() - .withUserConfiguration(InternalCatalogCacheConfig.class) + .withUserConfiguration(CacheConfiguration.class) .withBean(SpringTableMetadataCache.class, SpringTableMetadataCache::new); @Test public void testDefaultMetadataCacheConfiguration() { contextRunner - .withBean(InternalCatalogCacheProperties.class, InternalCatalogCacheProperties::new) + .withBean(InternalCatalogSettings.class, InternalCatalogSettings::new) .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(5), 1000)); } @Test public void testConfiguredMetadataCacheConfiguration() { contextRunner - .withBean( - InternalCatalogCacheProperties.class, - () -> { - InternalCatalogCacheProperties cacheProperties = new InternalCatalogCacheProperties(); - cacheProperties.setTtl(Duration.ofMinutes(7)); - cacheProperties.setMaxSize(42); - return cacheProperties; - }) + .withBean(InternalCatalogSettings.class, () -> buildSettings(Duration.ofMinutes(7), 42)) .run(context -> assertMetadataCacheConfiguration(context, Duration.ofMinutes(7), 42)); } @Test public void testSpringTableMetadataCacheUsesConfiguredTableMetadataCache() { tableMetadataCacheContextRunner - .withBean( - InternalCatalogCacheProperties.class, - () -> { - InternalCatalogCacheProperties cacheProperties = new InternalCatalogCacheProperties(); - cacheProperties.setTtl(Duration.ofMinutes(7)); - cacheProperties.setMaxSize(42); - return cacheProperties; - }) + .withBean(InternalCatalogSettings.class, () -> buildSettings(Duration.ofMinutes(7), 42)) .run( context -> { CaffeineCache tableMetadataCache = @@ -80,14 +67,20 @@ public void testSpringTableMetadataCacheUsesConfiguredTableMetadataCache() { }); } + private InternalCatalogSettings buildSettings(Duration ttl, long maxSize) { + InternalCatalogSettings settings = new InternalCatalogSettings(); + settings.getMetadataCache().setTtl(ttl); + settings.getMetadataCache().setMaxSize(maxSize); + return settings; + } + private CaffeineCache assertMetadataCacheConfiguration( AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { Assertions.assertNull(context.getStartupFailure()); - InternalCatalogCacheProperties cacheProperties = - context.getBean(InternalCatalogCacheProperties.class); - Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); - Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); + InternalCatalogSettings settings = context.getBean(InternalCatalogSettings.class); + Assertions.assertEquals(expectedTtl, settings.getMetadataCache().getTtl()); + Assertions.assertEquals(expectedMaxSize, settings.getMetadataCache().getMaxSize()); CaffeineCacheManager cacheManager = context.getBean("internalCatalogCacheManager", CaffeineCacheManager.class); diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogBeans.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogBeans.java new file mode 100644 index 000000000..df4c5f20d --- /dev/null +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogBeans.java @@ -0,0 +1,28 @@ +package com.linkedin.openhouse.tables.config; + +import com.linkedin.openhouse.internal.catalog.config.InternalCatalogSettings; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +@EnableConfigurationProperties(InternalCatalogProperties.class) +public class InternalCatalogBeans { + + @Bean + public InternalCatalogSettings internalCatalogSettings(InternalCatalogProperties properties) { + InternalCatalogSettings settings = new InternalCatalogSettings(); + InternalCatalogProperties.MetadataCache metadataCacheOverrides = properties.getMetadataCache(); + + if (metadataCacheOverrides != null) { + if (metadataCacheOverrides.getTtl() != null) { + settings.getMetadataCache().setTtl(metadataCacheOverrides.getTtl()); + } + if (metadataCacheOverrides.getMaxSize() != null) { + settings.getMetadataCache().setMaxSize(metadataCacheOverrides.getMaxSize()); + } + } + + return settings; + } +} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java deleted file mode 100644 index 745f4e361..000000000 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfig.java +++ /dev/null @@ -1,18 +0,0 @@ -package com.linkedin.openhouse.tables.config; - -import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -@EnableConfigurationProperties -public class InternalCatalogCachePropertiesConfig { - - @Bean - @ConfigurationProperties(prefix = "cluster.iceberg.tables.metadata-cache") - public InternalCatalogCacheProperties internalCatalogCacheProperties() { - return new InternalCatalogCacheProperties(); - } -} diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogProperties.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogProperties.java new file mode 100644 index 000000000..e5a59d18b --- /dev/null +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogProperties.java @@ -0,0 +1,21 @@ +package com.linkedin.openhouse.tables.config; + +import java.time.Duration; +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; + +@Getter +@Setter +@ConfigurationProperties(prefix = "cluster.iceberg.tables") +public class InternalCatalogProperties { + + private MetadataCache metadataCache = new MetadataCache(); + + @Getter + @Setter + public static class MetadataCache { + private Duration ttl; + private Long maxSize; + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogBeansTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogBeansTest.java new file mode 100644 index 000000000..39e01125a --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogBeansTest.java @@ -0,0 +1,76 @@ +package com.linkedin.openhouse.tables.config; + +import com.linkedin.openhouse.internal.catalog.cache.CacheConfiguration; +import com.linkedin.openhouse.internal.catalog.config.InternalCatalogSettings; +import java.time.Duration; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cache.CacheManager; + +class InternalCatalogBeansTest { + + private final ApplicationContextRunner contextRunner = + new ApplicationContextRunner().withUserConfiguration(InternalCatalogBeans.class); + + private final ApplicationContextRunner crossModuleContextRunner = + new ApplicationContextRunner() + .withUserConfiguration(InternalCatalogBeans.class, CacheConfiguration.class); + + @Test + public void testDefaultInternalCatalogSettings() { + contextRunner.run( + context -> { + assertMetadataCacheOverrides(context, null, null); + assertMetadataCacheSettings(context, Duration.ofMinutes(5), 1000); + Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + }); + } + + @Test + public void testOverriddenInternalCatalogSettings() { + contextRunner + .withPropertyValues( + "cluster.iceberg.tables.metadata-cache.ttl=7m", + "cluster.iceberg.tables.metadata-cache.max-size=42") + .run( + context -> { + assertMetadataCacheOverrides(context, Duration.ofMinutes(7), 42L); + assertMetadataCacheSettings(context, Duration.ofMinutes(7), 42); + Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); + }); + } + + @Test + public void testInternalCatalogSettingsPropagateToCacheConfiguration() { + crossModuleContextRunner + .withPropertyValues( + "cluster.iceberg.tables.metadata-cache.ttl=7m", + "cluster.iceberg.tables.metadata-cache.max-size=42") + .run( + context -> { + assertMetadataCacheOverrides(context, Duration.ofMinutes(7), 42L); + assertMetadataCacheSettings(context, Duration.ofMinutes(7), 42); + Assertions.assertNotNull(context.getBean(CacheManager.class)); + }); + } + + private void assertMetadataCacheOverrides( + AssertableApplicationContext context, Duration expectedTtl, Long expectedMaxSize) { + Assertions.assertNull(context.getStartupFailure()); + + InternalCatalogProperties properties = context.getBean(InternalCatalogProperties.class); + Assertions.assertEquals(expectedTtl, properties.getMetadataCache().getTtl()); + Assertions.assertEquals(expectedMaxSize, properties.getMetadataCache().getMaxSize()); + } + + private void assertMetadataCacheSettings( + AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { + Assertions.assertNull(context.getStartupFailure()); + + InternalCatalogSettings settings = context.getBean(InternalCatalogSettings.class); + Assertions.assertEquals(expectedTtl, settings.getMetadataCache().getTtl()); + Assertions.assertEquals(expectedMaxSize, settings.getMetadataCache().getMaxSize()); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java deleted file mode 100644 index aa815191e..000000000 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/config/InternalCatalogCachePropertiesConfigTest.java +++ /dev/null @@ -1,67 +0,0 @@ -package com.linkedin.openhouse.tables.config; - -import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheConfig; -import com.linkedin.openhouse.internal.catalog.cache.InternalCatalogCacheProperties; -import java.time.Duration; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.springframework.boot.test.context.assertj.AssertableApplicationContext; -import org.springframework.boot.test.context.runner.ApplicationContextRunner; -import org.springframework.cache.CacheManager; - -class InternalCatalogCachePropertiesConfigTest { - - private final ApplicationContextRunner contextRunner = - new ApplicationContextRunner() - .withUserConfiguration(InternalCatalogCachePropertiesConfig.class); - - private final ApplicationContextRunner crossModuleContextRunner = - new ApplicationContextRunner() - .withUserConfiguration( - InternalCatalogCachePropertiesConfig.class, InternalCatalogCacheConfig.class); - - @Test - public void testDefaultMetadataCacheProperties() { - contextRunner.run( - context -> { - assertMetadataCacheProperties(context, Duration.ofMinutes(5), 1000); - Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); - }); - } - - @Test - public void testOverriddenMetadataCacheProperties() { - contextRunner - .withPropertyValues( - "cluster.iceberg.tables.metadata-cache.ttl=7m", - "cluster.iceberg.tables.metadata-cache.max-size=42") - .run( - context -> { - assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42); - Assertions.assertFalse(context.containsBean("internalCatalogCacheManager")); - }); - } - - @Test - public void testMetadataCachePropertiesPropagateToInternalCatalogCacheConfiguration() { - crossModuleContextRunner - .withPropertyValues( - "cluster.iceberg.tables.metadata-cache.ttl=7m", - "cluster.iceberg.tables.metadata-cache.max-size=42") - .run( - context -> { - assertMetadataCacheProperties(context, Duration.ofMinutes(7), 42); - Assertions.assertNotNull(context.getBean(CacheManager.class)); - }); - } - - private void assertMetadataCacheProperties( - AssertableApplicationContext context, Duration expectedTtl, long expectedMaxSize) { - Assertions.assertNull(context.getStartupFailure()); - - InternalCatalogCacheProperties cacheProperties = - context.getBean(InternalCatalogCacheProperties.class); - Assertions.assertEquals(expectedTtl, cacheProperties.getTtl()); - Assertions.assertEquals(expectedMaxSize, cacheProperties.getMaxSize()); - } -} From fbf3a65a5c73c216835a639f55bb6c824e30f66a Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Sun, 5 Apr 2026 20:41:03 -0700 Subject: [PATCH 15/15] Simplify internal catalog cache bean naming Rely on Spring's default method-based bean name for the internal catalog cache manager so the wiring stays explicit in tests without duplicating the same name in configuration. Made-with: Cursor --- .../openhouse/internal/catalog/cache/CacheConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java index 9fa9ee0f7..ca19b44e6 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/cache/CacheConfiguration.java @@ -20,7 +20,7 @@ public InternalCatalogSettings internalCatalogSettings() { return new InternalCatalogSettings(); } - @Bean(name = "internalCatalogCacheManager") + @Bean public CacheManager internalCatalogCacheManager(InternalCatalogSettings settings) { CaffeineCacheManager cacheManager = new CaffeineCacheManager(); cacheManager.setAllowNullValues(false);