Skip to content

Commit eb92413

Browse files
marcelogpinheiromarcelogpinheiro
authored
Merge pull request #1232 from commercetools/bugfix/product-type-cache
Bugfix/product type cache
2 parents a9bf00d + 61c783f commit eb92413

File tree

4 files changed

+236
-11
lines changed

4 files changed

+236
-11
lines changed

‎.github/workflows/cd.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
uses: actions/checkout@v4
2525
- name: Fetch Library version
2626
id: vars
27-
run: echo ::set-output name=libVersion::${GITHUB_REF#refs/*/}
27+
run: echo libVersion=${GITHUB_REF#refs/*/} >> $GITHUB_OUTPUT
2828
- name: benchmark test
2929
if: ${{ success() }}
3030
run: ./gradlew clean setLibraryVersion benchmark
@@ -41,7 +41,7 @@ jobs:
4141
uses: actions/checkout@v4
4242
- name: Set output
4343
id: vars
44-
run: echo ::set-output name=tag::${GITHUB_REF#refs/*/}
44+
run: echo tag=${GITHUB_REF#refs/*/} >> $GITHUB_OUTPUT
4545
- name: Set up Java
4646
uses: actions/setup-java@v4
4747
with:

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
uses: actions/checkout@v4
6161
- name: Fetch Library version
6262
id: vars
63-
run: echo ::set-output name=libVersion::${GITHUB_REF#refs/*/}
63+
run: echo libVersion=${GITHUB_REF#refs/*/} >> $GITHUB_OUTPUT
6464
- name: benchmark test
6565
if: ${{ success() }}
6666
run: ./gradlew clean setLibraryVersion benchmark

‎src/integration-test/java/com/commercetools/sync/integration/services/impl/ProductTypeServiceImplIT.java‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,4 +448,185 @@ void updateProductType_WithValidChanges_ShouldUpdateProductTypeCorrectly() {
448448
assertThat(fetchedProductType.getName()).isEqualTo(updatedProductType.getName());
449449
assertThat(fetchedProductType.getAttributes()).isEqualTo(updatedProductType.getAttributes());
450450
}
451+
452+
/*
453+
* This test verifies the cache stampede fix by making concurrent calls
454+
and ensuring the cache is populated correctly without race conditions.
455+
456+
What this test verifies:
457+
1. All concurrent calls complete successfully (no race conditions)
458+
2. All calls return the same cached data (cache consistency)
459+
3. No exceptions occur during concurrent access
460+
461+
NOTE: The tests were execute with logs and it can be seen that only one query is executed.
462+
*/
463+
@Test
464+
void
465+
fetchCachedProductAttributeMetaDataMap_WithConcurrentCalls_ShouldHandleCacheStampedeCorrectly()
466+
throws Exception {
467+
468+
// preparation - create a product type with attributes
469+
final ProductTypeDraft productTypeDraft =
470+
ProductTypeDraftBuilder.of()
471+
.key("cache-stampede-test-type")
472+
.name("Cache Stampede Test Type")
473+
.description("Test product type for cache stampede fix")
474+
.attributes(ATTRIBUTE_DEFINITION_DRAFT_1)
475+
.build();
476+
477+
final ProductType createdProductType =
478+
CTP_TARGET_CLIENT.productTypes().post(productTypeDraft).execute().join().getBody();
479+
480+
final ProductTypeSyncOptions productTypeSyncOptions =
481+
ProductTypeSyncOptionsBuilder.of(CTP_TARGET_CLIENT).build();
482+
final ProductTypeService productTypeService =
483+
new ProductTypeServiceImpl(productTypeSyncOptions);
484+
485+
// test - make 10 concurrent calls to fetchCachedProductAttributeMetaDataMap
486+
// Used a CountDownLatch to ensure all threads start at approximately the same time
487+
final int numberOfConcurrentCalls = 10;
488+
final java.util.concurrent.CountDownLatch startLatch =
489+
new java.util.concurrent.CountDownLatch(1);
490+
final java.util.concurrent.CountDownLatch readyLatch =
491+
new java.util.concurrent.CountDownLatch(numberOfConcurrentCalls);
492+
final java.util.concurrent.ExecutorService executorService =
493+
java.util.concurrent.Executors.newFixedThreadPool(numberOfConcurrentCalls);
494+
final java.util.List<
495+
java.util.concurrent.CompletableFuture<Optional<Map<String, AttributeMetaData>>>>
496+
futures = new java.util.ArrayList<>();
497+
498+
for (int i = 0; i < numberOfConcurrentCalls; i++) {
499+
final java.util.concurrent.CompletableFuture<Optional<Map<String, AttributeMetaData>>>
500+
future =
501+
java.util.concurrent.CompletableFuture.supplyAsync(
502+
() -> {
503+
try {
504+
readyLatch.countDown();
505+
startLatch.await(); // Wait for all threads to be ready
506+
return productTypeService
507+
.fetchCachedProductAttributeMetaDataMap(createdProductType.getId())
508+
.toCompletableFuture()
509+
.join();
510+
} catch (InterruptedException e) {
511+
Thread.currentThread().interrupt();
512+
throw new RuntimeException(e);
513+
}
514+
},
515+
executorService);
516+
futures.add(future);
517+
}
518+
519+
final boolean allThreadsReady = readyLatch.await(5, java.util.concurrent.TimeUnit.SECONDS);
520+
assertThat(allThreadsReady).as("All threads should be ready within timeout").isTrue();
521+
522+
// Start all threads at once
523+
startLatch.countDown();
524+
525+
// Wait for all futures to complete
526+
java.util.concurrent.CompletableFuture.allOf(
527+
futures.toArray(new java.util.concurrent.CompletableFuture[0]))
528+
.join();
529+
530+
executorService.shutdown();
531+
final boolean executorTerminated =
532+
executorService.awaitTermination(10, java.util.concurrent.TimeUnit.SECONDS);
533+
assertThat(executorTerminated).as("Executor service should terminate within timeout").isTrue();
534+
535+
// assertions - all calls should return the same result
536+
final Optional<Map<String, AttributeMetaData>> firstResult = futures.get(0).join();
537+
assertThat(firstResult).isPresent();
538+
539+
for (java.util.concurrent.CompletableFuture<Optional<Map<String, AttributeMetaData>>> future :
540+
futures) {
541+
assertThat(future).isCompleted();
542+
final Optional<Map<String, AttributeMetaData>> result = future.join();
543+
assertThat(result).isPresent();
544+
assertThat(result.get()).containsKey(ATTRIBUTE_DEFINITION_DRAFT_1.getName());
545+
// Verify all results are identical (same cached instance)
546+
assertThat(result.get()).isEqualTo(firstResult.get());
547+
}
548+
549+
// cleanup
550+
CTP_TARGET_CLIENT
551+
.productTypes()
552+
.withId(createdProductType.getId())
553+
.delete()
554+
.withVersion(createdProductType.getVersion())
555+
.execute()
556+
.join();
557+
}
558+
559+
@Test
560+
void fetchCachedProductAttributeMetaDataMap_WithPopulatedCache_ShouldReturnCachedData() {
561+
// This test verifies that after the first call, subsequent calls use the cache
562+
563+
// preparation - create a product type
564+
final ProductTypeDraft productTypeDraft =
565+
ProductTypeDraftBuilder.of()
566+
.key("cache-reuse-test-type")
567+
.name("Cache Reuse Test Type")
568+
.description("Test product type for cache reuse")
569+
.attributes(ATTRIBUTE_DEFINITION_DRAFT_1, ATTRIBUTE_DEFINITION_DRAFT_2)
570+
.build();
571+
572+
final ProductType createdProductType =
573+
CTP_TARGET_CLIENT.productTypes().post(productTypeDraft).execute().join().getBody();
574+
575+
final ProductTypeSyncOptions productTypeSyncOptions =
576+
ProductTypeSyncOptionsBuilder.of(CTP_TARGET_CLIENT).build();
577+
final ProductTypeService productTypeService =
578+
new ProductTypeServiceImpl(productTypeSyncOptions);
579+
580+
// test - first call to populate cache
581+
final Optional<Map<String, AttributeMetaData>> firstResult =
582+
productTypeService
583+
.fetchCachedProductAttributeMetaDataMap(createdProductType.getId())
584+
.toCompletableFuture()
585+
.join();
586+
587+
// test - second call should use cache
588+
final Optional<Map<String, AttributeMetaData>> secondResult =
589+
productTypeService
590+
.fetchCachedProductAttributeMetaDataMap(createdProductType.getId())
591+
.toCompletableFuture()
592+
.join();
593+
594+
// assertions
595+
assertThat(firstResult).isPresent();
596+
assertThat(secondResult).isPresent();
597+
assertThat(firstResult.get()).isEqualTo(secondResult.get());
598+
assertThat(firstResult.get()).hasSize(2);
599+
assertThat(firstResult.get())
600+
.containsKeys(
601+
ATTRIBUTE_DEFINITION_DRAFT_1.getName(), ATTRIBUTE_DEFINITION_DRAFT_2.getName());
602+
603+
// cleanup
604+
CTP_TARGET_CLIENT
605+
.productTypes()
606+
.withId(createdProductType.getId())
607+
.delete()
608+
.withVersion(createdProductType.getVersion())
609+
.execute()
610+
.join();
611+
}
612+
613+
@Test
614+
void
615+
fetchCachedProductAttributeMetaDataMap_WithNonExistentProductType_ShouldReturnEmptyOptional() {
616+
// preparation
617+
final ProductTypeSyncOptions productTypeSyncOptions =
618+
ProductTypeSyncOptionsBuilder.of(CTP_TARGET_CLIENT).build();
619+
final ProductTypeService productTypeService =
620+
new ProductTypeServiceImpl(productTypeSyncOptions);
621+
622+
// test - query for non-existent product type ID
623+
final Optional<Map<String, AttributeMetaData>> result =
624+
productTypeService
625+
.fetchCachedProductAttributeMetaDataMap("non-existent-id-12345")
626+
.toCompletableFuture()
627+
.join();
628+
629+
// assertions
630+
assertThat(result).isEmpty();
631+
}
451632
}

‎src/main/java/com/commercetools/sync/services/impl/ProductTypeServiceImpl.java‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ public final class ProductTypeServiceImpl
4343
private final Map<String, Map<String, AttributeMetaData>> productsAttributesMetaData =
4444
new ConcurrentHashMap<>();
4545

46+
private volatile CompletableFuture<Void> cacheLoadingFuture = null;
47+
private final Object cacheLoadingLock = new Object();
48+
4649
public ProductTypeServiceImpl(@Nonnull final BaseSyncOptions syncOptions) {
4750
super(syncOptions);
4851
}
@@ -82,16 +85,58 @@ private static Map<String, AttributeMetaData> getAttributeMetaDataMap(
8285
public CompletionStage<Optional<Map<String, AttributeMetaData>>>
8386
fetchCachedProductAttributeMetaDataMap(@Nonnull final String productTypeId) {
8487

85-
if (productsAttributesMetaData.isEmpty()) {
86-
return fetchAndCacheProductMetaData(productTypeId);
88+
if (!productsAttributesMetaData.isEmpty()) {
89+
return CompletableFuture.completedFuture(
90+
Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
91+
}
92+
93+
CompletableFuture<Void> loadingFuture = cacheLoadingFuture;
94+
if (loadingFuture != null && !loadingFuture.isDone()) {
95+
return loadingFuture.thenApply(
96+
ignored -> Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
97+
}
98+
99+
synchronized (cacheLoadingLock) {
100+
if (!productsAttributesMetaData.isEmpty()) {
101+
return CompletableFuture.completedFuture(
102+
Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
103+
}
104+
105+
loadingFuture = cacheLoadingFuture;
106+
if (loadingFuture != null && !loadingFuture.isDone()) {
107+
return loadingFuture.thenApply(
108+
ignored -> Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
109+
}
110+
111+
final CompletableFuture<Void> newLoadingFuture = new CompletableFuture<>();
112+
cacheLoadingFuture = newLoadingFuture;
113+
114+
fetchAndCacheAllProductMetaData()
115+
.whenComplete(
116+
(result, throwable) -> {
117+
if (throwable != null) {
118+
newLoadingFuture.completeExceptionally(throwable);
119+
} else {
120+
newLoadingFuture.complete(null);
121+
}
122+
});
123+
124+
return newLoadingFuture.thenApply(
125+
ignored -> Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
87126
}
88-
return CompletableFuture.completedFuture(
89-
Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
90127
}
91128

129+
/**
130+
* Fetches all product types from CTP and caches their attribute metadata.
131+
*
132+
* <p>This method is called only once during the first request for attribute metadata. All
133+
* subsequent requests will use the cached data.
134+
*
135+
* @return a {@link CompletionStage} that completes when all product types have been fetched and
136+
* cached
137+
*/
92138
@Nonnull
93-
private CompletionStage<Optional<Map<String, AttributeMetaData>>> fetchAndCacheProductMetaData(
94-
@Nonnull final String productTypeId) {
139+
private CompletionStage<Void> fetchAndCacheAllProductMetaData() {
95140
final Consumer<List<ProductType>> productTypePageConsumer =
96141
productTypePage ->
97142
productTypePage.forEach(
@@ -102,8 +147,7 @@ private CompletionStage<Optional<Map<String, AttributeMetaData>>> fetchAndCacheP
102147
final ByProjectKeyProductTypesGet byProjectKeyProductTypesGet =
103148
this.syncOptions.getCtpClient().productTypes().get();
104149

105-
return QueryUtils.queryAll(byProjectKeyProductTypesGet, productTypePageConsumer)
106-
.thenApply(result -> Optional.ofNullable(productsAttributesMetaData.get(productTypeId)));
150+
return QueryUtils.queryAll(byProjectKeyProductTypesGet, productTypePageConsumer);
107151
}
108152

109153
@Nonnull

0 commit comments

Comments
 (0)