-
-
Notifications
You must be signed in to change notification settings - Fork 506
Reduce noisy change events #2438
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
5b319fb
f8449f0
15e27df
e3e1f6d
8895405
2e96f54
d9508d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,7 @@ private async Task SavePendingOrganizationUsageAsync(DateTime utcNow) | |
| var bucketDiscarded = await _cache.GetAsync<int>(GetBucketDiscardedCacheKey(bucketUtc, organizationId)); | ||
| var bucketTooBig = await _cache.GetAsync<int>(GetBucketTooBigCacheKey(bucketUtc, organizationId)); | ||
| var bucketDeleted = await _cache.GetAsync<int>(GetBucketDeletedCacheKey(bucketUtc, organizationId)); | ||
| var hourlyThrottleTransition = await _cache.GetAsync<bool>(GetHourlyThrottleTransitionKey(bucketUtc, organizationId)); | ||
|
|
||
| bool hasIngestion = (bucketTotal?.Value ?? 0) > 0 || (bucketBlocked?.Value ?? 0) > 0 || (bucketDiscarded?.Value ?? 0) > 0 || (bucketTooBig?.Value ?? 0) > 0; | ||
| if (hasIngestion) | ||
|
|
@@ -113,11 +114,13 @@ await _cache.RemoveAllAsync(new[] { | |
| GetBucketDiscardedCacheKey(bucketUtc, organizationId), | ||
| GetBucketTooBigCacheKey(bucketUtc, organizationId), | ||
| GetBucketDeletedCacheKey(bucketUtc, organizationId), | ||
| GetThrottledKey(bucketUtc, organizationId) | ||
| GetThrottledKey(bucketUtc, organizationId), | ||
| GetHourlyThrottleTransitionKey(bucketUtc, organizationId) | ||
| }); | ||
|
|
||
| await _cache.SetAsync(GetTotalCacheKey(utcNow, organizationId), usage.Total, TimeSpan.FromHours(8)); | ||
| await _organizationRepository.SaveAsync(organization); | ||
| // Routine usage updates stay silent, but clients need one refresh when an hourly throttle clears. | ||
| await _organizationRepository.SaveAsync(organization, o => o.Notifications(hourlyThrottleTransition?.Value ?? false)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an organization crosses the monthly limit from pending usage, this save no longer emits Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 9bd0f02: the legacy organization cache is now cleared on PlanOverage, so its notification refresh reads current overage state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an organization is already over its monthly limit and the calendar rolls into a new month, the first usage save for the new month is a routine usage-only save with Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in bf75993: the first usage save of a new month detects an over-limit previous-month record and emits one OrganizationChanged. Later buckets remain silent. The regression test failed before the fix with Expected 1, Actual 0; the full UsageServiceTests suite now passes 21 of 21. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -204,7 +207,8 @@ await _cache.RemoveAllAsync(new[] { | |
|
|
||
| await _cache.SetAsync(GetTotalCacheKey(utcNow, project.OrganizationId, projectId), usage.Total, TimeSpan.FromHours(8)); | ||
|
|
||
| await _projectRepository.SaveAsync(project); | ||
| // Project configuration changes have their own save path and should remain the source of ProjectChanged messages. | ||
| await _projectRepository.SaveAsync(project, o => o.Notifications(false)); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -257,8 +261,9 @@ public async Task HandleOrganizationChangeAsync(Organization modified, Organizat | |
|
|
||
| if (bucketTotal.Value >= bucketLimit) | ||
| { | ||
| await _messagePublisher.PublishAsync(new PlanOverage { OrganizationId = modified.Id, IsHourly = true }); | ||
| await _cache.SetAsync(GetThrottledKey(utcNow, modified.Id), true, TimeSpan.FromMinutes(5)); | ||
| await _cache.SetAsync(GetHourlyThrottleTransitionKey(utcNow, modified.Id), true, TimeSpan.FromHours(8)); | ||
| await _messagePublisher.PublishAsync(new PlanOverage { OrganizationId = modified.Id, IsHourly = true }); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -449,8 +454,9 @@ public async Task IncrementTotalAsync(string organizationId, string projectId, i | |
| if (bucketTotal >= bucketLimit && bucketTotal - bucketLimit < eventCount) | ||
| { | ||
| // org will be throttled during the current bucket of time | ||
| await _messagePublisher.PublishAsync(new PlanOverage { OrganizationId = organizationId, IsHourly = true }); | ||
| await _cache.SetAsync(GetThrottledKey(utcNow, organizationId), true, TimeSpan.FromMinutes(5)); | ||
| await _cache.SetAsync(GetHourlyThrottleTransitionKey(utcNow, organizationId), true, TimeSpan.FromHours(8)); | ||
| await _messagePublisher.PublishAsync(new PlanOverage { OrganizationId = organizationId, IsHourly = true }); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -614,6 +620,12 @@ private string GetThrottledKey(DateTime utcTime, string organizationId) | |
| return $"usage:{bucket}:{organizationId}:throttled"; | ||
| } | ||
|
|
||
| private string GetHourlyThrottleTransitionKey(DateTime utcTime, string organizationId) | ||
| { | ||
| int bucket = GetCurrentBucket(utcTime); | ||
| return $"usage:{bucket}:{organizationId}:throttled-transition"; | ||
| } | ||
|
|
||
| private string GetProjectSetKey(DateTime utcTime) | ||
| { | ||
| int bucket = GetCurrentBucket(utcTime); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -929,6 +929,7 @@ private async Task AfterResultMapAsync<TDestination>(ICollection<TDestination> m | |
| currentUsage.Discarded = realTimeUsage.CurrentUsage.Discarded; | ||
| currentUsage.TooBig = realTimeUsage.CurrentUsage.TooBig; | ||
| currentUsage.Deleted = realTimeUsage.CurrentUsage.Deleted; | ||
| viewOrganization.IsOverMonthlyLimit = currentUsage.Limit >= 0 && currentUsage.Total >= currentUsage.Limit; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an organization's temporary event bonus expires without another usage bucket being saved, AGENTS.md reference: AGENTS.md:L67-L67 Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 2aa10d3: real-time usage totals are now compared against the organization view model live effective monthly limit, so an expired bonus cannot keep the overage state false. The focused regression failed before the fix with Expected True, Actual False; OrganizationEndpointTests now pass 103 of 103 and the build is clean. |
||
|
|
||
| var currentHourUsage = viewOrganization.GetCurrentHourlyUsage(timeProvider); | ||
| currentHourUsage.Total = realTimeUsage.CurrentHourUsage.Total; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { QueryClient } from '@tanstack/svelte-query'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { invalidatePlanOverageQueries, queryKeys } from './api.svelte'; | ||
|
|
||
| describe('invalidatePlanOverageQueries', () => { | ||
| it('invalidates only the affected organization state', async () => { | ||
| // Arrange | ||
| const queryClient = new QueryClient(); | ||
| const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries').mockImplementation(async () => {}); | ||
|
|
||
| // Act | ||
| await invalidatePlanOverageQueries(queryClient, { | ||
| is_hourly: false, | ||
| organization_id: 'organization-id' | ||
| }); | ||
|
|
||
| // Assert | ||
| expect(invalidateSpy).toHaveBeenCalledTimes(3); | ||
| expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.id('organization-id', undefined) }); | ||
| expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.id('organization-id', 'stats') }); | ||
| expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.list(undefined) }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ | |
| import { ChangeType, type WebSocketMessageValue } from '$features/websockets/models'; | ||
| import { useEventListener } from 'runed'; | ||
| import { SvelteSet } from 'svelte/reactivity'; | ||
| import { debounce } from 'throttle-debounce'; | ||
|
|
||
| import FreePlanNotification from './notifications/free-plan-notification.svelte'; | ||
| import HourlyOverageNotification from './notifications/hourly-overage-notification.svelte'; | ||
|
|
@@ -95,19 +94,21 @@ | |
| ); | ||
| const requiresPremiumUpgrade = $derived(requiresPremium && !organization?.has_premium_features && !needsProjectConfiguration); | ||
|
|
||
| const refetchConfigurationState = debounce(1500, async () => { | ||
| await Promise.all([organizationQuery.refetch(), projectsQuery.refetch()]); | ||
| }); | ||
|
|
||
| useEventListener(document, 'PersistentEventChanged', (event) => { | ||
| const message = (event as CustomEvent<WebSocketMessageValue<'PersistentEventChanged'>>).detail; | ||
|
|
||
| if (message.change_type === ChangeType.Removed || message.organization_id !== currentOrganizationId.current || !message.project_id) { | ||
| const projectNeedsConfiguration = projects.some((project) => project.id === message.project_id && project.is_configured === false); | ||
|
|
||
| if ( | ||
| message.change_type === ChangeType.Removed || | ||
| message.organization_id !== currentOrganizationId.current || | ||
| !message.project_id || | ||
| !projectNeedsConfiguration || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 910499a: valid PersistentEventChanged project IDs are now recorded immediately for the current organization, without requiring the projects query to have loaded first. Once project data arrives, the local configured-ID set suppresses the stale setup notification without refetching organization or project data. Added a focused early-event regression; Svelte validation is clean and all 404 frontend unit tests pass. |
||
| configuredProjectIds.has(message.project_id) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| configuredProjectIds.add(message.project_id); | ||
| void refetchConfigurationState(); | ||
| }); | ||
| </script> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { ChangeType } from '$features/websockets/models'; | ||
| import { render } from '@testing-library/svelte'; | ||
| import { tick } from 'svelte'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import OrganizationNotifications from './organization-notifications.svelte'; | ||
|
|
||
| const organizationRefetch = vi.hoisted(() => vi.fn()); | ||
| const projectsRefetch = vi.hoisted(() => vi.fn()); | ||
| const projects = vi.hoisted(() => [{ id: 'project-id', is_configured: false, organization_id: 'organization-id' }]); | ||
|
|
||
| vi.mock('$features/organizations/api.svelte', () => ({ | ||
| getOrganizationQuery: () => ({ data: undefined, refetch: organizationRefetch }), | ||
| getOrganizationsQuery: () => ({ data: { data: [] } }) | ||
| })); | ||
|
|
||
| vi.mock('$features/organizations/context.svelte', () => ({ | ||
| organization: { current: 'organization-id' } | ||
| })); | ||
|
|
||
| vi.mock('$features/projects/api.svelte', () => ({ | ||
| getOrganizationProjectsQuery: () => ({ data: { data: projects }, isSuccess: true, refetch: projectsRefetch }) | ||
| })); | ||
|
|
||
| vi.mock('$features/users/api.svelte', () => ({ | ||
| getMeQuery: () => ({ data: { organization_ids: ['organization-id'], roles: [] } }) | ||
| })); | ||
|
|
||
| describe('OrganizationNotifications', () => { | ||
| beforeEach(() => { | ||
| organizationRefetch.mockReset(); | ||
| projectsRefetch.mockReset(); | ||
| projects[0]!.is_configured = false; | ||
| }); | ||
|
|
||
| it('does not refetch organization or project state for persistent event changes', async () => { | ||
| render(OrganizationNotifications, { | ||
| isChatEnabled: false, | ||
| openChat: vi.fn() | ||
| }); | ||
|
|
||
| const message = { | ||
| change_type: ChangeType.Added, | ||
| organization_id: 'organization-id', | ||
| project_id: 'project-id' | ||
| }; | ||
|
|
||
| document.dispatchEvent(new CustomEvent('PersistentEventChanged', { detail: message })); | ||
| document.dispatchEvent(new CustomEvent('PersistentEventChanged', { detail: message })); | ||
| await tick(); | ||
|
|
||
| expect(organizationRefetch).not.toHaveBeenCalled(); | ||
| expect(projectsRefetch).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not refresh configuration state for an already configured project', async () => { | ||
| projects[0]!.is_configured = true; | ||
| render(OrganizationNotifications, { | ||
| isChatEnabled: false, | ||
| openChat: vi.fn() | ||
| }); | ||
|
|
||
| document.dispatchEvent( | ||
| new CustomEvent('PersistentEventChanged', { | ||
| detail: { | ||
| change_type: ChangeType.Added, | ||
| organization_id: 'organization-id', | ||
| project_id: 'project-id' | ||
| } | ||
| }) | ||
| ); | ||
| await tick(); | ||
|
|
||
| expect(organizationRefetch).not.toHaveBeenCalled(); | ||
| expect(projectsRefetch).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this repeatable migration repairs a current-month usage total from below the live limit to at or above it,
Notifications(false)removes theOrganizationChangedsignal that previously refreshed connected clients, while this path publishes no replacementPlanOverage. Users with warm Svelte or Angular organization caches therefore continue seeing the organization as under limit until an unrelated refresh; suppress routine migration notifications, but emit a targeted signal when the repaired current-month total crosses the effective limit.AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in ee08a86: the repeatable usage repair migration now detects only a current-month transition from below the live effective monthly limit to at or above it, saves the repaired usage silently, then publishes one monthly PlanOverage. Repeat runs remain silent because the persisted total is already repaired. The regression reproduced the issue before the fix (Expected 1, Actual 0), runs the migration twice to prove one-shot behavior, and both UpdateEventUsageMigrationTests plus all 27 UsageServiceTests pass; the build is clean.