Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: transcode in stats #14418

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: transcode in stats #14418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d348d15
Transcoding storage included in Server Total.
Nov 30, 2024
1959818
tests for including transcode in server total
Nov 30, 2024
26052f4
Skip empty strings and handle fs errors.
Nov 30, 2024
d485522
transcoding in user summaries
Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions i18n/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,7 @@
"toggle_settings": "Toggle settings",
"toggle_theme": "Toggle dark theme",
"total_usage": "Total usage",
"transcoding": "Transcoding",
"trash": "Trash",
"trash_all": "Trash All",
"trash_count": "Trash {count, number}",
Expand Down
15 changes: 14 additions & 1 deletion open-api/immich-openapi-specs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10972,7 +10972,8 @@
"videos": 1,
"diskUsageRaw": 2,
"usagePhotos": 1,
"usageVideos": 1
"usageVideos": 1,
"usageTranscode": 1
}
],
"items": {
Expand All @@ -10986,6 +10987,11 @@
"format": "int64",
"type": "integer"
},
"usageTranscode": {
"default": 0,
"format": "int64",
"type": "integer"
},
"usageVideos": {
"default": 0,
"format": "int64",
Expand All @@ -11001,6 +11007,7 @@
"usage",
"usageByUser",
"usagePhotos",
"usageTranscode",
"usageVideos",
"videos"
],
Expand Down Expand Up @@ -12529,6 +12536,11 @@
"format": "int64",
"type": "integer"
},
"usageTranscode": {
"default": 0,
"format": "int64",
"type": "integer"
},
"usageVideos": {
"format": "int64",
"type": "integer"
Expand All @@ -12548,6 +12560,7 @@
"quotaSizeInBytes",
"usage",
"usagePhotos",
"usageTranscode",
"usageVideos",
"userId",
"userName",
Expand Down
6 changes: 6 additions & 0 deletions server/src/dtos/server.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ export class UsageByUserDto {
@ApiProperty({ type: 'integer', format: 'int64' })
usageVideos!: number;
@ApiProperty({ type: 'integer', format: 'int64' })
usageTranscode: number = 0;
@ApiProperty({ type: 'integer', format: 'int64' })
quotaSizeInBytes!: number | null;
}

Expand All @@ -109,6 +111,9 @@ export class ServerStatsResponseDto {
@ApiProperty({ type: 'integer', format: 'int64' })
usageVideos = 0;

@ApiProperty({ type: 'integer', format: 'int64' })
usageTranscode = 0;

@ApiProperty({
isArray: true,
type: UsageByUserDto,
Expand All @@ -120,6 +125,7 @@ export class ServerStatsResponseDto {
diskUsageRaw: 2,
usagePhotos: 1,
usageVideos: 1,
usageTranscode: 1,
},
],
})
Expand Down
1 change: 1 addition & 0 deletions server/src/interfaces/user.interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface UserStatsQueryResponse {
usagePhotos: number;
usageVideos: number;
quotaSizeInBytes: number | null;
encodedVideoPaths: string[];
}

export interface UserFindOptions {
Expand Down
5 changes: 5 additions & 0 deletions server/src/repositories/user.repository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ export class UserRepository implements IUserRepository {
'usageVideos',
)
.addSelect('users.quotaSizeInBytes', 'quotaSizeInBytes')
.addSelect(
`ARRAY_AGG(assets.encodedVideoPath) FILTER (WHERE assets.encodedVideoPath IS NOT NULL)`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is gonna be veeeery slow

'encodedVideoPaths',
)
.leftJoin('users.assets', 'assets')
.leftJoin('assets.exifInfo', 'exif')
.groupBy('users.id')
Expand All @@ -130,6 +134,7 @@ export class UserRepository implements IUserRepository {
stat.usagePhotos = Number(stat.usagePhotos);
stat.usageVideos = Number(stat.usageVideos);
stat.quotaSizeInBytes = stat.quotaSizeInBytes;
stat.encodedVideoPaths = stat.encodedVideoPaths || [];
}

return stats;
Expand Down
20 changes: 18 additions & 2 deletions server/src/services/server.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as fs from 'node:fs';
import { SystemMetadataKey } from 'src/enum';
import { IStorageRepository } from 'src/interfaces/storage.interface';
import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface';
Expand Down Expand Up @@ -177,8 +178,16 @@ describe(ServerService.name, () => {
});
});

vi.mock('fs', async () => {
const actual = (await vi.importActual<typeof fs>('fs'))!;
return {
...actual,
statSync: vi.fn().mockReturnValue({ size: 500 }),
};
});
Comment on lines +181 to +187
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now how you should be doing that. You should be mocking repo methods if you have to


describe('getStats', () => {
it('should total up usage by user', async () => {
it('should total up usage by user and server', async () => {
userMock.getUserStats.mockResolvedValue([
{
userId: 'user1',
Expand All @@ -189,6 +198,7 @@ describe(ServerService.name, () => {
usagePhotos: 1,
usageVideos: 11_345,
quotaSizeInBytes: 0,
encodedVideoPaths: ['upload/encoded-video/video1.mp4'],
},
{
userId: 'user2',
Expand All @@ -199,6 +209,7 @@ describe(ServerService.name, () => {
usagePhotos: 100,
usageVideos: 23_456,
quotaSizeInBytes: 0,
encodedVideoPaths: ['upload/encoded-video/video2.mp4', 'upload/encoded-video/video3.mp4'],
},
{
userId: 'user3',
Expand All @@ -209,15 +220,17 @@ describe(ServerService.name, () => {
usagePhotos: 900,
usageVideos: 87_654,
quotaSizeInBytes: 0,
encodedVideoPaths: [''],
},
]);

await expect(sut.getStatistics()).resolves.toEqual({
photos: 120,
videos: 31,
usage: 1_123_455,
usage: 1_124_955,
usagePhotos: 1001,
usageVideos: 122_455,
usageTranscode: 1500,
usageByUser: [
{
photos: 10,
Expand All @@ -228,6 +241,7 @@ describe(ServerService.name, () => {
userName: '1 User',
userId: 'user1',
videos: 11,
usageTranscode: 500,
},
{
photos: 10,
Expand All @@ -238,6 +252,7 @@ describe(ServerService.name, () => {
userName: '2 User',
userId: 'user2',
videos: 20,
usageTranscode: 1000,
},
{
photos: 100,
Expand All @@ -248,6 +263,7 @@ describe(ServerService.name, () => {
userName: '3 User',
userId: 'user3',
videos: 0,
usageTranscode: 0,
},
],
});
Expand Down
16 changes: 16 additions & 0 deletions server/src/services/server.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common';
import * as fs from 'node:fs';
import { serverVersion } from 'src/constants';
import { StorageCore } from 'src/cores/storage.core';
import { OnEvent } from 'src/decorators';
Expand Down Expand Up @@ -131,11 +132,26 @@ export class ServerService extends BaseService {
usage.usageVideos = user.usageVideos;
usage.quotaSizeInBytes = user.quotaSizeInBytes;

for (const path of user.encodedVideoPaths) {
if (path.trim() === '') {
continue;
}

try {
const stats = fs.statSync(path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we already have a repo method for that. If we don't, we should add one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is terribly slow and will scale poorly as the number of videos increases, both in fetching every single path from the DB at once and in stat'ing them like this.

usage.usageTranscode += stats.size;
} catch (error) {
console.error(`Error accessing file at path "${path}":`, (error as Error).message);
}
}

serverStats.photos += usage.photos;
serverStats.videos += usage.videos;
serverStats.usage += usage.usage;
serverStats.usage += usage.usageTranscode; // Necessary because transcoded videos don't count toward quota
serverStats.usagePhotos += usage.usagePhotos;
serverStats.usageVideos += usage.usageVideos;
serverStats.usageTranscode += usage.usageTranscode;

serverStats.usageByUser.push(usage);
}
Expand Down
19 changes: 11 additions & 8 deletions web/src/lib/components/admin-page/server-stats/server-stats-panel.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
usage: 0,
usagePhotos: 0,
usageVideos: 0,
usageTranscode: 0,
usageByUser: [],
},
}: Props = $props();
Expand Down Expand Up @@ -93,10 +94,11 @@
class="mb-4 flex h-12 w-full rounded-md border bg-gray-50 text-immich-primary dark:border-immich-dark-gray dark:bg-immich-dark-gray dark:text-immich-dark-primary"
>
<tr class="flex w-full place-items-center">
<th class="w-1/4 text-center text-sm font-medium">{$t('user')}</th>
<th class="w-1/4 text-center text-sm font-medium">{$t('photos')}</th>
<th class="w-1/4 text-center text-sm font-medium">{$t('videos')}</th>
<th class="w-1/4 text-center text-sm font-medium">{$t('usage')}</th>
<th class="w-1/5 text-center text-sm font-medium">{$t('user')}</th>
<th class="w-1/5 text-center text-sm font-medium">{$t('photos')}</th>
<th class="w-1/5 text-center text-sm font-medium">{$t('videos')}</th>
<th class="w-1/5 text-center text-sm font-medium">{$t('usage')}</th>
<th class="w-1/5 text-center text-sm font-medium">{$t('transcoding')}</th>
</tr>
</thead>
<tbody
Expand All @@ -106,14 +108,14 @@
<tr
class="flex h-[50px] w-full place-items-center text-center odd:bg-immich-gray even:bg-immich-bg odd:dark:bg-immich-dark-gray/75 even:dark:bg-immich-dark-gray/50"
>
<td class="w-1/4 text-ellipsis px-2 text-sm">{user.userName}</td>
<td class="w-1/4 text-ellipsis px-2 text-sm"
<td class="w-1/5 text-ellipsis px-2 text-sm">{user.userName}</td>
<td class="w-1/5 text-ellipsis px-2 text-sm"
>{user.photos.toLocaleString($locale)} ({getByteUnitString(user.usagePhotos, $locale, 0)})</td
>
<td class="w-1/4 text-ellipsis px-2 text-sm"
<td class="w-1/5 text-ellipsis px-2 text-sm"
>{user.videos.toLocaleString($locale)} ({getByteUnitString(user.usageVideos, $locale, 0)})</td
>
<td class="w-1/4 text-ellipsis px-2 text-sm">
<td class="w-1/5 text-ellipsis px-2 text-sm">
{getByteUnitString(user.usage, $locale, 0)}
{#if user.quotaSizeInBytes}
/ {getByteUnitString(user.quotaSizeInBytes, $locale, 0)}
Expand All @@ -129,6 +131,7 @@
{/if}
</span>
</td>
<td class="w-1/5 text-ellipsis px-2 text-sm">{getByteUnitString(user.usageTranscode, $locale, 0)}</td>
</tr>
{/each}
</tbody>
Expand Down
Loading