Skip to content

Fix flaky "Entity not found: <service>" on list (concurrent cascade delete) #29150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
harshach wants to merge 1 commit into from
Closed

Fix flaky "Entity not found: <service>" on list (concurrent cascade delete) #29150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/BaseEntityIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.openmetadata.it.util.TestNamespaceExtension;
import org.openmetadata.it.util.UpdateType;
import org.openmetadata.schema.EntityInterface;
import org.openmetadata.schema.ServiceEntityInterface;
import org.openmetadata.schema.api.policies.CreatePolicy;
import org.openmetadata.schema.api.teams.CreateRole;
import org.openmetadata.schema.api.teams.CreateUser;
Expand All @@ -45,7 +46,10 @@
import org.openmetadata.schema.entity.teams.User;
import org.openmetadata.schema.type.ApiStatus;
import org.openmetadata.schema.type.ChangeDescription;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.schema.type.Include;
import org.openmetadata.schema.type.MetadataOperation;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.schema.type.TagLabel;
import org.openmetadata.schema.type.api.BulkOperationResult;
import org.openmetadata.schema.type.api.BulkResponse;
Expand All @@ -58,6 +62,10 @@
import org.openmetadata.sdk.services.policies.PolicyService;
import org.openmetadata.sdk.services.teams.RoleService;
import org.openmetadata.sdk.services.teams.UserService;
import org.openmetadata.service.Entity;
import org.openmetadata.service.jdbi3.CollectionDAO;
import org.openmetadata.service.jdbi3.EntityDAO;
import org.openmetadata.service.jdbi3.EntityRepository;
import org.openmetadata.service.util.TestUtils;

/**
Expand Down Expand Up @@ -1048,6 +1056,76 @@ void get_entityListWithInvalidPaginationCursors_4xx(TestNamespace ns) {
"Listing with invalid before cursor should fail");
}

/**
* Regression for the flaky {@code Entity not found: <service> <id>} failure on the LIST path (seen
* in CI on {@code ContainerResourceIT}/{@code MlModelResourceIT}). A list resolves every row's
* parent service in bulk, in a statement separate from the relationship lookup; when a sibling
* test cascade-hard-deletes the service in between, the {@code CONTAINS} relationship row is still
* seen but the service entity row is already gone. A non-lenient bulk resolver then threw {@code
* EntityNotFoundException} and failed the whole list instead of the single affected row.
*
* <p>Runs for every entity directly contained by a service this test created (skips entities with
* no owned service parent). Reproduces the window deterministically: delete only the service
* entity row (the {@code CONTAINS} relationship survives) and evict it from the entity cache —
* exactly the state a concurrent cascade delete leaves mid-flight — then list scoped by the
* now-dangling service FQN. The list must succeed; the bulk resolver must tolerate the deleted
* service rather than fail the whole page.
*/
@Test
void list_toleratesConcurrentlyHardDeletedService(TestNamespace ns) {
T entity = createEntity(createMinimalRequest(ns));
EntityReference serviceRef = findOwnedServiceParent(ns, entity);
Assumptions.assumeTrue(
serviceRef != null,
getEntityType() + " is not directly contained by a service it owns; tolerance test N/A");

String serviceType = serviceRef.getType();
EntityDAO<?> serviceDao = Entity.getEntityRepository(serviceType).getDao();
int rowsRemoved = serviceDao.delete(serviceDao.getTableName(), serviceRef.getId());
assertEquals(1, rowsRemoved, "Setup: should have removed exactly the service entity row");
EntityRepository.invalidateCacheForEntity(
serviceType, serviceRef.getId(), serviceRef.getFullyQualifiedName());

org.openmetadata.sdk.models.ListParams params = new org.openmetadata.sdk.models.ListParams();
params.setService(serviceRef.getFullyQualifiedName());
params.setLimit(1000);

org.openmetadata.sdk.models.ListResponse<T> response = listEntities(params);
Comment on lines +1089 to +1093

@gitar-bot gitar-bot Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Fully-qualified names used instead of imports in BaseEntityIT

In the new list_toleratesConcurrentlyHardDeletedService test, ListParams and ListResponse are referenced via fully-qualified names rather than imports:

org.openmetadata.sdk.models.ListParams params = new org.openmetadata.sdk.models.ListParams();
...
org.openmetadata.sdk.models.ListResponse<T> response = listEntities(params);

The project's code standards explicitly forbid fully-qualified names ("No fully qualified names"). Add import org.openmetadata.sdk.models.ListParams; and import org.openmetadata.sdk.models.ListResponse; and use the simple names. (Note ListResponse/ListParams may already be imported elsewhere in this file given listEntities is used; verify to avoid duplicate-import.)

Was this helpful? React with 👍 / 👎

assertNotNull(response);
assertNotNull(
response.getData(),
"List must tolerate the concurrently hard-deleted service, not fail the whole page");
}

/**
* Find this entity's immediate {@code CONTAINS} parent that is a service AND was created by this
* test (tracked as a namespace root) so deleting it can never disturb a shared fixture. Returns
* {@code null} when the entity is not directly service-scoped or its parent service is not owned
* here.
*/
private EntityReference findOwnedServiceParent(TestNamespace ns, T entity) {
EntityReference serviceRef = null;
List<CollectionDAO.EntityRelationshipRecord> parents =
Entity.getCollectionDAO()
.relationshipDAO()
.findFrom(entity.getId(), getEntityType(), Relationship.CONTAINS.ordinal());
for (CollectionDAO.EntityRelationshipRecord parent : parents) {
boolean ownedByThisTest =
ns.trackedRoots().stream().anyMatch(root -> root.id().equals(parent.getId()));
if (ownedByThisTest && isServiceEntity(parent.getType(), parent.getId())) {
serviceRef = Entity.getEntityReferenceById(parent.getType(), parent.getId(), Include.ALL);
break;
}
}
return serviceRef;
}

private boolean isServiceEntity(String entityType, UUID id) {
EntityInterface candidate =
Entity.getEntity(new EntityReference().withId(id).withType(entityType), "", Include.ALL);
return candidate instanceof ServiceEntityInterface;
}
Comment on lines +1123 to +1127

@greptile-apps greptile-apps Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 isServiceEntity performs a full entity fetch just to type-check.

Entity.getEntity(ref, "", Include.ALL) loads and deserialises the entire entity to check instanceof ServiceEntityInterface. For the service-type check you could instead call Entity.getEntityRepository(entityType) and inspect the repository's entity class, or check against the known service type string constants — both avoid the DB round-trip.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


@Test
void get_entityWithInvalidFields_4xx(TestNamespace ns) {
if (!supportsFieldsQueryParam) return;
Expand Down
24 changes: 24 additions & 0 deletions openmetadata-service/src/main/java/org/openmetadata/service/Entity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,30 @@ public static EntityReference getEntityReferenceById(
return repository.getReference(id, include);
}

/**
* Lenient variant of {@link #getEntityReferenceById(String, UUID, Include)} that returns {@code
* null} instead of throwing when the referenced entity was concurrently hard-deleted. A bulk read
* path resolves a relationship's target entity in a statement separate from the relationship
* lookup; if the target is hard-deleted in between (e.g. a sibling test cascade-deleting a parent
* service), the relationship row was seen but the entity row is already gone. Returning {@code
* null} there — rather than 500'ing the whole list with "Entity not found: &lt;type&gt;
* &lt;id&gt;" — mirrors {@link EntityRepository#getFromEntityRef}'s existing single-entity
* tolerance and the batch resolver {@link #getEntityReferencesByIds}, whose underlying
* {@code findEntitiesByIds} already omits concurrently-deleted rows.
*/
public static EntityReference getEntityReferenceByIdOrNull(
@NonNull String entityType, @NonNull UUID id, Include include) {
EntityReference reference;
try {
reference = getEntityReferenceById(entityType, id, include);
} catch (EntityNotFoundException e) {
LOG.debug(
"Skipping concurrently-deleted reference {} {}: {}", entityType, id, e.getMessage());
reference = null;
}
return reference;
}

public static List<EntityReference> getEntityReferencesByIds(
@NonNull String entityType, @NonNull List<UUID> ids, Include include) {
// Check if this is a time-series entity
Expand Down
20 changes: 13 additions & 7 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ContainerRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import static org.openmetadata.service.Entity.FIELD_PARENT;
import static org.openmetadata.service.Entity.FIELD_TAGS;
import static org.openmetadata.service.Entity.STORAGE_SERVICE;
import static org.openmetadata.service.Entity.getEntityReferenceById;
import static org.openmetadata.service.Entity.getEntityReferenceByIdOrNull;
import static org.openmetadata.service.resources.tags.TagLabelUtil.addDerivedTagsGracefully;
import static org.openmetadata.service.resources.tags.TagLabelUtil.addDerivedTagsWithPreFetched;
import static org.openmetadata.service.resources.tags.TagLabelUtil.batchFetchDerivedTags;
Expand Down Expand Up @@ -187,9 +187,11 @@ private Map<UUID, EntityReference> batchFetchParents(List<Container> containers)
// Only consider container parents, not service parents
if (CONTAINER.equals(record.getFromEntity())) {
EntityReference parentRef =
getEntityReferenceById(
getEntityReferenceByIdOrNull(
record.getFromEntity(), UUID.fromString(record.getFromId()), NON_DELETED);
parentsMap.put(containerId, parentRef);
if (parentRef != null) {
parentsMap.put(containerId, parentRef);
}
}
}

Expand Down Expand Up @@ -226,8 +228,10 @@ private Map<UUID, List<EntityReference>> batchFetchChildren(List<Container> cont
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID parentId = UUID.fromString(record.getFromId());
EntityReference childRef =
getEntityReferenceById(CONTAINER, UUID.fromString(record.getToId()), NON_DELETED);
childrenMap.get(parentId).add(childRef);
getEntityReferenceByIdOrNull(CONTAINER, UUID.fromString(record.getToId()), NON_DELETED);
if (childRef != null) {
childrenMap.get(parentId).add(childRef);
}
}

return childrenMap;
Expand Down Expand Up @@ -273,8 +277,10 @@ private Map<UUID, EntityReference> batchFetchServices(List<Container> containers
UUID serviceId = UUID.fromString(record.getFromId());
EntityReference serviceRef =
serviceRefById.computeIfAbsent(
serviceId, id -> getEntityReferenceById(STORAGE_SERVICE, id, NON_DELETED));
serviceMap.put(containerId, serviceRef);
serviceId, id -> getEntityReferenceByIdOrNull(STORAGE_SERVICE, id, NON_DELETED));
if (serviceRef != null) {
serviceMap.put(containerId, serviceRef);
}
Comment on lines 278 to +283

@greptile-apps greptile-apps Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 computeIfAbsent silently drops null results — the deduplication comment doesn't hold for the concurrent-delete case.

When getEntityReferenceByIdOrNull returns null, Map.computeIfAbsent does not store a null mapping in serviceRefById (per Java spec: the entry is only inserted when the mapping function returns non-null). On the next iteration for the same deleted serviceId, the key is still absent, so the mapping function is called again. For a page of N containers all under the same concurrently-deleted service you'll perform N DB round-trips instead of one — each hitting the catch block in getEntityReferenceByIdOrNull. This is correct but defeats the deduplication the comment advertises. A sentinel Optional.empty() or a computeIfAbsent-then-check approach can fix it: serviceRefById.computeIfAbsent(serviceId, id -> Optional.ofNullable(getEntityReferenceByIdOrNull(...))), then unwrap with orElse(null). The same pattern applies to DirectoryRepository.batchFetchFromByType's refById map.

}

return serviceMap;
Expand Down
8 changes: 6 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DirectoryRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.openmetadata.service.Entity.FILE;
import static org.openmetadata.service.Entity.SPREADSHEET;
import static org.openmetadata.service.Entity.getEntityReferenceById;
import static org.openmetadata.service.Entity.getEntityReferenceByIdOrNull;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -281,8 +282,11 @@ private Map<UUID, EntityReference> batchFetchFromByType(
UUID fromId = UUID.fromString(record.getFromId());
EntityReference ref =
refById.computeIfAbsent(
fromId, id -> getEntityReferenceById(fromEntityType, id, Include.NON_DELETED));
resultMap.put(directoryId, ref);
fromId,
id -> getEntityReferenceByIdOrNull(fromEntityType, id, Include.NON_DELETED));
if (ref != null) {
resultMap.put(directoryId, ref);
}
}
}
return resultMap;
Expand Down
6 changes: 4 additions & 2 deletions ...ata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<IngestionPipeline> pi
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID pipelineId = UUID.fromString(record.getToId());
EntityReference serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
record.getFromEntity(), UUID.fromString(record.getFromId()), Include.NON_DELETED);
serviceMap.put(pipelineId, serviceRef);
if (serviceRef != null) {
serviceMap.put(pipelineId, serviceRef);
}
}

return serviceMap;
Expand Down
6 changes: 4 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/LLMModelRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<LLMModel> llmModels)
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID llmModelId = UUID.fromString(record.getToId());
EntityReference serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
Entity.LLM_SERVICE, UUID.fromString(record.getFromId()), NON_DELETED);
serviceMap.put(llmModelId, serviceRef);
if (serviceRef != null) {
serviceMap.put(llmModelId, serviceRef);
}
}

return serviceMap;
Expand Down
6 changes: 4 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/MlModelRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<MlModel> mlModels) {
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID mlModelId = UUID.fromString(record.getToId());
EntityReference serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
Entity.MLMODEL_SERVICE, UUID.fromString(record.getFromId()), NON_DELETED);
serviceMap.put(mlModelId, serviceRef);
if (serviceRef != null) {
serviceMap.put(mlModelId, serviceRef);
}
}

return serviceMap;
Expand Down
6 changes: 4 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/PipelineRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,9 +972,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<Pipeline> pipelines)
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID pipelineId = UUID.fromString(record.getToId());
EntityReference serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
Entity.PIPELINE_SERVICE, UUID.fromString(record.getFromId()), NON_DELETED);
serviceMap.put(pipelineId, serviceRef);
if (serviceRef != null) {
serviceMap.put(pipelineId, serviceRef);
}
}

return serviceMap;
Expand Down
6 changes: 4 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/SearchIndexRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<SearchIndex> searchIn
if (Entity.SEARCH_SERVICE.equals(record.getFromEntity())) {
UUID searchIndexId = UUID.fromString(record.getToId());
EntityReference serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
Entity.SEARCH_SERVICE, UUID.fromString(record.getFromId()), Include.NON_DELETED);
serviceMap.put(searchIndexId, serviceRef);
if (serviceRef != null) {
serviceMap.put(searchIndexId, serviceRef);
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TopicRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,9 +573,11 @@ private Map<UUID, EntityReference> batchFetchServices(List<Topic> topics) {
record -> {
var topicId = UUID.fromString(record.getToId());
var serviceRef =
Entity.getEntityReferenceById(
Entity.getEntityReferenceByIdOrNull(
Entity.MESSAGING_SERVICE, UUID.fromString(record.getFromId()), NON_DELETED);
serviceMap.put(topicId, serviceRef);
if (serviceRef != null) {
serviceMap.put(topicId, serviceRef);
}
});

return serviceMap;
Expand Down
Loading