mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-03-10 10:33:11 +03:00
fix: address code review findings for rollover storage
- C1: Rewrite breakdown endpoint with JOIN queries (eliminates N+1) - C2: Use getUnitSize() for consistent MiB/GiB display - C3: Add retain_policy to global FrigateConfig TypeScript type - I1: Only fetch breakdown data when rollover mode is active - I2: Handle NULL end_time for in-progress events/reviews - I5: Add model_validator warning when rollover + days are both set - S4: Hide breakdown when total is 0
This commit is contained in:
parent
154246331c
commit
dad34a71cc
@ -63,91 +63,90 @@ def get_recordings_storage_usage(request: Request):
|
||||
dependencies=[Depends(allow_any_authenticated())],
|
||||
)
|
||||
def get_recordings_storage_breakdown(request: Request):
|
||||
"""Return storage usage broken down by category: overwritable, event retention, and protected."""
|
||||
"""Return storage usage in MB broken down by category: overwritable, event retention, and protected."""
|
||||
|
||||
now = datetime.now().timestamp()
|
||||
|
||||
# 1. Protected (indefinite): recordings overlapping retain_indefinitely events
|
||||
retained_events = (
|
||||
Event.select(Event.camera, Event.start_time, Event.end_time)
|
||||
.where(Event.retain_indefinitely == True, Event.has_clip == True)
|
||||
# Uses a JOIN to avoid N+1 query problem
|
||||
protected_query = (
|
||||
Recordings.select(
|
||||
Recordings.id,
|
||||
Recordings.segment_size,
|
||||
)
|
||||
.join(
|
||||
Event,
|
||||
on=(
|
||||
(Event.camera == Recordings.camera)
|
||||
& (Event.start_time < Recordings.end_time)
|
||||
& (
|
||||
(Event.end_time.is_null())
|
||||
| (Event.end_time > Recordings.start_time)
|
||||
)
|
||||
),
|
||||
)
|
||||
.where(
|
||||
Event.retain_indefinitely == True,
|
||||
Event.has_clip == True,
|
||||
Recordings.segment_size > 0,
|
||||
)
|
||||
.distinct()
|
||||
.namedtuples()
|
||||
)
|
||||
|
||||
protected_size = 0.0
|
||||
protected_recording_ids = set()
|
||||
for event in retained_events:
|
||||
end_time_clause = (
|
||||
Recordings.start_time < event.end_time
|
||||
if event.end_time is not None
|
||||
else True
|
||||
)
|
||||
overlapping = (
|
||||
Recordings.select(Recordings.id, Recordings.segment_size)
|
||||
.where(
|
||||
Recordings.camera == event.camera,
|
||||
end_time_clause,
|
||||
Recordings.end_time > event.start_time,
|
||||
Recordings.segment_size > 0,
|
||||
)
|
||||
.namedtuples()
|
||||
)
|
||||
for rec in overlapping:
|
||||
if rec.id not in protected_recording_ids:
|
||||
protected_recording_ids.add(rec.id)
|
||||
protected_size += rec.segment_size
|
||||
for rec in protected_query:
|
||||
protected_recording_ids.add(rec.id)
|
||||
protected_size += rec.segment_size
|
||||
|
||||
# 2. Event retention (aging out): recordings overlapping non-expired review segments
|
||||
# Use global config for expiry thresholds
|
||||
config = request.app.frigate_config
|
||||
alert_expire = now - timedelta(days=config.record.alerts.retain.days).total_seconds()
|
||||
detection_expire = (
|
||||
now - timedelta(days=config.record.detections.retain.days).total_seconds()
|
||||
)
|
||||
|
||||
active_reviews = (
|
||||
ReviewSegment.select(
|
||||
ReviewSegment.camera,
|
||||
ReviewSegment.start_time,
|
||||
ReviewSegment.end_time,
|
||||
# Include in-progress reviews (end_time IS NULL) as they are not overwritable
|
||||
event_query = (
|
||||
Recordings.select(
|
||||
Recordings.id,
|
||||
Recordings.segment_size,
|
||||
)
|
||||
.join(
|
||||
ReviewSegment,
|
||||
on=(
|
||||
(ReviewSegment.camera == Recordings.camera)
|
||||
& (ReviewSegment.start_time < Recordings.end_time)
|
||||
& (
|
||||
(ReviewSegment.end_time.is_null())
|
||||
| (ReviewSegment.end_time > Recordings.start_time)
|
||||
)
|
||||
),
|
||||
)
|
||||
.where(
|
||||
Recordings.segment_size > 0,
|
||||
(
|
||||
(ReviewSegment.severity == "alert")
|
||||
& (ReviewSegment.end_time >= alert_expire)
|
||||
)
|
||||
| (
|
||||
(ReviewSegment.severity == "detection")
|
||||
& (ReviewSegment.end_time >= detection_expire)
|
||||
)
|
||||
(ReviewSegment.end_time.is_null())
|
||||
| (
|
||||
(ReviewSegment.severity == "alert")
|
||||
& (ReviewSegment.end_time >= alert_expire)
|
||||
)
|
||||
| (
|
||||
(ReviewSegment.severity == "detection")
|
||||
& (ReviewSegment.end_time >= detection_expire)
|
||||
)
|
||||
),
|
||||
)
|
||||
.distinct()
|
||||
.namedtuples()
|
||||
)
|
||||
|
||||
event_size = 0.0
|
||||
event_recording_ids = set()
|
||||
for review in active_reviews:
|
||||
end_time_clause = (
|
||||
Recordings.start_time < review.end_time
|
||||
if review.end_time is not None
|
||||
else True
|
||||
)
|
||||
overlapping = (
|
||||
Recordings.select(Recordings.id, Recordings.segment_size)
|
||||
.where(
|
||||
Recordings.camera == review.camera,
|
||||
end_time_clause,
|
||||
Recordings.end_time > review.start_time,
|
||||
Recordings.segment_size > 0,
|
||||
)
|
||||
.namedtuples()
|
||||
)
|
||||
for rec in overlapping:
|
||||
if (
|
||||
rec.id not in protected_recording_ids
|
||||
and rec.id not in event_recording_ids
|
||||
):
|
||||
event_recording_ids.add(rec.id)
|
||||
event_size += rec.segment_size
|
||||
for rec in event_query:
|
||||
if rec.id not in protected_recording_ids:
|
||||
event_size += rec.segment_size
|
||||
|
||||
# 3. Total recordings size
|
||||
total_size = (
|
||||
@ -159,12 +158,14 @@ def get_recordings_storage_breakdown(request: Request):
|
||||
# Overwritable = total - protected - event
|
||||
overwritable_size = max(0.0, total_size - protected_size - event_size)
|
||||
|
||||
# All values are in MB (matching segment_size column units)
|
||||
return JSONResponse(
|
||||
content={
|
||||
"total": round(total_size, 2),
|
||||
"overwritable": round(overwritable_size, 2),
|
||||
"event_retention": round(event_size, 2),
|
||||
"protected": round(protected_size, 2),
|
||||
"units": "MB",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@ -1,13 +1,16 @@
|
||||
import logging
|
||||
from enum import Enum
|
||||
from typing import Optional, Union
|
||||
|
||||
from pydantic import Field
|
||||
from pydantic import Field, model_validator
|
||||
|
||||
from frigate.const import MAX_PRE_CAPTURE
|
||||
from frigate.review.types import SeverityEnum
|
||||
|
||||
from ..base import FrigateBaseModel
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
__all__ = [
|
||||
"RecordConfig",
|
||||
"RecordExportConfig",
|
||||
@ -152,6 +155,17 @@ class RecordConfig(FrigateBaseModel):
|
||||
description="Indicates whether recording was enabled in the original static configuration.",
|
||||
)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def warn_rollover_with_days(self) -> "RecordConfig":
|
||||
if self.retain_policy == RetainPolicyEnum.continuous_rollover:
|
||||
if self.continuous.days > 0 or self.motion.days > 0:
|
||||
logger.warning(
|
||||
"retain_policy is 'continuous_rollover' - continuous.days and "
|
||||
"motion.days are ignored. Recordings will fill available disk "
|
||||
"space and oldest footage will be overwritten when needed."
|
||||
)
|
||||
return self
|
||||
|
||||
@property
|
||||
def event_pre_capture(self) -> int:
|
||||
return max(
|
||||
|
||||
@ -543,6 +543,7 @@ export interface FrigateConfig {
|
||||
record: {
|
||||
enabled: boolean;
|
||||
enabled_in_config: boolean | null;
|
||||
retain_policy: "time" | "continuous_rollover";
|
||||
events: {
|
||||
objects: string[] | null;
|
||||
post_capture: number;
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
import { CombinedStorageGraph } from "@/components/graph/CombinedStorageGraph";
|
||||
import { StorageGraph } from "@/components/graph/StorageGraph";
|
||||
import { getUnitSize } from "@/utils/storageUtil";
|
||||
import { FrigateStats } from "@/types/stats";
|
||||
import { useMemo } from "react";
|
||||
import {
|
||||
@ -35,12 +36,14 @@ export default function StorageMetrics({
|
||||
}: StorageMetricsProps) {
|
||||
const { data: cameraStorage } = useSWR<CameraStorage>("recordings/storage");
|
||||
const { data: stats } = useSWR<FrigateStats>("stats");
|
||||
const { data: storageBreakdown } = useSWR<StorageBreakdown>(
|
||||
"recordings/storage/breakdown",
|
||||
);
|
||||
const { data: config } = useSWR<FrigateConfig>("config", {
|
||||
revalidateOnFocus: false,
|
||||
});
|
||||
const isRollover =
|
||||
config?.record?.retain_policy === "continuous_rollover";
|
||||
const { data: storageBreakdown } = useSWR<StorageBreakdown>(
|
||||
isRollover ? "recordings/storage/breakdown" : null,
|
||||
);
|
||||
const { t } = useTranslation(["views/system"]);
|
||||
const timezone = useTimezone(config);
|
||||
const { getLocaleDocUrl } = useDocDomain();
|
||||
@ -131,7 +134,7 @@ export default function StorageMetrics({
|
||||
used={totalStorage.camera}
|
||||
total={totalStorage.total}
|
||||
/>
|
||||
{storageBreakdown && (
|
||||
{storageBreakdown && storageBreakdown.total > 0 && (
|
||||
<div className="mt-3 space-y-1 text-xs">
|
||||
<div className="flex justify-between text-primary-variant">
|
||||
<span>
|
||||
@ -140,9 +143,7 @@ export default function StorageMetrics({
|
||||
"Continuous (overwritable)",
|
||||
)}
|
||||
</span>
|
||||
<span>
|
||||
{(storageBreakdown.overwritable / 1024).toFixed(1)} GB
|
||||
</span>
|
||||
<span>{getUnitSize(storageBreakdown.overwritable)}</span>
|
||||
</div>
|
||||
<div className="flex justify-between text-primary-variant">
|
||||
<span>
|
||||
@ -151,9 +152,7 @@ export default function StorageMetrics({
|
||||
"Events (aging out)",
|
||||
)}
|
||||
</span>
|
||||
<span>
|
||||
{(storageBreakdown.event_retention / 1024).toFixed(1)} GB
|
||||
</span>
|
||||
<span>{getUnitSize(storageBreakdown.event_retention)}</span>
|
||||
</div>
|
||||
<div className="flex justify-between text-primary-variant">
|
||||
<span>
|
||||
@ -162,9 +161,7 @@ export default function StorageMetrics({
|
||||
"Protected (indefinite)",
|
||||
)}
|
||||
</span>
|
||||
<span>
|
||||
{(storageBreakdown.protected / 1024).toFixed(1)} GB
|
||||
</span>
|
||||
<span>{getUnitSize(storageBreakdown.protected)}</span>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user