sdk icon indicating copy to clipboard operation
sdk copied to clipboard

onRelationshipsFlushed, onEntitiesFlushed possible unnecessarily locking?

Open zemberdotnet opened this issue 3 years ago • 2 comments

https://github.com/JupiterOne/sdk/blob/2139b03dc669199ed5146423e279c5d0013027f4/packages/integration-sdk-runtime/src/storage/FileSystemGraphObjectStore/FileSystemGraphObjectStore.ts#L286-L320

Does the onRelationshipsFlushed hook also need to be locked in the code above? My impression is that after the relationships/entities have been flushed, the hook should be free to run concurrently. If separately a step is running and able to fill the buffer, it must wait until the onRelationshipsFlushed/onEntitiesFlushed hook has finished. If the hook is an upload, that may unnecessarily slow down filling up and flushing the buffer again while uploads are happening. The hook seems like it might be unnecessarily blocking.

zemberdotnet avatar Jul 24 '22 14:07 zemberdotnet

Thinking about this further, I think that onRelationshipsFlushed/onEntitiesFlushed does not need to be locked. The order of uploads shouldn't matter as long as the uploads are not duplicated. The locking only needs to ensure that the upload isn't duplicated. It can do that by ensuring the buffer is cleared without needing to ensure the upload is finished.

zemberdotnet avatar Sep 03 '22 17:09 zemberdotnet

However, this function would need to be refactored to support that as the reference to the entities or relationships is created during the lock operation itself, but it doesn't escape the block currently.

zemberdotnet avatar Sep 03 '22 17:09 zemberdotnet