From 19e2faae8e53ba5bbb58e0491bb96c97eda71b82 Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Tue, 26 Mar 2024 14:00:29 +0100 Subject: [PATCH] fix: correct range resolution for hourly (#4349) --- .../__tests__/pick-table-by-period.spec.ts | 49 ++++++++++++ .../operations/lib/pick-table-by-provider.ts | 80 +++++++++++++++++++ .../operations/providers/operations-reader.ts | 77 ++---------------- 3 files changed, 134 insertions(+), 72 deletions(-) create mode 100644 packages/services/api/src/modules/operations/lib/__tests__/pick-table-by-period.spec.ts create mode 100644 packages/services/api/src/modules/operations/lib/pick-table-by-provider.ts diff --git a/packages/services/api/src/modules/operations/lib/__tests__/pick-table-by-period.spec.ts b/packages/services/api/src/modules/operations/lib/__tests__/pick-table-by-period.spec.ts new file mode 100644 index 000000000..4fb7142e2 --- /dev/null +++ b/packages/services/api/src/modules/operations/lib/__tests__/pick-table-by-period.spec.ts @@ -0,0 +1,49 @@ +import { subDays } from 'date-fns'; +import { pickTableByPeriod } from '../pick-table-by-provider'; + +describe('pickTableByPeriod', () => { + test('3 day period -> hourly', () => { + const now = new Date(); + const table = pickTableByPeriod({ + now, + period: { + from: subDays(new Date(), 3), + to: now, + }, + }); + expect(table).toBe('hourly'); + }); + test('7 day period -> hourly', () => { + const now = new Date(); + const table = pickTableByPeriod({ + now, + period: { + from: subDays(new Date(), 7), + to: now, + }, + }); + expect(table).toBe('hourly'); + }); + test('14 day period -> hourly', () => { + const now = new Date(); + const table = pickTableByPeriod({ + now, + period: { + from: subDays(new Date(), 14), + to: now, + }, + }); + expect(table).toBe('hourly'); + }); + test('28 day period -> hourly', () => { + const now = new Date(); + const table = pickTableByPeriod({ + now, + period: { + from: subDays(new Date(), 28), + to: now, + }, + }); + expect(table).toBe('daily'); + }); +}); diff --git a/packages/services/api/src/modules/operations/lib/pick-table-by-provider.ts b/packages/services/api/src/modules/operations/lib/pick-table-by-provider.ts new file mode 100644 index 000000000..970beb9e2 --- /dev/null +++ b/packages/services/api/src/modules/operations/lib/pick-table-by-provider.ts @@ -0,0 +1,80 @@ +import { + addMinutes, + format, + startOfDay, + startOfHour, + startOfMinute, + subHours, + subMinutes, +} from 'date-fns'; +import type { DateRange } from '../../../shared/entities'; +import type { Logger } from '../../shared/providers/logger'; + +const msMinute = 60 * 1_000; +const msHour = msMinute * 60; +const msDay = msHour * 24; + +// How long rows are kept in the database, per table. +const tableTTLInHours = { + daily: 365 * 24, + hourly: 30 * 24, + minutely: 24, +}; + +const thresholdDataPointPerDay = 28; +const thresholdDataPointPerHour = 24; + +function formatDate(date: Date): string { + return format(addMinutes(date, date.getTimezoneOffset()), 'yyyy-MM-dd HH:mm:ss'); +} + +/** pick the correct materialized view table for request data based on the input period */ +export function pickTableByPeriod(args: { + now: Date; + period: DateRange; + logger?: Logger; +}): 'hourly' | 'daily' | 'minutely' { + // The oldest data point we can fetch from the database, per table. + // ! We subtract 2 minutes as we round the date to the nearest minute on UI + // and there's also a chance that request will be made at 59th second of the minute + // and by the time it this function is called the minute will change. + // That's why we use 2 minutes as a buffer. + const tableOldestDateTimePoint = { + daily: subMinutes(startOfDay(subHours(args.now, tableTTLInHours.daily)), 2), + hourly: subMinutes(startOfHour(subHours(args.now, tableTTLInHours.hourly)), 2), + minutely: subMinutes(startOfMinute(subHours(args.now, tableTTLInHours.minutely)), 2), + }; + + if ( + args.period.to.getTime() <= tableOldestDateTimePoint.daily.getTime() || + args.period.from.getTime() <= tableOldestDateTimePoint.daily.getTime() + ) { + args.logger?.error( + `Requested date range ${formatDate(args.period.from)} - ${formatDate(args.period.to)} is too old.`, + ); + throw new Error(`The requested date range is too old for the selected query type.`); + } + + const daysDifference = Math.floor( + (args.period.to.getTime() - args.period.from.getTime()) / msDay, + ); + + if ( + daysDifference >= thresholdDataPointPerDay || + args.period.to.getTime() <= tableOldestDateTimePoint.hourly.getTime() || + args.period.from.getTime() <= tableOldestDateTimePoint.hourly.getTime() + ) { + return 'daily'; + } + + const hoursDifference = (args.period.to.getTime() - args.period.from.getTime()) / msHour; + if ( + hoursDifference >= thresholdDataPointPerHour || + args.period.to.getTime() <= tableOldestDateTimePoint.minutely.getTime() || + args.period.from.getTime() <= tableOldestDateTimePoint.minutely.getTime() + ) { + return 'hourly'; + } + + return 'minutely'; +} diff --git a/packages/services/api/src/modules/operations/providers/operations-reader.ts b/packages/services/api/src/modules/operations/providers/operations-reader.ts index ed1364b20..b8fd36e00 100644 --- a/packages/services/api/src/modules/operations/providers/operations-reader.ts +++ b/packages/services/api/src/modules/operations/providers/operations-reader.ts @@ -1,19 +1,11 @@ -import { - addMinutes, - differenceInDays, - format, - startOfDay, - startOfHour, - startOfMinute, - subHours, - subMinutes, -} from 'date-fns'; +import { addMinutes, differenceInDays, format } from 'date-fns'; import { Injectable } from 'graphql-modules'; import * as z from 'zod'; import { batch } from '@theguild/buddy'; import type { DateRange } from '../../../shared/entities'; import { batchBy } from '../../../shared/helpers'; import { Logger } from '../../shared/providers/logger'; +import { pickTableByPeriod } from '../lib/pick-table-by-provider'; import { ClickHouse, RowOf, sql } from './clickhouse-client'; import { calculateTimeWindow } from './helpers'; import { SqlValue } from './sql'; @@ -62,20 +54,6 @@ function ensureNumber(value: number | string): number { return parseFloat(value); } -const msMinute = 60 * 1_000; -const msHour = msMinute * 60; -const msDay = msHour * 24; - -// How long rows are kept in the database, per table. -const tableTTLInHours = { - daily: 365 * 24, - hourly: 30 * 24, - minutely: 24, -}; - -const thresholdDataPointPerDay = 28; -const thresholdDataPointPerHour = 24; - @Injectable({ global: true, }) @@ -122,56 +100,11 @@ export class OperationsReader { const now = new Date(); - // The oldest data point we can fetch from the database, per table. - // ! We subtract 2 minutes as we round the date to the nearest minute on UI - // and there's also a chance that request will be made at 59th second of the minute - // and by the time it this function is called the minute will change. - // That's why we use 2 minutes as a buffer. - const tableOldestDateTimePoint = { - daily: subMinutes(startOfDay(subHours(now, tableTTLInHours.daily)), 2), - hourly: subMinutes(startOfHour(subHours(now, tableTTLInHours.hourly)), 2), - minutely: subMinutes(startOfMinute(subHours(now, tableTTLInHours.minutely)), 2), - }; - - if ( - period.to.getTime() <= tableOldestDateTimePoint.daily.getTime() || - period.from.getTime() <= tableOldestDateTimePoint.daily.getTime() - ) { - this.logger.error( - `Requested date range ${formatDate(period.from)} - ${formatDate(period.to)} is too old.`, - ); - throw new Error(`The requested date range is too old for the selected query type.`); - } - - const daysDifference = Math.floor((period.to.getTime() - period.from.getTime()) / msDay); - - if ( - daysDifference >= thresholdDataPointPerDay || - period.to.getTime() <= tableOldestDateTimePoint.hourly.getTime() || - period.from.getTime() <= tableOldestDateTimePoint.hourly.getTime() - ) { - return { - ...queryMap['daily'], - queryType: 'daily', - }; - } - - const hoursDifference = (period.to.getTime() - period.from.getTime()) / msHour; - - if ( - hoursDifference >= thresholdDataPointPerHour && - period.to.getTime() <= tableOldestDateTimePoint.minutely.getTime() && - period.from.getTime() <= tableOldestDateTimePoint.minutely.getTime() - ) { - return { - ...queryMap['hourly'], - queryType: 'hourly', - }; - } + const resolvedTable = pickTableByPeriod({ now, period, logger: this.logger }); return { - ...queryMap['minutely'], - queryType: 'minutely', + ...queryMap[resolvedTable], + queryType: resolvedTable, }; }