Conversation
danieldietzler
force-pushed
the
fix/asset-update-race-condition
branch
6 times, most recently
from
fcd2a7f to
d0f1793
Compare
| const { exifInfo } = (await this.assetRepository.getById(asset.id, { exifInfo: true })) || {}; | ||
| await this.handleSidecarWrite({ | ||
| id: asset.id, | ||
| description: exifInfo?.description, | ||
| dateTimeOriginal: exifInfo?.dateTimeOriginal?.toISOString(), | ||
| latitude: exifInfo?.latitude ?? undefined, | ||
| longitude: exifInfo?.longitude ?? undefined, | ||
| rating: exifInfo?.rating ?? undefined, | ||
| tags: true, | ||
| }); | ||
| asset = (await this.assetRepository.getById(asset.id, { faces: { person: false } }))!; |
There was a problem hiding this comment.
This fetches the same asset on each line (handleSidecarWrite internally fetches the asset). Could you make it do one DB call including exifInfo, face and tag relations?
There was a problem hiding this comment.
My thinking was that this very rarely needs to run in the first place, so I didn't want to slow down the "happy path" by joining exif there. Either way I need at least two calls total since I need to get the sidecar path after the sidecar file got written.
There was a problem hiding this comment.
Oh, I meant one DB call for the "sad path" here vs 3. I agree joining exif for the main job wouldn't be ideal.
There was a problem hiding this comment.
Right, but I need exifInfo before the sidecar write and I need the sidecar path (that in most cases doesn't exist before) after the sidecar write 😅
There was a problem hiding this comment.
I don't get it. handleSidecarWrite just checks 1) if the asset exists and 2) what its sidecar path is, and you need exifInfo to update the sidecar. What part of this requires multiple queries?
To be clear, I'm suggesting you can make handleSidecarWrite's body a private method instead of calling it like it's a job.
danieldietzler
force-pushed
the
fix/asset-update-race-condition
branch
from
d0f1793 to
75d1d21
Compare
|
was wondering if this PR could get some love? would love to see this fixed. 🙇 |
|
Superseded by #24384 @danieldietzler ? |
|
Superseded by #24384 |
4 participants
Fixes #16747; a race condition between updating an asset and metadata extraction:
METADATA_EXTRACTIONis queuedSIDECAR_WRITEis queuedMETADATA_EXTRACTIONis scheduled, updating and overriding all changes to the asset in the databaseSIDECAR_WRITEis scheduled, but the changes are already lost