Repository navigation
Add --apply-retention to the audit instance - #6078
ramonsmits wants to merge 1 commit into
Conversation
A change of ServiceControl.Audit/AuditRetentionPeriod only reaches new messages: ingestion sets the expiry of each document when it is stored. The new command rewrites the expiry of the stored messages and saga snapshots to their processed time plus the configured retention period. - IAuditRetentionApplier in the persistence abstraction, registered only by a persister that stores an expiry per document - RavenDB: patch by query over ProcessedMessages and SagaSnapshots, progress reported per batch - Without a registered implementation the command reports that the persister applies the retention period when it removes expired messages - Help text, host arguments test, RavenDB persistence test
47c6b52 to
f33227e
Compare
|
Does changing retention on indexed documents causes the indexing engine to reindex them? |
Document metadata updates, as far as I recall, are regular document indexes and cause incremental index changes |
| tokenSource.Cancel(); | ||
| }; | ||
|
|
||
| using var host = hostBuilder.Build(); |
There was a problem hiding this comment.
Doesn't the host support async disposable?
| /// <c>ServiceControl.Audit/AuditRetentionPeriod</c>: ingestion sets the expiry of each message when it is | ||
| /// stored, so the stored messages keep the old retention period until this command rewrites their expiry. | ||
| /// </summary> | ||
| class ApplyRetentionCommand : AbstractCommand |
There was a problem hiding this comment.
So the idea is that this is an optional enforcement because we want to keep the existing expiry mechanism (since that can be seen as a feature), but when someone would like to enforce for old documents the new retention, they can do so?
| The new expiry of each message is its processed time plus the retention period. Messages that are already past the new | ||
| expiry are removed by the next expiration run. |
There was a problem hiding this comment.
| The new expiry of each message is its processed time plus the retention period. Messages that are already past the new | |
| expiry are removed by the next expiration run. | |
| The new expiry of each message is its processed time plus the retention period. Messages that are already past the new expiry are removed by the next expiration run. |
| Stop the instance before running this command. A persister that applies the retention period when it removes expired | ||
| messages needs no update, and the command reports that. |
There was a problem hiding this comment.
| Stop the instance before running this command. A persister that applies the retention period when it removes expired | |
| messages needs no update, and the command reports that. | |
| Stop the instance before running this command. A persister that applies the retention period when it removes expired messages needs no update, and the command reports that. |
| { | ||
| { | ||
| "apply-retention", | ||
| "Apply the configured audit retention period to the stored messages", |
There was a problem hiding this comment.
Should this mention saga audits too?
| var documentStore = await documentStoreProvider.GetDocumentStore(cancellationToken); | ||
| long updated = 0; | ||
|
|
||
| foreach (var collection in Collections) |
There was a problem hiding this comment.
My understanding is the wait only waits for the command to be accepted and then returns is that correct? If true, then when all makes no sense, correct?
| from {{collection}} | ||
| update | ||
| { | ||
| this['@metadata']['@expires'] = new Date(new Date(this.ProcessedAt).getTime() + $retentionMilliseconds).toISOString(); |
There was a problem hiding this comment.
I think we should introduce a cutoff date here rather than patching every document in both collections.
Newly ingested documents already have the correct expiration metadata based on the configured retention period. Rewriting those documents unnecessarily creates write and indexing overhead, and potentially changes their expiration because the patch uses ProcessedAt rather than ingestion time.
Could we limit the patch to documents stored before the retention configuration changed? I wonder whether @last-modified would be a better cutoff criterion than ProcessedAt, since audit messages can be ingested after their actual processing time.
A change of
ServiceControl.Audit/AuditRetentionPeriodonly reaches new messages. Ingestion sets the expiry of each document when it is stored. This PR adds a command that applies the configured retention period to the stored messages.IAuditRetentionApplierin the persistence abstraction. Only a persister that stores an expiry per document registers an implementation.ProcessedMessagesandSagaSnapshotssets the expiry of each document to its processed time plus the retention period. Progress is logged per batch.Tests: argument parsing for the command, and a RavenDB persistence test that ingests under one retention period, applies another, and checks the expiry of a message and a saga snapshot.
Open points for review:
ProcessedAt, the stored time closest to it.Docs: a matching change to the
AuditRetentionPeriodsection of the audit configuration page is prepared and follows when the name is final.