Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

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

IdentityMap / Cascade Locking? #4318

Closed
parisholley opened this issue May 5, 2023 · 0 comments
Closed

IdentityMap / Cascade Locking? #4318

parisholley opened this issue May 5, 2023 · 0 comments
Labels
enhancement New feature or request

Comments

@parisholley
Copy link
Contributor

Somewhat related to #2380 as I don't think this would be a problem in most write requests/scenarios. I am noticing some strange behavior when entities with the same id are being populated with different values across two different Promises. At times Collections just simply disappear or a Refs has been nulled out when accessed later. I suspect this may due to cascades happening across Promises at the same time. It is hard to create a reproduction for as it isn't consistent due to latency differences/IO. It is more prevalent in serverless frameworks which execute nested routes in parallel.

As a workaround, when I am in a read-only request, I simply set disableIdentityMap to true (I wrap EntityManager with my own implementation) so that there are no collisions but this comes at a potential performance cost (I assume) as it can't shared pre-fetched entities across Promises.

Digging through the code a bit, I think my suggestion would be to update the EntityFactory.create and introduce a queue here:

const exists = this.findEntity<T>(data, meta2, options);
let wrapped = exists && helper(exists);
if (wrapped && !options.refresh) {
wrapped.__processing = true;
this.mergeData(meta2, exists!, data, options);
wrapped.__processing = false;
if (wrapped.isInitialized()) {
return exists as New<T, P>;
}
}
data = { ...data };
const entity = exists ?? this.createEntity<T>(data, meta2, options);
wrapped = helper(entity);
wrapped.__processing = true;
wrapped.__initialized = options.initialized;
this.hydrate(entity, meta2, data, options);
wrapped.__touched = false;
if (exists && meta.discriminatorColumn && !(entity instanceof meta2.class)) {
Object.setPrototypeOf(entity, meta2.prototype as object);
}
if (options.merge && wrapped.hasPrimaryKey()) {
this.unitOfWork.registerManaged(entity, data, {
refresh: options.refresh && options.initialized,
newEntity: options.newEntity,
loaded: options.initialized,
});
}
if (this.eventManager.hasListeners(EventType.onInit, meta2)) {
this.eventManager.dispatchEvent(EventType.onInit, { entity, em: this.em });
}
wrapped.__processing = false;
return entity as New<T, P>;

Every find is added to a queue with a callback fn(existing:Entity) => Entity which would prevent merges/cascades happening out of order.

@parisholley parisholley added the enhancement New feature or request label May 5, 2023
@mikro-orm mikro-orm locked and limited conversation to collaborators May 5, 2023
@B4nan B4nan converted this issue into discussion #4319 May 5, 2023
parisholley added a commit to parisholley/mikro-orm that referenced this issue May 18, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant