fix(reindex): Fix deadlocks for sub-resources

NEWS.md

@@ -75,6 +75,7 @@
   * Ignore shadow locations and location units while indexing domain events ([MSEARCH-1154](https://folio-org.atlassian.net/browse/MSEARCH-1154))
   * Add support for exact match on isbn, honor '*' in IsbnSearchTermProcessor ([MSEARCH-1011](https://folio-org.atlassian.net/browse/MSEARCH-1011))
   * Update instance_call_number tenantId on Instance becoming shared ([MSEARCH-1168](https://folio-org.atlassian.net/browse/MSEARCH-1168))
+  * Fix deadlocks for sub-resources on reindex ([MSEARCH-1178](https://folio-org.atlassian.net/browse/MSEARCH-1178))
 
 ### Tech Dept
 * Migrate to Opensearch 3.0.0 ([MSEARCH-1033](https://folio-org.atlassian.net/browse/MSEARCH-1033))

src/main/java/org/folio/search/service/InstanceChildrenResourceService.java

@@ -62,8 +62,8 @@ public void persistChildrenOnReindex(String tenantId, ResourceType resourceType,
 
     var shared = consortiumTenantProvider.isCentralTenant(tenantId);
 
-    // Process child resources normally
+    // Process child resources for reindex (ON CONFLICT DO NOTHING for entities)
     extractors.forEach(resourceExtractor ->
-      resourceExtractor.persistChildren(tenantId, shared, events));
+      resourceExtractor.persistChildrenOnReindex(shared, events));
   }
 }

src/main/java/org/folio/search/service/converter/preprocessor/extractor/ChildResourceExtractor.java

@@ -38,14 +38,17 @@ public void persistChildren(String tenantId, boolean shared, List<ResourceEvent>
       return;
     }
 
-    var entities = new HashSet<Map<String, Object>>();
-    var relations = new LinkedList<Map<String, Object>>();
-    eventsForSaving.forEach(event -> {
-      var entitiesFromEvent = extractEntities(event);
-      relations.addAll(constructRelations(shared, event, entitiesFromEvent));
-      entities.addAll(entitiesFromEvent);
-    });
-    repository.saveAll(new ChildResourceEntityBatch(new ArrayList<>(entities), relations));
+    var batch = buildBatch(shared, eventsForSaving);
+    repository.saveAll(batch);
+  }
+
+  public void persistChildrenOnReindex(boolean shared, List<ResourceEvent> events) {
+    if (events.isEmpty()) {
+      return;
+    }
+
+    var batch = buildBatch(shared, events);
+    repository.saveAllOnReindex(batch);
   }
 
   protected abstract List<Map<String, Object>> constructRelations(boolean shared, ResourceEvent event,
@@ -64,6 +67,17 @@ protected Set<Map<String, Object>> getChildResources(Map<String, Object> event)
     return new HashSet<>((List<Map<String, Object>>) object);
   }
 
+  private ChildResourceEntityBatch buildBatch(boolean shared, List<ResourceEvent> events) {
+    var entities = new HashSet<Map<String, Object>>();
+    var relations = new LinkedList<Map<String, Object>>();
+    events.forEach(event -> {
+      var entitiesFromEvent = extractEntities(event);
+      relations.addAll(constructRelations(shared, event, entitiesFromEvent));
+      entities.addAll(entitiesFromEvent);
+    });
+    return new ChildResourceEntityBatch(new ArrayList<>(entities), relations);
+  }
+
   private void deleteParentsIfNeeded(String tenantId, List<ResourceEvent> events) {
     var parentIdsForDeletion = events.stream()
       .filter(event -> event.getType() != ResourceEventType.CREATE && event.getType() != ResourceEventType.REINDEX)

src/main/java/org/folio/search/service/reindex/jdbc/CallNumberRepository.java

@@ -154,6 +154,17 @@ LEFT JOIN (
     ON CONFLICT (id) DO UPDATE SET last_updated_date = CURRENT_TIMESTAMP;
     """;
 
+  private static final String INSERT_ENTITIES_FOR_REINDEX_SQL = """
+    INSERT INTO %s (
+        id,
+        call_number,
+        call_number_prefix,
+        call_number_suffix,
+        call_number_type_id
+    ) VALUES (?, ?, ?, ?, ?)
+    ON CONFLICT DO NOTHING;
+    """;
+
   private static final String INSERT_RELATIONS_SQL = """
     INSERT INTO %s (
         call_number_id,
@@ -198,7 +209,13 @@ public void updateLastUpdatedDate(String instanceId) {
 
   @Override
   public void saveAll(ChildResourceEntityBatch entityBatch) {
-    saveResourceEntities(entityBatch);
+    saveResourceEntities(INSERT_ENTITIES_SQL, entityBatch);
+    saveRelationshipEntities(entityBatch);
+  }
+
+  @Override
+  public void saveAllOnReindex(ChildResourceEntityBatch entityBatch) {
+    saveResourceEntities(INSERT_ENTITIES_FOR_REINDEX_SQL, entityBatch);
     saveRelationshipEntities(entityBatch);
   }
 
@@ -271,9 +288,9 @@ private Map<String, Object> getCallNumberMap(ResultSet rs) throws SQLException {
     return callNumberMap;
   }
 
-  private void saveResourceEntities(ChildResourceEntityBatch entityBatch) {
+  private void saveResourceEntities(String insertSqlTemplate, ChildResourceEntityBatch entityBatch) {
     var callNumberTable = getFullTableName(context, entityTable());
-    var callNumberSql = INSERT_ENTITIES_SQL.formatted(callNumberTable);
+    var callNumberSql = insertSqlTemplate.formatted(callNumberTable);
 
     try {
       jdbcTemplate.batchUpdate(callNumberSql, entityBatch.resourceEntities(), BATCH_OPERATION_SIZE,

src/main/java/org/folio/search/service/reindex/jdbc/ClassificationRepository.java

@@ -124,6 +124,11 @@ WHERE instance_id IN (%2$s) %3$s
       VALUES (?, ?, ?)
       ON CONFLICT (id) DO UPDATE SET last_updated_date = CURRENT_TIMESTAMP;
     """;
+  private static final String INSERT_ENTITIES_FOR_REINDEX_SQL = """
+      INSERT INTO %s.classification (id, number, type_id)
+      VALUES (?, ?, ?)
+      ON CONFLICT DO NOTHING;
+    """;
   private static final String INSERT_RELATIONS_SQL = """
       INSERT INTO %s.instance_classification (instance_id, classification_id, tenant_id, shared)
       VALUES (?::uuid, ?, ?, ?)
@@ -188,10 +193,20 @@ public void deleteByInstanceIds(List<String> instanceIds, String tenantId) {
     deleteByInstanceIds(DELETE_QUERY, instanceIds, tenantId);
   }
 
-  @SuppressWarnings("checkstyle:MethodLength")
   @Override
   public void saveAll(ChildResourceEntityBatch entityBatch) {
-    var entitiesSql = INSERT_ENTITIES_SQL.formatted(JdbcUtils.getSchemaName(context));
+    saveEntities(INSERT_ENTITIES_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  @Override
+  public void saveAllOnReindex(ChildResourceEntityBatch entityBatch) {
+    saveEntities(INSERT_ENTITIES_FOR_REINDEX_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  private void saveEntities(String insertSqlTemplate, ChildResourceEntityBatch entityBatch) {
+    var entitiesSql = insertSqlTemplate.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(entitiesSql, entityBatch.resourceEntities(), BATCH_OPERATION_SIZE,
         (statement, entity) -> {
@@ -206,7 +221,9 @@ public void saveAll(ChildResourceEntityBatch entityBatch) {
           entity.get("id"), entity.get(CLASSIFICATION_NUMBER_FIELD), entity.get(CLASSIFICATION_TYPE_FIELD));
       }
     }
+  }
 
+  private void saveRelations(ChildResourceEntityBatch entityBatch) {
     var relationsSql = INSERT_RELATIONS_SQL.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(relationsSql, entityBatch.relationshipEntities(), BATCH_OPERATION_SIZE,

src/main/java/org/folio/search/service/reindex/jdbc/ContributorRepository.java

@@ -130,6 +130,11 @@ WHERE instance_id IN (%2$s) %3$s
       VALUES (?, ?, ?, ?)
       ON CONFLICT (id) DO UPDATE SET last_updated_date = CURRENT_TIMESTAMP;
     """;
+  private static final String INSERT_ENTITIES_FOR_REINDEX_SQL = """
+      INSERT INTO %s.contributor (id, name, name_type_id, authority_id)
+      VALUES (?, ?, ?, ?)
+      ON CONFLICT DO NOTHING;
+    """;
   private static final String INSERT_RELATIONS_SQL = """
       INSERT INTO %s.instance_contributor (instance_id, contributor_id, type_id, tenant_id, shared)
       VALUES (?::uuid, ?, ?, ?, ?)
@@ -193,9 +198,19 @@ public void deleteByInstanceIds(List<String> instanceIds, String tenantId) {
   }
 
   @Override
-  @SuppressWarnings("checkstyle:MethodLength")
   public void saveAll(ChildResourceEntityBatch entityBatch) {
-    var entitiesSql = INSERT_ENTITIES_SQL.formatted(JdbcUtils.getSchemaName(context));
+    saveEntities(INSERT_ENTITIES_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  @Override
+  public void saveAllOnReindex(ChildResourceEntityBatch entityBatch) {
+    saveEntities(INSERT_ENTITIES_FOR_REINDEX_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  private void saveEntities(String insertSqlTemplate, ChildResourceEntityBatch entityBatch) {
+    var entitiesSql = insertSqlTemplate.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(entitiesSql, entityBatch.resourceEntities(), BATCH_OPERATION_SIZE,
         (statement, entity) -> {
@@ -211,7 +226,9 @@ public void saveAll(ChildResourceEntityBatch entityBatch) {
           entity.get("id"), entity.get("name"), entity.get("nameTypeId"), entity.get(AUTHORITY_ID_FIELD));
       }
     }
+  }
 
+  private void saveRelations(ChildResourceEntityBatch entityBatch) {
     var relationsSql = INSERT_RELATIONS_SQL.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(relationsSql, entityBatch.relationshipEntities(), BATCH_OPERATION_SIZE,

src/main/java/org/folio/search/service/reindex/jdbc/InstanceChildResourceRepository.java

@@ -10,5 +10,7 @@ public interface InstanceChildResourceRepository {
 
   void saveAll(ChildResourceEntityBatch childResourceEntityBatch);
 
+  void saveAllOnReindex(ChildResourceEntityBatch childResourceEntityBatch);
+
   ReindexEntityType entityType();
 }

src/main/java/org/folio/search/service/reindex/jdbc/SubjectRepository.java

@@ -131,6 +131,11 @@ WHERE instance_id IN (%2$s) %3$s
       VALUES (?, ?, ?, ?, ?)
       ON CONFLICT (id) DO UPDATE SET last_updated_date = CURRENT_TIMESTAMP;
     """;
+  private static final String INSERT_ENTITIES_FOR_REINDEX_SQL = """
+      INSERT INTO %s.subject (id, value, authority_id, source_id, type_id)
+      VALUES (?, ?, ?, ?, ?)
+      ON CONFLICT DO NOTHING;
+    """;
   private static final String INSERT_RELATIONS_SQL = """
       INSERT INTO %s.instance_subject (instance_id, subject_id, tenant_id, shared)
       VALUES (?::uuid, ?, ?, ?)
@@ -196,9 +201,19 @@ public void deleteByInstanceIds(List<String> instanceIds, String tenantId) {
   }
 
   @Override
-  @SuppressWarnings("checkstyle:MethodLength")
   public void saveAll(ChildResourceEntityBatch entityBatch) {
-    var entitiesSql = INSERT_ENTITIES_SQL.formatted(JdbcUtils.getSchemaName(context));
+    saveEntities(INSERT_ENTITIES_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  @Override
+  public void saveAllOnReindex(ChildResourceEntityBatch entityBatch) {
+    saveEntities(INSERT_ENTITIES_FOR_REINDEX_SQL, entityBatch);
+    saveRelations(entityBatch);
+  }
+
+  private void saveEntities(String insertSqlTemplate, ChildResourceEntityBatch entityBatch) {
+    var entitiesSql = insertSqlTemplate.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(entitiesSql, entityBatch.resourceEntities(), BATCH_OPERATION_SIZE,
         (statement, entity) -> {
@@ -215,7 +230,9 @@ public void saveAll(ChildResourceEntityBatch entityBatch) {
           entity.get(AUTHORITY_ID_FIELD), entity.get(SUBJECT_SOURCE_ID_FIELD), entity.get(SUBJECT_TYPE_ID_FIELD));
       }
     }
+  }
 
+  private void saveRelations(ChildResourceEntityBatch entityBatch) {
     var relationsSql = INSERT_RELATIONS_SQL.formatted(JdbcUtils.getSchemaName(context));
     try {
       jdbcTemplate.batchUpdate(relationsSql, entityBatch.relationshipEntities(), BATCH_OPERATION_SIZE,

src/test/java/org/folio/search/service/InstanceChildrenResourceServiceTest.java

@@ -80,7 +80,7 @@ void persistChildrenOnReindex(boolean shared) {
     service.persistChildrenOnReindex(TENANT_ID, ResourceType.INSTANCE, instances);
 
     instanceResourceExtractors.forEach(resourceExtractor ->
-      verify(resourceExtractor).persistChildren(TENANT_ID, shared, expectedEvents));
+      verify(resourceExtractor).persistChildrenOnReindex(shared, expectedEvents));
   }
 
   private ResourceEvent getResourceEvent(UUID id1, Map<String, Object> payload) {

src/test/java/org/folio/search/service/converter/preprocessor/extractor/impl/CallNumberResourceExtractorTest.java

@@ -50,6 +50,12 @@ void persistChildren() {
     verify(repository, times(1)).deleteByInstanceIds(anyList(), eq(TENANT_ID));
   }
 
+  @Test
+  void persistChildrenOnReindex() {
+    when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CALL_NUMBERS)).thenReturn(true);
+    persistChildrenOnReindexTest(extractor, repository, callNumberBodySupplier());
+  }
+
   @Test
   void shouldNotPersistEmptyCallNumber() {
     when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CALL_NUMBERS)).thenReturn(true);

src/test/java/org/folio/search/service/converter/preprocessor/extractor/impl/ClassificationResourceExtractorTest.java

@@ -39,6 +39,12 @@ void persistChildren() {
     persistChildrenTest(extractor, repository, classificationsBodySupplier());
   }
 
+  @Test
+  void persistChildrenOnReindex() {
+    when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CLASSIFICATIONS)).thenReturn(true);
+    persistChildrenOnReindexTest(extractor, repository, classificationsBodySupplier());
+  }
+
   @Test
   void shouldNotPersistEmptyClassification() {
     when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CLASSIFICATIONS)).thenReturn(true);

src/test/java/org/folio/search/service/converter/preprocessor/extractor/impl/ContributorResourceExtractorTest.java

@@ -40,6 +40,12 @@ void persistChildren() {
     persistChildrenTest(extractor, repository, contributorsBodySupplier());
   }
 
+  @Test
+  void persistChildrenOnReindex() {
+    when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CONTRIBUTORS)).thenReturn(true);
+    persistChildrenOnReindexTest(extractor, repository, contributorsBodySupplier());
+  }
+
   @Test
   void shouldNotPersistEmptyContributor() {
     when(configService.isEnabled(TenantConfiguredFeature.BROWSE_CONTRIBUTORS)).thenReturn(true);

src/test/java/org/folio/search/service/converter/preprocessor/extractor/impl/SubjectResourceExtractorTest.java

@@ -41,6 +41,12 @@ void persistChildren() {
     persistChildrenTest(extractor, repository, subjectsBodySupplier());
   }
 
+  @Test
+  void persistChildrenOnReindex() {
+    when(configService.isEnabled(TenantConfiguredFeature.BROWSE_SUBJECTS)).thenReturn(true);
+    persistChildrenOnReindexTest(extractor, repository, subjectsBodySupplier());
+  }
+
   @Test
   void shouldNotPersistEmptySubject() {
     when(configService.isEnabled(TenantConfiguredFeature.BROWSE_SUBJECTS)).thenReturn(true);

src/test/java/org/folio/search/service/reindex/jdbc/CallNumberRepositoryIT.java

@@ -47,7 +47,7 @@ class CallNumberRepositoryIT {
 
   private static final String INSTANCE_ID = "9f8febd1-e96c-46c4-a5f4-84a45cc499a2";
   private static final Map<String, List<UUID>> ITEM_IDS =
-    Map.of("1", getList(1), "2", getList(2));
+    Map.of("1", getList(1), "2", getList(2), "3", getList(2), "4", getList(2));
   private @MockitoSpyBean JdbcTemplate jdbcTemplate;
   private @MockitoBean FolioExecutionContext context;
   private CallNumberRepository repository;
@@ -114,6 +114,43 @@ void saveAll() {
             "locationId", null, "shared", false, "tenantId", TENANT_ID))));
   }
 
+  @Test
+  @Sql("/sql/populate-instances.sql")
+  @SuppressWarnings("checkstyle:MethodLength")
+  void saveAllOnReindex() {
+    // save entity "3" via saveAll first, capturing its last_updated_date
+    repository.saveAll(new ChildResourceEntityBatch(Set.of(callNumberEntity("3")),
+      List.of(callNumberRelation("3"))));
+    var lastUpdatedAfterSaveAll = jdbcTemplate.queryForObject(
+      "SELECT last_updated_date FROM call_number WHERE id = ?", Timestamp.class, "3");
+
+    // call saveAllOnReindex with existing entity "3" and new entity "4"
+    repository.saveAllOnReindex(new ChildResourceEntityBatch(
+      Set.of(callNumberEntity("3"), callNumberEntity("4")),
+      List.of(
+        callNumberRelation("3"),
+        callNumberRelation("4"),
+        callNumberRelation("4"))));
+
+    // assert: new entity "4" and its relations were saved
+    var ranges = repository.fetchByIdRange("0", "z");
+    assertThat(ranges)
+      .hasSize(2)
+      .extracting("callNumber", "instances")
+      .contains(
+        tuple("number3",
+          List.of(mapOf(
+            "instanceId", List.of(INSTANCE_ID), "locationId", null, "shared", false, "tenantId", TENANT_ID))),
+        tuple("number4",
+          List.of(mapOf(
+            "instanceId", List.of(INSTANCE_ID), "locationId", null, "shared", false, "tenantId", TENANT_ID))));
+
+    // assert: existing entity "3" was not updated (last_updated_date unchanged)
+    var lastUpdatedAfterReindex = jdbcTemplate.queryForObject(
+      "SELECT last_updated_date FROM call_number WHERE id = ?", Timestamp.class, "3");
+    assertThat(lastUpdatedAfterReindex).isEqualTo(lastUpdatedAfterSaveAll);
+  }
+
   @Test
   @Sql("/sql/populate-instances.sql")
   void updateLastUpdatedDate_updatesCallNumberLastUpdatedDate() {