-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Dirt/pm 33527/multi delete task types #7910
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
Draft
prograhamming
wants to merge
1
commit into
dirt/pm-33527/server-db-combined
Choose a base branch
from
dirt/pm-33527/multi-delete-task-types
base: dirt/pm-33527/server-db-combined
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| #nullable enable | ||
|
|
||
| using Azure; | ||
| using Bit.Core; | ||
| using Bit.Core.Dirt.Enums; | ||
| using Bit.Core.Dirt.Repositories; | ||
| using Bit.Core.Dirt.Services; | ||
| using Bit.Core.Jobs; | ||
| using Bit.Core.Services; | ||
| using Quartz; | ||
|
|
||
| namespace Bit.Admin.Jobs; | ||
|
|
||
| /// <summary> | ||
| /// Drains the <c>OrganizationDeleteTask</c> queue: claims the next pending task and dispatches it to | ||
| /// the <see cref="IOrganizationDeleteTaskHandler"/> registered for its type, deleting in bounded | ||
| /// batches within a run budget so a large cleanup resumes across runs. All lease, progress, and | ||
| /// error bookkeeping lives here; handlers only implement the per-type batch delete. | ||
| /// </summary> | ||
| public class OrganizationDeleteTasksJob : BaseJob | ||
| { | ||
| private static readonly TimeSpan _runBudget = TimeSpan.FromMinutes(4); | ||
|
|
||
| // A missing handler is expected transiently while a rolling deploy is in flight, but a | ||
| // genuinely orphaned type (enqueued with no handler that will ever be deployed) stays | ||
| // unhandled long after any deploy completes. Below this age we log the miss at Warning; | ||
| // beyond it we escalate to Error so a stuck type can be alerted on rather than lost in | ||
| // steady Warning volume. | ||
| private static readonly TimeSpan _orphanedTaskEscalationThreshold = TimeSpan.FromHours(1); | ||
|
|
||
| private readonly IOrganizationDeleteTaskRepository _cleanupRepository; | ||
| private readonly IReadOnlyDictionary<OrganizationDeleteTaskType, IOrganizationDeleteTaskHandler> _handlers; | ||
| private readonly IFeatureService _featureService; | ||
|
|
||
| public OrganizationDeleteTasksJob( | ||
| IOrganizationDeleteTaskRepository cleanupRepository, | ||
| IEnumerable<IOrganizationDeleteTaskHandler> handlers, | ||
| IFeatureService featureService, | ||
| ILogger<OrganizationDeleteTasksJob> logger) | ||
| : base(logger) | ||
| { | ||
| _cleanupRepository = cleanupRepository; | ||
| // Throws at construction if two handlers claim the same type, failing fast on misconfiguration. | ||
| _handlers = handlers.ToDictionary(handler => handler.TaskType); | ||
| _featureService = featureService; | ||
| } | ||
|
|
||
| protected override async Task ExecuteJobAsync(IJobExecutionContext context) | ||
| { | ||
| if (!_featureService.IsEnabled(FeatureFlagKeys.OrganizationEventCleanup)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var pending = await _cleanupRepository.ClaimNextPendingAsync(); | ||
| if (pending is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!_handlers.TryGetValue(pending.TaskType, out var handler)) | ||
| { | ||
| // No handler is registered for this type. This is expected transiently when the server | ||
| // enqueuing tasks is ahead of this worker during a rolling deploy. We deliberately do NOT | ||
| // record a failure: doing so would burn the retry budget and could permanently abandon a | ||
| // task that only needs a newer worker. The claim lease expires and the task is reclaimed | ||
| // on a later run. Escalate to Error once it has been unhandled long enough that deploy | ||
| // skew is no longer a plausible explanation. | ||
| var unhandledFor = DateTime.UtcNow - pending.CreationDate; | ||
| var logLevel = unhandledFor >= _orphanedTaskEscalationThreshold ? LogLevel.Error : LogLevel.Warning; | ||
| _logger.Log(logLevel, Constants.BypassFiltersEventId, | ||
| "No handler registered for organization delete task type {TaskType} (task {TaskId}); leaving for retry. Unhandled for {UnhandledMinutes:N0} minutes.", | ||
| pending.TaskType, pending.Id, unhandledFor.TotalMinutes); | ||
| return; | ||
| } | ||
|
|
||
| _logger.LogInformation(Constants.BypassFiltersEventId, | ||
| "Starting {TaskType} cleanup for organization {OrganizationId} (task {TaskId})", | ||
| pending.TaskType, pending.OrganizationId, pending.Id); | ||
|
|
||
| var deadline = DateTime.UtcNow.Add(_runBudget); | ||
| var deleted = 0; | ||
| var totalDeleted = 0L; | ||
|
|
||
| try | ||
| { | ||
| while (DateTime.UtcNow < deadline && !context.CancellationToken.IsCancellationRequested) | ||
| { | ||
| deleted = await handler.DeleteBatchAsync(pending, context.CancellationToken); | ||
| if (deleted == 0) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| await _cleanupRepository.UpdateProgressAsync(pending.Id, deleted); | ||
| totalDeleted += deleted; | ||
| } | ||
|
|
||
| if (deleted == 0) | ||
| { | ||
| await _cleanupRepository.UpdateCompletedAsync(pending.Id); | ||
| _logger.LogInformation(Constants.BypassFiltersEventId, | ||
| "Completed {TaskType} cleanup for organization {OrganizationId}; deleted {Deleted} items this run", | ||
| pending.TaskType, pending.OrganizationId, totalDeleted); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogInformation(Constants.BypassFiltersEventId, | ||
| "Paused {TaskType} cleanup for organization {OrganizationId}; deleted {Deleted} items this run, will resume", | ||
| pending.TaskType, pending.OrganizationId, totalDeleted); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| // Store a sanitized error, never ex.Message: Azure SDK messages can embed | ||
| // row-key identifiers (e.g. UserId=..., CipherId=...) that must not be persisted. | ||
| await _cleanupRepository.UpdateErrorAsync(pending.Id, BuildSanitizedError(ex)); | ||
| throw; | ||
| } | ||
| } | ||
|
|
||
| private static string BuildSanitizedError(Exception ex) => ex switch | ||
| { | ||
| RequestFailedException rfe => $"{nameof(RequestFailedException)} (Status: {rfe.Status}, ErrorCode: {rfe.ErrorCode})", | ||
| _ => ex.GetType().FullName ?? ex.GetType().Name, | ||
| }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| using Bit.Core.Dirt.Entities; | ||
| using Bit.Core.Dirt.Enums; | ||
|
|
||
| namespace Bit.Core.Dirt.Services; | ||
|
|
||
| /// <summary> | ||
| /// Handles the type-specific work for a single <see cref="OrganizationDeleteTask"/>. One | ||
| /// implementation exists per <see cref="OrganizationDeleteTaskType"/>; the cleanup job resolves the | ||
| /// matching handler by <see cref="TaskType"/> and drives it batch-by-batch. A team can add a new | ||
| /// cleanup type by adding an enum value and registering a handler β no job changes required. | ||
| /// </summary> | ||
| public interface IOrganizationDeleteTaskHandler | ||
| { | ||
| /// <summary> | ||
| /// The task type this handler is responsible for. Each type must have exactly one handler. | ||
| /// </summary> | ||
| OrganizationDeleteTaskType TaskType { get; } | ||
|
|
||
| /// <summary> | ||
| /// Deletes a single batch of data for the given task and returns the number of items deleted. | ||
| /// Return 0 when there is nothing left to delete, which signals completion. The job calls this | ||
| /// repeatedly within a bounded run budget, persisting progress between calls, so implementations | ||
| /// should delete a bounded amount per call and be safely resumable across runs. | ||
| /// </summary> | ||
| Task<int> DeleteBatchAsync(OrganizationDeleteTask task, CancellationToken cancellationToken); | ||
| } |
24 changes: 24 additions & 0 deletions
24
src/Core/Dirt/Services/Implementations/EventsCleanupOrganizationDeleteTaskHandler.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| using Bit.Core.Dirt.Entities; | ||
| using Bit.Core.Dirt.Enums; | ||
| using Bit.Core.Repositories; | ||
|
|
||
| namespace Bit.Core.Dirt.Services.Implementations; | ||
|
|
||
| /// <summary> | ||
| /// Purges an organization's event logs (e.g. from Table Storage) after the organization is deleted, | ||
| /// satisfying the <see cref="OrganizationDeleteTaskType.EventsCleanup"/> task type. | ||
| /// </summary> | ||
| public class EventsCleanupOrganizationDeleteTaskHandler : IOrganizationDeleteTaskHandler | ||
| { | ||
| private readonly IEventRepository _eventRepository; | ||
|
|
||
| public EventsCleanupOrganizationDeleteTaskHandler(IEventRepository eventRepository) | ||
| { | ||
| _eventRepository = eventRepository; | ||
| } | ||
|
|
||
| public OrganizationDeleteTaskType TaskType => OrganizationDeleteTaskType.EventsCleanup; | ||
|
|
||
| public Task<int> DeleteBatchAsync(OrganizationDeleteTask task, CancellationToken cancellationToken) | ||
| => _eventRepository.DeleteManyByOrganizationIdAsync(task.OrganizationId); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
β Can/do we want to keep taskTypes optional?