483 Replies
no, is dev-update necessary?
there is no such command within
/server/DockerfileYes, in some cases you need to rebuild some parts of the code
You run it in the root, so not in server
You might need to run that first, usually you get a module not found error, then you run make dev
It might just need a rebuild. Otherwise let me know here and we'll debug
was there a change in packages for this PR?
make dev needs Makefile, I don't understand what command is that, I run following command in my CI...
Let's backtrack a little then, I'm not sure how you are building the code. Obviously you are using the git version, but are you running a development setup with vscode or building from your CI?
Also, in chrome and the f12 menu, what do the time bucket calls look like?
I'm in the same TZ as you so I recognize the 2 hour offset bug but don't remember how I fixed that one
also, what timezone do you have inside of your container?
I'm running production setup only, same as Docker
requests look in Firefox like
/api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z
also +2can you post your docker-compose please?
I do not use Docker at all
I wonder why do you think it's caused by the environment.
As I look to the code changes, there is only a change with time bucket calculation, there is no environment change.
db migration to
localDateTime seems to be successfula single photo example...
I'm just trying to understand how the app is built maybe I could add some debug logging to your app to se where the data flows and timezones get converted
yes, that would be helpful
the build process should be same as it is built by Dockerfile
you said this in github: the buckets request this: /api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z
never mind, I misread
what happens if you try in chrome?
the same
ok, thanks
let's focus on the initial request from the browser. In my setup I see the 00:00Z in chrome. You instead are seeing a request for 22:00Z
and that's likely the root cause
right, I didn't notice... in chrome as well...
/api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22%3A00%3A00.000Z
however changing it to /api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30 has no effectChange to /api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-10-01T00%3A00%3A00.000Z
i.e. two hours later
during development I saw issues where requests that should be at midnight were skewed to two hours before = no assets returned
that works 😮
yep
hm...
what is the response to the time-bucket api call? (note: no s at the end)
json with 45 assets...
sorry I'm stupid
I mean the time-buckets (with s)
you mean
api/asset/time-buckets?size=MONTH&isArchived=false&timeBucket=2023-10-01T00%3A00%3A00.000Z ?yes
23 objects...
alright, all of them are 22:00
which is wrong
are you sure there's a parameter called &timeBucket= to that call?
Here is mine:
in the request
and here's my response:
yes, that was result of
/api/asset/time-buckets?you had some extra parameters in the request: api/asset/time-buckets?size=MONTH&isArchived=false&timeBucket=2023-10-01T00%3A00%3A00.000Z
yes, I just cut the query parametrs to point the S at the end
are you on the main immich page?
don't follow
what happens if you call just api/asset/time-buckets?size=MONTH&isArchived=false
without the &timeBucket=2023-10-01T00%3A00%3A00.000Z
seems the same
Ok
to explain, currently I'm on Windows machine with +2 offset, server on Fedora is running in the same network, also +2
do you thing it's related to the browser?
what could cause 22:00?
Not sure right now
Time zones are magic 😎
I'll be back later
thank you very much, appreciated
tried to revert to 1.81.1, it requests 22h as well, however time-bucket with
2023-09-30T22:00:00.000Z returns JSON properlystrange
the get time buckets code is unchanged
I'm still thinking and debugging on my end
do you have access to the sql database?
maybe you can run
also, I checked the timezone of my database via the sql command "show timezone" and it says it's UTC
maybe your db is running localtime?
what is the output of "show timezone" in the db?
I'm investigating on my end but I think this could be the root cause
I havent tested the case in which the db server has a non-utc timezone
if you run "set timezone=UTC" in sql and try again?
it might not be enough though
it has changed now...
any difference in the timeline?
this is my db dump
it affected that query within pgadmin only
ok
there are so many moving parts, hard to know what affects what
yes, that query results have now +00 instead previous +02
you might need to set the TZ and PGTZ environment variable for postgres
is what I'm googling
TZ=UTC and PGTZ=UTC
how are you deploying postgres?
that should be set by the app itself
standard distro installation
I'm running other databases and apps, so timezone change shouldn't affect them
usualy apps itself set the timezone while connection to the db
is there possible to set timezone via .env file?
hm...maybe this is the wrong way...lets backtrack a little
we want the query to return 00:00:00+00 to the app
in firefox you get 2 hours before the month
while in the sql client you got 00:00:00+02
which side is responsible for creating that timestamp for the query? server/client?
or db? no idea
midnight in our tz is 22:00:00Z
I wonder if it was working before with 22, why now is that a problem?
I don't know why there is a difference, but I tried some db queries. If we ask "what assets are in the month of 2023-09-30" we get 0 results
if we ask "what assets are in the month of 2023-09-01" or 2023-10-01 we get results
the time-buckets query should return midnight at the first day of months. It doesn't
the new column localDateTime we introduced stores in timestamp without tz
do you have the ability to compile a custom version of the code?
yes, I can
maybe we should have timestamptz in the localDateTime
do you have a test instance? or a db backup?
I can backup the db
that's a good idea, try changing localDateTime to timestamptz when you have taken a backup and then recompile
timestamp = assume timestamp is in local time. Since you are in CET, "midnight" is 22:00:00Z and this is read back to the app
but if we have timestamptz the midnight result from the db will be interpreted as midnight UTC
which will then be propertly used
so it might be a bug on my end 🙂
(I'm talking about asset.entity.ts")
I think
localDateTime was introduced without TZ intentionally to get rid of timezone issues, right? why to enable TZ there now?you are right, and that was why I stored the time as without tz. But it might have been a mistake since we don't really mean what the db means here
we interpret the Z timezone in localDateTime to mean "the local timezone" which when you think about it is different than "please rewrite the timestamp according to my timezone"
but I have a hunch here, and if you could try it I would be grateful, I can also push a fix today
sure, I will test that, enjoy your meal
in all my testing I use docker-compose which means the postgres server runs on UTC time
I read "launch", your wrote "hunch", anyway 😄
haha
no worries, lunch is already done 😉 but now I am hungry to see if my hunch is correct 😉
then let's try...
what about the
localTimeZone data in the db? shouldn't be those converted to the correct TZ?let's try first to see if it works straight away
otherwise we could try a metadata refresh
I think that the underlying sql data should be unchanged though
let's see
again, there are so many moving parts we need to try one thing at a time
I've changed
localDateTime type to timestamptz within server/src/infra/entities/asset.entity.ts, rebuilt server and nothing changedok
can you try setting timezone='UTC' in postgresql.conf?
that's I'm a bit worry about, as I said there is plenty apps on it
ah, of course
so you and I are in the same timezone, running the same version of immich but are seeing very different results
and the only difference I can see it that your db is in CET and mine is in UTC
I can try to set timezone for the single db only
How to Change the Timezone of a Postgres Database
PostgreSQL provides an “ALTER DATABASE” command that is used with the “SET TIMEZONE” clause to change the timezone of a Postgres database.
you can also do this first
if you go to database config ts in immich and add "timezone: Z"
that would set the session tz to Z
timezone: 'Z',
which file exactly?
database.config.ts
server/src/infra/
yes, have it
that I was speaking about previously, to set timezone for the db session, let's try...
apparently postgres always stores in UTC but changes tz per client session
I think so
like this...
that has unfortunatelly no effect
hang on
maybe that option was only for mysql
try to remove the timezone: Z then
I assume the time-buckets call still returns 22:00.00Z?
yes, no change
anyway immich should force UTC timezone for the session somehow and don't rely on database configuration
that should not matter, and if it does it's a bug I need to fix
I've checked, and the time-buckets call does not use localDateTime at all
ok then, we need to find what has broken it
can you set debug: true in the config?
sorry, logging: true
maybe @jrasm91 has some ideas here. Basically, time-buckets return all time buckets as two hours before midnight instead of midnight
done, getting plenty of sql queries in the log
try to access the time-buckets endpoint in the browser
and try to catch the calls in the log
hm...that did not say too much I'm afraid
can you check sql again, find an asset you know the date of and check the fileCreatedAt and localDateTime values. How does the timestamp compare?
I have checked timestamps on those fields on a single asset and the only change that setting of timezone to UTC makes is that
dateTimeOriginal is returned in UTC, localDateTime is the same
dateTimeOriginal UTC+2 2014-06-02 21:29:44+02
dateTimeOriginal UTC 2014-06-02 19:29:44+00
localDateTime in both cases 2014-06-02 14:29:44
timeZone of the asset from exif table is UTC-7
and the real DateTimeOriginal of the asset in the exif is 2014-06-02 12:29:44
it's a mess kind of 😄this all looks correct
I wonder why
localDateTime is different to real exif datathat is the special one
would you like to tell me where that photo was taken?
because setting timezone in the db has no effect to that field
San Francisco
ah, so SF. You took that photo at 14:29 SF time
right
that's what we've coded that column to represent (although it is stored in Z time, it's confusing but correct)
no 12:29
oh, really?
are you in a sql session now?
that's the only relevant time of the asset
run "set timezone to 'utc'"
and check the fields again
yes, I still have that session
note: localDateTime does NOT matter when calling time-buckets
I'm aware, but you've got that results bit above ^^
ah, I understand
filename is
20140602_122945_Richtone(HDR) exif says 12:29:45, and database fields ^^^so dateTimeOriginal is 19:29Z, which is 12:29 local time UTC-7
that makes sense
we can regenerate localDateTime in the future
at least you got this before, when sql tz is UTC
that's right
that's good
but it was in the pgadmin session only
immich needs to tell the db, give me all timestamptz in UTC
it sure feels like it
I think thats the whole story, because immich is getting timestamps in UTC+2, it can misbehave some code
however all timestamps shown on the web version of immich were correct previously
yes
as you say, it worked before
perhaps the time-buckets (which we haven't touched) are correct and it should respond with that format
if you turn on logging again, can you try calling the time-bucket (without s) and see what happens this time?
same parametes as chrome uses
even
/api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z with the wrong timestamptry it
on my own system, I get zero results when I do that
and you said that /api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z got results on the old version, right?
we did touch the time-bucket call in the new version
that's the
/api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z resultthanks
there's an useUTC parameter for typeorm, maybe we can try that one?
yes, it returned assets properly
ok, good
try the useUTC
it could also be that we "just" need to refresh metadata, but try the useUTC first
still the same, still creates GET request with
2023-09-30T22:00:00.000Z
I would focus on the code that affects time-bucket endpointSorry to interrupt - @etnoy is the post in #immich maybe related?
this is not in a release version yet, so no
this is only in the latest git
Ah ok sorry, then nvm
no worries, thanks for suggesting
ok, let's remove the logging and useUTC part and focus on the time-bucket part
I'm testing the relese 1.81.1, the question is why the
time-bucket needs timestamp in format 2023-09-30T22:00:00.000Z, not 23h, or 21h, or 2023-09-30, nothing of those worksso in 1.81.1 it only works with 2023-09-30T22:00:00.000Z, and not 2023-09-30T23:00:00.000Z nor 2023-10-01T00:00:00.000Z ?
exactly
ah, that's interesting
so there's some kind of date conversion back and forth then
your PR works only with 2023-10-01T00:00:00.000Z
alright, we have something to work with 🙂
that's a start
then let's try this: use the git version and set localDateTime to use timestamptz (in the code)
then we rerun all metadata
did you have many assets?
if you have a db backup we can do it manually, too
16000+ 😄
nice
update "assets"
set "localDateTime" = "fileCreatedAt" at TIME ZONE "exif"."timeZone"
from "exif"
where
"exif"."assetId" = "assets"."id" and
"exif"."timeZone" is not null
in sql (please have the backup first, but you probably already have)
one question first, what's the purpose of
localDateTime? to have timestamp affected by the timezone?
so the same as is stored within exif?if you take a localDateTime field and look at it
and just trim off the Z
that should be the local date time where the photo was taken
like in SF, it should say 12:29 if I'm not mistaken
correct
I think immich
exif table field dateTimeOriginal shouldn't be affected by the timezone, but it has calculated timezone inside
it's not well designed schemadateTimeOriginal will be stored in the db as UTC time
but it's different story
yep
timestamps are always a minefield when programming
I'm trying to understand why setting
localDateTime to timestamptz would helpin the new version it matches on localDateTime
so that needs to be correct
what's the real purpose of
localDateTime ?to put things in the right bucket
otherwise, a photo taken in the evening in the US can be put in the next day's bucket when viewing in Europe
ok, I see, the real exif
dateTimeOriginal contentsomething like that
then to have this with timezone will not help
I have another hunch
sorry it's all quite vague
revert the tz change in asset entity
then go to asset server/src/infra/repositories/asset.repository.ts
change line 477 to say "fileCreatedAt" instead of "localDateTime", i.e.
that will make a partial revert of our changes
but should keep the bucket ordering
ok, let's try...
sorry for all the back and forth btw
no problem
now we got something... it works like before
nice
that might be better taking care of your situation with different tz in the database
and the buckets seems to be correct 👍
and as far as I can see, it will still give the functionality we wanted with bucket
so I guess I'll prepare a PR 🙂
thank you so much for reporting it quickly so we got it fixed straight away
glad to help, I think so, thanks for guiding me
it does bother me though that localDateTime is incorrect
can we try something?
if we set localDateTime to be timestamptz, restart the server, and then run the "all metadata" job and then check the db for the SF pictures
GitHub
fix(server): use fileCreatedAt in bucket calculation by etnoy · Pul...
This is a minor fix that uses fileCreatedAt when computing time buckets. This fixes a bug we found for some users that don't use docker but instead run postgres locally. In that case, the db ti...
alternatively, you can just go to that SF photo and refresh the metadata of that file only
if that fixes it, I'll create a separate PR for it
why do you think it's not correct? otherwise I think it's corect to have it without timezone
you said it was taken on 12:29 local time
then localDateTime should say 12:29 in Z
but now it's 2h off
if I understoot that PR correctly,
localDateTime is used for calculation day in the month, wihich was incorrect in some border casesit should be stored exactly as 12:29:44+00 in the database
yes, but if that photo was taken near midnight the day would be a day off 🙂
which is a bug
yes, 14:29 within
localDateTime is not correct, but how would timestamptz help?I think immich stored it as 12:29 but it was converted two hours due to timezone conversion
I might be wrong though, and I unfortunately can't replicate the results you are seeing
therefore I'm bugging you with a little more info so I can fix that, too
🙂
btw, the PR is merged already
so there is still +2h bug
yes, it appears so
What's the issue?
so assets near to midnight could be in the wrong bucket, will check it
this was a long thread, but we got it resolved here: https://github.com/immich-app/immich/pull/4354
GitHub
fix(server): use fileCreatedAt in bucket calculation by etnoy · Pul...
This is a minor fix that uses fileCreatedAt when computing time buckets. This fixes a bug we found for some users that don't use docker but instead run postgres locally. In that case, the db ti...
this is a non-standard setup without docker so the db is in another timezone
so the main issue is fixed, however I also noticed that localDateTime wasn't stored as timestamptz, possibly causing the localDateTime to be 2h off (the timezone here is UTC+2)
This undoes the local date time PR changes though?
yes, assets after 22:00 are in the bucket of the following day 😄
nope, still works! it still orders by localDateTime, but the PR changed the SELECT query to not miss assets in this specific setup
They will be in the wrong bucket now
be aware that many admins can use external db outside docker image with any timezone
What was the actual problem? A database with a different timezone caused an issue for the time bucket queries?
all time-bucket api calls returned empty
hm...I have tested them
or do you mean monthly buckets?
I only tested daily
It is not going to put them in the right group across different time zones now
here is the return for time-buckets:
I think it's possible with that PR,
localDateTime removes the TZ problemWith the fix you mean?
but when the time-bucket api calls uses those timebucket strings (notice 22:00:00 and not midnight) they returned empty
it's necessary just finetune
localDateTime calculationThe way it needs to work is get buckets based on local date time and then query local date time based on those buckets. Of you query based on another field you will probably get wrong data.
Can you verify the time bucket detail endpoint still returns the counts from your screenshot? I don't think the detail endpoint will match anymore.
that screenshot contains monthly buckets from my current timezone, those from UTC-7 are not visible there (are below, 2014)
the count of assets in the bucket can be different of course
I just created a new test case and tested it. The month boundaries still seem to work correctly
I think @etnoy 's solution work's properly
If you get the "bucket statistics" and it returns 10 for this date, do you get ten assets back for the detail query?
the only thing that needs to be fixed is
localDateTime calculation, db is adding +2h offset to that timestamp fieldWhy?
we don't know yet
that's a separate thing I'm investigating
let's focus on the buckets for now so we don't lose track
the PR was for fixing the missing assets problem
OK, I uploaded a bunch of assets with modified exif dates to test it out
Man I'm kind of annoyed. I'm like 99% sure this is not working as intended.
there is an issue with the red car being in the wrong bucket
sorry, let's look at it closely
It is in the wrong bucket because of your fix.
You should revert it and we should offset something by two hours somewhere else probably
this only appears to happen along month boundaries, not day boundaries
It would also happen in day boundaries but the UI is querying by month.
that makes sense
How do you reproduce the issue before the fix? Run the database with a non UTC value?
I am not able to reproduce it, but I have been working with @waclaw66 to reproduce it on their machine
I found a fix, tested it on my machine and it appeared to work
but I realize I didn't test month boundaries, only day boundaries
note that the screenshot above is of several photos, of which the red car is in a different timezone
I'm not even sure that the time buckets are wrong, it could just be a sorting issue
I've checked the time bucket details and count matches
those assets are from different timezones UTC+2, UTC-4, UTC-7
for the month, checking days...
Are you missing one? 3617 vs 3616?
I think I know what the original problem was.
its 0-3616, so 3617
Ah, right.
daily buckets seems to be also correct
the bucket seems to be correct here, the red car being in the wrong bucket seems to be an issue with the UI rather than backend
but I might need to go afk for a while
if you want to revert I have no objections
What I think is happening is:
- bucket queries are working correctly going against local date time
- buckets come back to the server without a timezone
- when the date is serialized into json it is converted into iso, and defaults to the containers timezone, which is +2.
- Then the UI sends the wrong time bucket, which now is the wrong day and no results are returned
so time-buckets are returned with timestamps 2h before midnight
and time-bucket is then queried with a timestamp 2h before the month ends
then date_trunc finds no matches
that's part of the debugging we did earlier today
Right. I think it's like:
1. "Get me time buckets"
2. OK, 2023-01-01 has 10
3. Server says ok, let me send, 2023-01-01 to the UI
4. Server says "this date doesn't have a timezone", must be +2, since that is the timezone I am in
5. Server sends 2022-12-31 22:00:00 to the UI
6. UI says "give me time buckets for "2022-12-31 22:00:00"
7. Server does a "day truncate" of "2022-12-31" and ends up with "22-12-31" and says there are not results
I think the issue is in step 5, the returned date doesn't have a timezone and the server needs to to serialize it into an iso timestamp, so it assumes it is local time
Sounds plausible
to avoid all this TZ issues would be helpful to send to the db command that force using UTC
I think that would fix it, yeah. When sending, it needs to be specifically told "this date it UTC". When receiving, it should also be whatever we sent back in the first place. It just uses the value as is, but we can also move it to UTC either way.
Importantly, this all works in 1.81.1
Did you test it already?
I unfortunately am only on my phone right now but can test later tonight
What did you mean by this?
We didn't have the timebucket PR in 1.81.1
Time buckets return the time 2h before month end, this was also in1.81.1
do you know how to force UTC? I can test it right now
Exactly my point. We only touched time-bucket and not time-buckets in the pr
true
Hold on, are you saying the 22 hour scenario was happening before we merged the local time bucket change yesterday?
The difference is that time-bucket expects midnight utc after pr
yes
Yes, but that is 22h utc, midnight local time
Right, but that's isn't a problem when you are querying against time stamps with time zones in the database
Maybe the correct fix is to have buckets return midnight utc, too
I think you mean 22h local time, midnight UTC?
Yes, we should probably aim for that, buckets need to be midnight utc
local time 22h = midnight utc though
so it doesn't matter when you query a date time with timezone
It's only an issue when you query a date time without a time zone
Drum roll: localdatetime is without tz!
I found this earlier today. Check asset entity
No, 22h local time has a timezone of +2
Oh, yes, localDateTime is without a time zone by design.
If you stored it "with a timezone" it would be the same value as
fileCreatedAt.I did try that for a long time, trying to set it to have to but no avail
But maybe data needs to be rewritten too
I think it needs to be stored with tz, otherwise it will be converted when passed to the backend from db
I mean there are two options for this:
- Convert
localDateTime to UTC, but then the conversion from fileCreatedAt to localDateTime becomes take fileCreatedAt and offset of the date by the timezone offset. So if a picture was taken at 5pm, it needs to be stored at 5pm UTC.
- Store it in local time (no timezone) and then queries need to be used with the same time zone they were extracted with.
Basically, if a photo was taken at 5pm local time, and lets say 2am UTC we would need to store:
- fileCreatedAt at 2am (UTC)
- localDateTime at 5pm (UTC)I think we already do the first option. 15:00Z means 1500 local time
So that the day and month is correct for buckets and memories
Correct
But since I wrote timestamp without tz, it does wonky conversions
Oh, I think that code might be wrong.
Ok
I have been at this all day, my head is becoming confused, haha
I think there were two different ways to do this and I did one in the migration and you did another one in the metadata service
That makes sense
So a metadata extraction should solve it?
It'll force an update of localdatetime
What I do in the migration is take the 2am date and display it at the local time zone, so I get 5pm (in local timezone), not 5pm UTC. And I store that in the database column. Since there is no time zone 5pm is stored.
Ah. That SQL code just went right over my head anyway
This timezone stuff always is so complicated and confusing whenever it gets update lol.
Tell me about it, haha
So plan of action: revert my latest fix and do metadata refresh, then confirm if issue is fixed? Then maybe edit the migration?
@waclaw66 can you try refreshing the metadata of the sf photo we checked? Three dots menu
Then check db and see localdatetime of the asset
I think there is another bug though. though, which explains why your fixed worked in the first place. T he date_trunc needs to run on the same field in the stats and detail queries. It looks like we only ever updated the detail one, not the stats one to use localDateTime.
Yes, there was certainly a mismatch there. I might have fixed it in the wrong place
yes, it was refresh to correct value of 12:29, but with the fix applied,
fileCreatedAt has to be there, otherwise time-bucket returns empty jsonYou revert "fixed" it by matching the fields, but I think also changing the other one to local date time would have matched them against the other column
When you run the sql from the migration did it update it to the wrong value or the correct one?
This one
the original PR set it to 14:29 (which is wrong), after that query "fix" and refresh of metadata it was fixed to 12:29
So two hours off
I wonder if setting it to "local time" only works this way if the database is in UTC, which isn't the case here.
That might be where the timestamp without tz comes in?
Or no
We're talking raw SQL migration code
OK. I'm thinking we do just switch this to a timestamptz column, fix both bucket queries to use
localDateTime, ditch the initial migration to set localDateTime, and then make sure we put the value as "5pm UTC" in the metadata service, which I think is already happening. With all the done, it doesn't matter if we make sure it is "using UTC" in the values sent back and forth.Agreed
I thought it would be simpler using a timestamp without a timezone, but it looks like it is more of a headache, so we can use time stamp with timezone and store in that column the local time, but stored with a UTC timezone.
This migration hasn't run in production yet, should I just ammend it?
Fine by me, it's only in git
Worst case, it's a metadata job to fix
it means for me, to revert the db migration right?
Yes, can you do that easily? If not you can manually run the steps.
no problem for me
Hmm with a time stamp with time zone I'm running into the immutable thing, since EXTRACT(timestamp with timezone) is stable not immutable, which is different from EXTRACT(timestamp)
Oh
Dang
I think if the viewing timezone changes or something the index is now wrong? Something based on the timezone makes the index not immutable I guess.
I can think more on this topic later
This seemed to work:
About to head off for the trail
Have fun!
Looks good
enjoy!
@waclaw66 - this is the set of changes we're thinking of making. Can you verify if you still have an issue or not?
https://github.com/immich-app/immich/pull/4358/files
going to test it...
Awesome, will test later
no, that doesn't work as expected, now the border between day buckets is at 15:00, for UTC-7 assets
now update script ignores
"exif"."timeZone", there is no timezone offset calculated within localDateTime
basically all fileCreatedAt fields are just copied to localDateTime
Anyway using filesystem file creation date fileCreatedAt for bucketing and other sorting functions is not a good idea, you should rather use dateTimeOriginal from exif if possible. When you do some file postprocessing (e.g. color corrections), some apps modify file creation date (because of recreation of the file) and those assets are wrongly sorted to the future then. I have more than 10 years old albums with such postprocessed assets and they are shown upper then albums with much newer assets.A few clarifications/explanations:
1.
dateTimeOriginal is on exif and doesn't exist until after metadata extraction, but we need to see and sort assets before then otherwise you'll never see in in the UI if metadata extraction fails
2. fileCreatedAt is dateTimeOriginal after metadata extraction runs. It is updated to the new value.
3. There is no easy way to do a migration and use localDateTime with a timezone, so it's required to run metadata extraction to correctly populate the field originally.
4. The naming of fileCreatedAt is potentially confusing, but it starts with file created date and is updated to exif.dateTimeOriginal, so it essentially is dateTimeOriginal but we needed an initial value when the asset is first created.
In the code, having localDateTime be not null makes it significantly simpler in a lot of places, so when the column is added it needs a starting value. I have no idea how to take exifInfo.dateTimeOriginal + exifInfo.timeZone come up with a timezone offset interval and add that to fileCreatedAt in a migration, so we're just copying fileCreatedAt.1. understood
2. need investigate, not always true
3. do I need to run metadata extraction job now?
4. sure
2. is 100% true. If the job is successful, it will copy dateTimeOriginal to fileCreatedAt
there was adding timezone offset to
localDateTime in the original PR and it worked (considering +2h offset due to timezone type)That code is still there as is.
really? I see that it was removed...
Oh, you mean in the initial migration itself. I thought you were talking about the offset getting applied in the metadata extraction process.
No, we had to remove it. Since the column is now a timestamp with time zone that doesn't work anymore.
ahh, there is another place where the timezone is calculated, understood
Yeah, I think you need to run the job to have correct data before you can validate if it is working correctly or not. You can try on a small batch first and then potentially re-process everything.
ok, I will do metadata extraction job...
In the web you can multi-select a group of assets and re-run just those ones. Maybe do that for a few days/month and verify it?
If you don't want to have to run it for everything immediately.
it is simplied to run that job in my case, too much images with timezone offsets
still not correct, there is bucket border at 22:00 after metadata job
I'll test later tonight
checked that
EXTRACT(DAY FROM entity.localDateTime AT TIME ZONE 'UTC') does the good job - returns 27, nevertheless the asset appears in the bucket of 28thfor the reference
Is the value in the database correct or no?
database is correct, it's caused by the client browser timezone offset 😄
when I set Windows to UTC, it's shown in the right bucket
I think we should go through one at a time and make sure each of these is correct:
- Correct time in
localDateTime
- Correct bucket list is returned
- Correct assets are returned for the specific bucket
- Correct assets are label under the right day on the UI
We need to figure out if it is a database logic issue, server logic issue, or client logic issue.I think that everything is caused by the UI, because...
- buckets are fixed by the UTC on web client - that shouldn't have any effect
- wrong month (-1) is shown on the scrollbar timeline when system is set to UTC and db to UTC+2, bucket dates are correct, but the scrollbar shows month-1 😄
try to play just with the client side timezone
So you can validate that the item that shows up incorrectly in the UI is being returned in a different time bucket, the correct one?
I'm sorry, need to leave, I'll check it tomorrow...
I think you are right.
localDateTime is now a bit weird. It is a date with a UTC timezone, but should be interpreted as "local time". I think that is causing it to be labelled incorrectly. To clarify are the photos grouped together correctly, they just have the wrong labels or are photos in the wrong groups itself?Client timezone affects grouping to buckets, that's weird
Yes, and I think it is because local date time has a timezone of +0 and is being adjusted accordingly.
Will have to look at it with @etnoy later. Thanks for helping debug though.
I might just test locally with the new e2e branch
Set up a number of tests for it
Yeah, that's a good idea.
First quick test: assets are sorted as expected
(dates are set in the future on purpose)
scratch that, I hadn't done the migrations correctly
first check is localDateTime. Unfortunately it appears 2h off (I'm UTC+2 just, too).
the metadata service extracts the correct localDateTime: 2024-04-01T00:47:40.000Z
I'd like to determine if the API is working correctly or not with the query and returning assets.
but the db shows it as 2024-03-31 22:47:40+00
yep, taking it step by step here first
22:47 should be correct right?
no, 00:47:40.000Z is the correct localdatetime
the metadata is obviously edited to create this scenario
The picture was taken 12:47 am?
yes, 12:47 local time
(I'm not used to AM/PM so I think 00:47:40 local time)
Got it. After running the metadata job it is wrong then?
Or is this just after the migration?
yes, I suspect the db is converting it 2h
this is after metadata extraction, I didn't check the after-migration data closely enough
I have a console.log in metadata service printing out the timestamp it tries to save to the db: 2024-04-01T00:47:40.000Z
That is the expected value, yes?
yes, that is indeed correct
so there appears to be some kind of conversion happening between metadata service and the db
How are you printing the value out?
just a quick'n'dirty console.log
Can you print to iso string?
sure
me right now
Lol
immich_microservices-dev | 2024-04-01T00:47:40.000Z
this is when I refresh metadata of that asset
And how are you verifying the value in the database?
Just a select?
yes, I do a select in psql
And it comes back as 22 UTC 0? That seems problematic
i...hate...timezones haha
That should be fine
why on earth would the db take a nice string WITH TIME ZONE and save it as a DIFFERENT timezone?
it really converts 00:47:40Z to 22:47:40+00
This is the same asset id?
haven't compared, but I created those six assets with create date in 2024 for simplicity's sake
and uploaded to immich, should be the exact same
Can you set a timezone on the date object before it is saved?
Maybe something stupid in typeorm
isn't there already a timezone? the timestamp says Z = UTC
You are just doing new Date
I'm digging through the docs now
maybe I should use luxor
Oh wait. Can you log the SQL for the update statement?
Does this only happen when the immich-microservices is running with a timezone? I never saw these issues.
yes, immich-server and immich-microservices is running in UTC+2 timezone
this is also the default now since I pushed the timezone PR the other day
one second
wait
are you kidding me
it now has the correct time in the db
never mind, i didn't save the file
but it's still correct...looking good
as far as I can tell, everything works correctly. localDateTime is correct, api calls are correct, browser looks fine etc.
I tried resetting the state to after migration but before metadata extraction, and it's a bit odd, with April 1st appearing twice:
After metadata extraction, still two April 1st, but different
will try to get db logs
query: UPDATE "assets" SET "localDateTime" = $1, "updatedAt" = CURRENT_TIMESTAMP WHERE "id" IN ($2) RETURNING "updatedAt" -- PARAMETERS: ["2024-04-01T00:47:40.000Z","a70410e6-97b7-4328-9e18-2684ca5a9436"]
I'm pushing a small correction that should fix the issue, now it works after the first metadata extraction
Oh man. Was it not using the most recent value?
I moved an assignment down a few lines, not sure what it was reading before
at least everything checks out now
will wait for @waclaw66 to test tomorrow
myself, I'm off for today 🙂
hokay, I think I have it displaying and grouping in "local time" without timezone stuff getting messed up.
Hi guys, I've tested those latest commits and it looks much better now. Bucket grouping is now correct and client timezone independent. The only small flaw is when the client TZ is set from UTC+2 to UTC it still shows month-1 on the scrollbar. It's probably related to
time-bucket request param with T22:00:00.000Z which still persists for db with UTC+2.Can you show a screenshot of the issue?
windows with UTC+2 - OK ("kvě" meaning May)
If the photos are showing the correct date I think this is as good as it will get currently. I know the scrollbar isn't very exact, especially with many photos. Is the scrollbar any different to 1.81.1?
windows with UTC - flaw ("dub" meaning April)
it's only scrollbar issue, I believe it was in 1.81.1 also, it's all caused by those 22:00
time-bucket queries probablyCan you also check those California photos to see if the local date time was correct in the db? After you refresh their metadata.
Oh and you might need to rerun the migration manually
Do you know how?
done already, everything seems ok,
localDateTime of 12:29 photo is stored as 2014-06-02 14:29:44+02Scrollbar offset has been a bug for a while
I believe it is the regression from this pr https://github.com/immich-app/immich/pull/3536
GitHub
fix(web): scrollbar by jrasm91 · Pull Request #3536 · immich-app/im...
In this PR:
Fix issue where scroll top becomes hidden
Fix issue where scroll line does not go all the way to the bottom
Fix issue where dragging outside of the asset grid (and then coming back) st...
I've checked db timezone with UTC (read about it and there is no harm to change it "on the fly") and then scrollbar month is showing correctly in any client TZ
as I said before, it would be helpfull to set the db timezone to UTC after creating the session
unfortunatelly that typeorm flag
useUTC has no effectGitHub
Sessions timezone for PostgreSQL · Issue #8325 · typeorm/typeorm
Implement the session timezone for PostgreSQL Right now you can config the connection timezone for MySQL databases, but not for PostgreSQL. An option can be use the same timezone property for MySQL...
what is the issue you are seeing with the db?
still the same 22h query for UTC+2 db...
api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-09-30T22:00:00.000Z
that causes that above decribed scrollbar issuebut are you seeing the correct items? How time is shifted under the hood should not matter as long as it works in the end
yes, buckets with asssets are fine now, that's solved, however that scrollbar issue with non UTC db still persists
that's another bug we found while digging buckets
nevertheless the root cause of both is the same - non UTC db
those fixes you've made are just workarounds to make buckets work with non UTC db, or rather vice versa, UTC db is workaround to make buckets and scrollbar work without issues
I think we will merge the current PR to make localDateTime work correctly. Please file a separate issue for the scrollbar so we can track it
I agree
however it's related to time buckets
found the place where scrollbar label is calculated, I'll try to fix it (to make it work with UTC+2 db and non UTC+2 client)
Does the stats time bucket endpoint return 22:00 time bucket labels add well?
I'm sorry, I don't follow, what do you mean by that?
There are two bucket endpoints. Did the first one return 22:00 hour time buckets?
ahh, understood, yes...
because of db UTC+2
and that
timeBucket field is used for scrollbar label, for UTC+2 clients converted the right month, for UTC shows month-1 (because it's UTC already - no conversion)
the problem is that it has a timezone at allI wonder if we could say in the query to date teenage the date
at 'UTC' here
https://github.com/immich-app/immich/blob/fix/time-buckets/server/src/infra/repositories/asset.repository.ts#L485-L495GitHub
immich/server/src/infra/repositories/asset.repository.ts at fix/tim...
Self-hosted photo and video backup solution directly from your mobile phone. - immich-app/immich
I proposed to call
set timezone 'utc' when db session is created, that would solve everythingI will have to check how the label is created for the scrollbar, I didn't look at that yet.
Agreed, but I don't think it is possible with TypeORM yet
GitHub
immich/web/src/lib/components/shared-components/scrollbar/scrollbar...
Self-hosted photo and video backup solution directly from your mobile phone. - immich-app/immich
I do actually it is possible, if we make a call manually after loading typeorm...?
Yeah, this looks like the same issue I fixed for the labels for the date groups. It's adjusting instead of keeping it in the original timezone
tried to fix it in that way, but haven't succeeded yet
The session timezones needs to be sent per connection though, and TypeORM uses a pool of connections, no?
ah, that would complicate things. I think I saw it on stackoverflow somewhere, let's see if I can find it again
It might also require the database session timezone getting fixed first.
the post I found was quite vague, only setting timezone in a sql command
now I understand the actual issue
I think it uses the database setting first, then the timezones setting for the session, then you can also override it by using
at "target"
That would be an easy fix specifically for the time buckets.
The other thing we could look at is how pg parses different data types.so we can add 'at "target"' to the time buckets call?
GitHub
immich/server/src/infra/repositories/asset.repository.ts at fix/tim...
Self-hosted photo and video backup solution directly from your mobile phone. - immich-app/immich
I think something like
date_trunc(... at 'UTC')I'll test and see if it makes a difference at all on my system (where time buckets return 00:00:00Z as desired)
maybe I'll set my docker compose to start postgres in my local timezone to see if that triggers the issue
You could also try using psql to set the connection timezone and run some basic select tests.
Anyways if that doesn't work, then I think we'd have to figure out a session level timezone solution.
Ah, looks like the syntax is
at time zone 'UTC' btwchanging the timezone of an existing postgres container without deleting the db seems tricky...still working on that
I just changed
timezone = 'UTC' within postgresql.conf and reloaded service and it worked immediately, but without Dockerwe're talking about docker, do you think it'll be easy? 😉 (already done that to no avail)
oh wait, c hanged log_timezone and not timezone, maybe now
done! now I can see the issue. The timeline does become quite strange, and keeps updating when you scroll
now on to changing the sql commands
it...just worked!
amazing
I think I can push the fix to the PR straight away
@waclaw66 try to see if the latest push works better for you, with UTC+2
awesome, all lights green!
tested all scenarios and everything works flawlessly
Let's goooooo
not entirely, tested situation when client has UTC-xx
scrollbar shows month-1
I can fix that one. Are the dates coming back from the time bucket API call correctly though? At 00
yes, api gives all
time-bucket like...
Sweet
no matter what timezone db uses
yes, same here 🙂
Out of curiosity how are you deploying the new code @waclaw66 ?
I pushed a fix for the scrollbar if you want to take a look.
quite simple, I have a script that does the same as Dockerfile files, and update automatization when a new release is released
plus systemd service files
seems to be fixed now 🎆
Let's gooooo (
_final_copy (3).txt)I'll just push a smol documentation thing
So you are saying that the timeline should:
- Group assets by local day taken
- Display assets on the local day there were taken
- Work for databases in different timezones
- Work for servers in different timezones
- Work for clients in different timezones
I say we have reached all those goals
but creating automated tests for all this might be tricky, but probably worth it
yes, all of those
specifically, what do you mean by "Display assets on the local day there were taken"?
my old https://github.com/immich-app/immich/issues/3616 can be closed
I get it like when it was friday 13th in the target timezone, then it's showed like Fri 13, no matter that it was Saturday at your home location already
I think we are achieving this now
Yeah. I meant specifically, like if you are viewing it on October 6th, in Europe it might be October 5th in the USA. If you took a picture at the same time, both would show up in October 6th, but now the USA one shows up in October 5th.
So, like @Wale#1914 said, it shows up on the day it was taken regardless where you are currently using the app, which is super nice. Before we were limited to absolute sorting.
yes, I'm really liking this outcome
I really didn't know that I would open pandora's box when saynig "hm...it would be nice to bucket by local day" haha
The only thing we might want to add in the future is detect when a single date has photos from two different timezones/location and potentially sort or sub-group them differently.
But I think sorting by
fileCreatedAt (second) should more or less already accomplish that.yes, that's the drawback we knowingly have added now. If two users are in two timezones and take photos at the same time and both upload to immich, it can be a little strange. But not too strange IMO
Right, they will be inter-woven one here, one there, etc.
We can leave it as is for now and see how it goes. In the future though, we might be able to group them by timezone or something like that.
So show group 1 in the USA all together and show group 2 in Europe all together.
interesting idea, but I'll have to do a timezone detox now and will push such a feature in the future (for my part at least)
haha sounds good. I think the PR is fine to merge now.
agreed!
you can merge it if you want
ooh.. are you sure?
lol oh no
trying to find a gif of the men in black "push the red button" scene, this was almost it
Congrats on getting it fixed! 🚀
Another discovery. I've noticed that I had set during above testing
TZ=UTC within .env. Because of that everything works fine in any timezone (server / db / client).
Unfortunatelly when I removed that TZ from .env file. It started to return time-buckets like 2023-09-30T22:00:00.000Z and empty time-bucket json responses - that's the problem. Server, db and client have UTC+2. Setting db to UTC has no effect.
However api/asset/time-bucket?size=MONTH&isArchived=false&timeBucket=2023-10-01T00:00:00.000Z returns valid data. I believe time-buckets endpoint needs to be fixed to return UTC timestamps.This is on current main?
Yes
Oh no, not again
lol
I don't have the mental capacity to go through all that again lol
While playing with code and trying to fix that, I've discovered that removing
at time zone 'UTC' from https://github.com/immich-app/immich/blob/d2807b8d6ab1a72f37a662423ddda54f41c742ce/server/src/infra/repositories/asset.repository.ts#L491 helps to solve time-bucket empty response, however the month-1 scrollbar issue is back 😄GitHub
immich/server/src/infra/repositories/asset.repository.ts at d2807b8...
Self-hosted photo and video backup solution directly from your mobile phone. - immich-app/immich
So is it applying the translation in the immich-server then?
not sure what do you mean by that
I'll need to set up some e2e tests to reproduce the behavior first, then I can start testing possible solutions
seems that immich-server (TZ not specified) on server (UTC+2) gets
time-buckets correctly in UTC+2 from db (UTC+2), but client receives it in UTC with -2h...
It looks like the second screenshot is returning correctly the data it got back. Imo the date in the first screenshot is wrong. Midnight at GMT +2 is not what we want the query to return.
you are probably right, because
time-buckets sql query returns no timezone result, but is comprehended as GMT+2 in immich-server because of the environment, where TZ is not set and taken from the system settings
when I set
TZ=UTC within .env again...
it seems that the
timestamp without time zone value comprehension is based on system/env settings
as I checked PostgresConnectionOptions there is no way how to force timezone comprehension for timestamp values without timezone, the only way is probably via proccess.env.TZ
I see two ways how to fix that, first not to force UTC using at time zone 'UTC' and deal somehow with timestamps in local timezone, second force immich process to UTC via proccess.env.TZ internally (in the same way as tests do that)
I have solved the same issue in my apps in the second way, backend runs with UTC (regardless of system or db settings) and local timezone operations are done mostly on client side or the client TZ is handed to backend if needed
I've found a simple fix for all of that 😄
Replace line https://github.com/immich-app/immich/blob/3d7e9b71845877e13611f5463276ead14465b316/server/src/infra/repositories/asset.repository.ts#L485
with .addSelect(`to_char(date_trunc('${truncateValue}', "${TIME_BUCKET_COLUMN}" at time zone 'UTC'), 'YYYY-MM-DD')`, 'timeBucket')
The main problem is returning value of timeBucket as Date (although its property of TimeBucketItem is defined as string), it's undesirably converted to timestamp with local timezone and then strigified to UTC by JSON.stringify() while returnnig the response.
no time part, no problemwe can't use to_char since it massively hurts performance, having to scan the whole library every call
unless we create another index, @jrasm91
Imo the problem is the fact that it returns midnight +2 GMT. That is just wrong. Why does it take a timestamp with timezone column, ignore the timezone, and add a random one. This seems like a massive bug. It is just plain returning the wrong value
postgres seems to use a per-connection timezone setting and "helpfully" translates timestamps to your timezone
I don't think you can configure that in typeorm
No, that is not what is happening.
If it sent the date as +2 it would send 22:00+2.
It is correctly sending 00:00+0 and the client somewhere is incorrectly assuming/changing it to 00:00+2
I think it is a problem where the new date object is being constructed
ah, got it
I mean, yes somewhere something if trying to "helpfully" translate the time, but I think postgres is not at fault here, some node package implementation or TypeORM
JSON.stringify() translates it from local to UTC, you cannot affect what db returns unless you set process.env.TZ to UTC
yes, while saving it to Date variable, it's returned without TZ and driver assumes +2 TZ, it's done on postgres db driver level, therefore I worked it around with that stringificationWhere does that happen exactly? Is that in the pg package?
somewhere behind
const raw = await databaseConnection.query(query, parameters);
pg/lib/client.jsIt sounds exactly like this issue, no? Although it is marked as fixed.
https://github.com/typeorm/typeorm/issues/838
I'm going to see if this changes anything:
It looks like this, but look to comment https://github.com/typeorm/typeorm/issues/838#issuecomment-366641483 and DateUtils.ts link
looks promising
It looks like the native javascript date just has limitations with it.
Internally it is just a number (of seconds since 1980)
That doesn't come with any timezone information or something
OK, so I can reproduce, this has the server offset and returns 0 assets
Looks like that didn't change anything.
The type is "timestamp without time zone".....
I can check that
parseInputDatesAsUTC, where to put it?OK I think I fixed it.
Basically adding a conversion to
::timestamptz seems to fix it in the time bucket query seemed to fix it.
Can you try the changes here?
https://github.com/immich-app/immich/pull/4498
I'm not 100% sure what this will do in a non-utc database.it works, but month-1 scrollbar reappeared
so the time bucket stats and detail endpoints are returning the correct values though?
time-buckets returns 2023-06-30T22:00:00.000ZThat is still wrong.
Mine was a non 00 hour, like yours is 22, but the new query changed that to 00 again.
is your database non-utc?
yes, UTC+2 everywhere for now
changing db to UTC has effect now...
time-buckets returns 2023-07-01T00:00:00.000Z
scrollbar ok, I can live with that, but setting process.env.TZ to UTC would solve it for everyoneI mean, this seems to work:
hmm
This also works fine and returns a timestamp with time zone
yes, result seems to be the same
I think the problem is that
at time zone returns a time stamp without a timezone and then that makes nodejs use a local time. Or, you cast it to timestamptz, but then it uses the local database time. So the other option it to have several at time zone 'UTC' everywhere and at the end cast to ::timestamptz.
I think this seems to work for a non-utc database, but always return utc results, which should fix everything else. Do you still get the scrollbar issue with this?awesome, db UTC+2, scrollbar issues no more 🎆
Would these casts have any impact on performance? I would think no, right?
I'm not db expert, but performance is not hurt on my instance, at least nothing visible
How many assets do you have?
currently 18k+
When I run an explain plan, it still is just an sequential scan, which seems expected since this is just a group of assets filtered by date.
So... Still linear complexity?
I assume it is still fine. It seems like it just adds a bunch of annoying "at time zone 'UTC'" stuff.
seems like it got merged, nice work. Haven't tested it yet but maybe I'll write one more of my beloved e2e tests
Yeah. I was thinking about how you could test it.
The issue is essentially the timezone of the database and the timezone of the server being one or both on different timezones than utc.
However, I think we can set the timezone of the database via
UPDATE <db name> SET TIMEZONE='something else'. Then try the query, then change it back to UTC.
This is what I did and verified that it wasn't working correctly. Another test would be set something like process.env.TZ=something else and do the time bucket query, then make sure it still works, and clear it afterwards.
The latter one should not really be a problem unless the query returns time stamp without timezone. I think that is the only place the server timestamp was problematic.