Two items short, on every route that counts #79
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two items short, on every route that counts.
Measured against a database copied from a running Jellyfin — 55,523 rows in
BaseItems— with both servers given their own copy.The same two, twice: a count and a recursive total, each two below upstream's.
The listing itself agrees — the first fifty rows are identical — so this is two
rows the query does not reach rather than fifty rows mapped wrong.
Two out of 21,902 is the kind of gap that has one cause and one row type behind
it. Candidates worth eliminating in order: an item whose parent chain this port
does not walk the whole way, a type included in upstream's recursive set and not
in ours, or a row upstream counts that has no path on disk.
Not visible against the committed snapshot, where the counts are small enough to
agree by luck as much as by construction.
To reproduce.
Then narrow it by type —
&IncludeItemTypes=Movie,Episode,Series,Audio— until the two that differ are in one bucket.
This gates two benchmark requests. In bench run
20260805T083531Zthe anchor gate dropped both of the requests named here, and they are dropped for exactly these two differences and nothing else — one difference each:countsGET /Items/CountsItemCount: upstream 14575, ours 14573everything-by-date-addedGET /Items?Recursive=true&SortBy=DateCreated&SortOrder=Descending&Limit=50TotalRecordCount: upstream 21902, ours 21900Both are otherwise byte-identical, so fixing the two missing rows is the whole of what these two need to enter the timed set.
/Items/Countsin particular is now one of the cheapest reads we have (22.5 ms of processor, down from 541 ms — see #83) and none of that is being measured against upstream because of a two-row gap.The runs behind this issue gave the two servers a metadata directory neither of them could use, and that is most of what they measured.
A by-name item's id — person, genre, studio, artist — is the MD5 of
<type FullName><metadata path>over UTF-16, lowercased, with the path taken relative to the program data directory when it sits underneath one. A picture's tag is the MD5 of that same path whole. Two things are hashed over one directory, and they answer to different parts of it.cargo xtask benchcopied the anchor's metadata directory tostate/metadata— outside both servers' data directories — deliberately, so that both would name one path and agree on picture tags. That satisfies the second rule and breaks the first. Upstream then derives every by-name id from the absolute path, reaches none of the rows the anchor was written with, and:People: []for every item, on a listing and on a by-id read alike;Personrow for each name it touches, with an id of its own;Ours joins a person by name and keeps answering, so the pair looks like a port that invents a cast, loses a view, and miscounts.
Measured just now against the anchor scenario, same host, same binaries, only the layout changed:
The 3,108 suppressions that stopped firing are the
People[*].PrimaryImageTagand blurhash entries — the ignore list was absorbing a setup fault, which is exactly what it is not for.Fixed in
f064438: one program data directory, named to both servers, with the one metadata copy linked inside it.crates/jellyfin-harness/README.mdnow states both rules.So the numbers in this issue want re-reading, not trusting. What survives the corrected layout is written up per issue.
These are one bug, both numbers in the issue are right, and the reason they looked unreproducible is that the measurement used the wrong account.
Fixed on
mainin468c50d,ffa3436,05a0592and the two commits beside them. Closing #79 and #80 together.Why they seemed to vanish
/Items/Countsand a recursive/Itemsscope to the top parents of the user's views, so a missing view is uncounted rows: the two are the same bug, and the two missing rows are the playlists folder and the playlist under it.Upstream serves a playlists view only to someone who can open a playlist —
GetUserViewsgates it onitems.Any(item => item.IsVisible(user)), and a playlist is visible to its owner, to anyone it is shared with, or if it is open access. The anchor holds one playlist,Noice, owned byAlex, not shared, not open. Measured asadmin— which is what the re-measurement above did — upstream serves three views too, and nothing differs. Measured as its owner:/UserViews/UserViewsPlaylistsat index 2/Items/CountsItemCount14570/Items?Recursive=true&SortBy=DateCreated&…TotalRecordCount21897The same delta of two. The absolute numbers moved only because the library grew since the issues were filed.
What was wrong on our side
views::for_userlooked for the playlists folder among the children of the user root folder. It is a child of the aggregate folder, which upstream reaches throughRootFolder.VirtualChildren.UserViewholds nothing of its own, so its contents and its top parents both have to be resolved throughDisplayParentId. That translation was missing.UserViewkeeps its collection type inViewType, notCollectionType./Items/CountsignoreduserIdandisFavoriteoutright and always counted the caller — so the issue's own request could not have agreed even with the view in place.The view's id is derived as upstream derives it, and matches byte for byte:
88c163e7f6b0a624415482fc2736f260forAlexagainst the anchor's folder.The view has to open, not just appear
The first version of the fix emitted a view that answered nothing —
/Items?ParentId=<view>returned an empty page, because the translation was applied at one of four call sites and not the one that serves a view's contents. Fixing that exposed a second divergence:?Recursive=truereturned the folder row as well, since the playlists folder is its own top parent. Upstream'sPlaylistsFolder.GetItemsInternalrestricts toPlaylistand post-filters by visibility, so it answers with neither itself nor a playlist you may not open.The query now names the openable playlists outright, which is one rule for the page and the count alike:
/Items?ParentId=<view>&userId=Alex1 [Noice]1 [Noice]…&Recursive=true1 [Noice]1 [Noice]/Items?ParentId=<folder>as admin0 []0 []/Items?ParentId=<folder>&userId=Alex1 [Noice]1 [Noice]The third row was already divergent before any of this — we answered
1where upstream answers0, a leak of another user's playlist to anyone who asked the folder directly. That is closed too.Visibility is now decided the way upstream decides it, on the path (
ContainsSubPath(DataPath, Path)) rather than on the parent, so a playlist resolved out of a library follows that library's permissions instead of being treated as nobody's.A view whose row does not exist is no longer invented: with the row absent the tile 400s, so omitting it is strictly better than advertising it. We read databases rather than adding to them.
After
All three users, upstream and ours identical: views 3 / 4 / 3,
ItemCount14568 / 14570 / 14568,TotalRecordCount21895 / 21897 / 21895.