Fix per-message MongoDB query caused by TimeStampConfig null cache issue#25344
Fix per-message MongoDB query caused by TimeStampConfig null cache issue#25344danotorrey merged 5 commits intomasterfrom
Conversation
The grace period cache in ProcessBufferProcessor uses null to mean both "not yet loaded" and "loaded value was null." When the grace period is null (normalization disabled, the default), the cache can never store the result, so every message triggers a fresh MongoDB read. - Add gracePeriodLoaded boolean flag to distinguish not-loaded from null - Delete TimeStampConfig document when normalization is disabled instead of writing one with a null payload - Add remove action to ConfigurationsStore for cluster config deletion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes an issue where TimeStampConfig being null caused ProcessBufferProcessor to re-query MongoDB on every ingested message by making the grace-period cache able to represent “loaded null”.
Changes:
- Server: cache
TimeStampConfig.gracePeriod()correctly even when it isnull, and invalidate cache on cluster-config change events. - Web UI: when timestamp normalization is disabled, delete the
TimeStampConfigcluster config entry instead of writing anullpayload; add aremoveaction to support this. - Tests/changelog: add regression tests covering null caching and update UI tests + changelog entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
graylog2-server/src/main/java/org/graylog2/shared/buffers/processors/ProcessBufferProcessor.java |
Fixes grace-period caching so null can be cached without repeated cluster-config reads. |
graylog2-server/src/test/java/org/graylog2/shared/buffers/processors/MessageTimestampTest.java |
Adds tests verifying caching behavior for null/non-null and invalidation transitions. |
graylog2-web-interface/src/stores/configurations/ConfigurationsStore.ts |
Adds remove action to delete cluster config entries from the UI. |
graylog2-web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.tsx |
Switches disabled normalization behavior to delete the cluster config entry. |
graylog2-web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.test.tsx |
Updates tests to expect deletion instead of writing grace_period: undefined. |
changelog/unreleased/pr-25344.toml |
Documents the fix in the changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.tsx
Outdated
Show resolved
Hide resolved
...web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.tsx
Outdated
Show resolved
Hide resolved
@kodjo-anipah Thanks for the feedback. Good point. Since the backend already handles that case, we can just keep that existing default of null. Makes sense. I'll work that in. |
|
@kodjo-anipah I went ahead and reverted all frontend changes, since the backend changes should now handle the null |
There was a problem hiding this comment.
Pull request overview
Fixes an ingestion-time performance regression where TimeStampConfig being null caused ProcessBufferProcessor to query MongoDB (cluster_config) for every ingested message indefinitely, by making the grace-period cache able to distinguish “not loaded yet” from “loaded value is null”. The web UI is also adjusted to delete the TimeStampConfig document when future timestamp normalization is disabled.
Changes:
- Server: cache
nullgrace period values correctly inProcessBufferProcessorand invalidate the cache onClusterConfigChangedEvent. - Server: expand
MessageTimestampTestto verify caching behavior fornull, transitions, and repeated calls. - Web UI: add a cluster-config
removeaction and use it to deleteTimeStampConfigwhen normalization is disabled.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| graylog2-web-interface/src/stores/configurations/ConfigurationsStore.ts | Adds remove(configType) action + store handler to DELETE cluster-config entries. |
| graylog2-web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.tsx | Switches from updating TimeStampConfig with undefined to deleting it when disabled. |
| graylog2-web-interface/src/components/configurations/message-processors/ProcessingConfigModalForm.test.tsx | Updates tests to expect deletion (remove) when normalization is disabled. |
| graylog2-server/src/main/java/org/graylog2/shared/buffers/processors/ProcessBufferProcessor.java | Fixes grace-period caching so null is cached and doesn’t trigger per-message DB reads. |
| graylog2-server/src/test/java/org/graylog2/shared/buffers/processors/MessageTimestampTest.java | Adds tests proving null/non-null caching and invalidation behavior. |
| changelog/unreleased/pr-25344.toml | Adds changelog entry documenting the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The backend fix (caching null properly in ProcessBufferProcessor) is sufficient on its own. The frontend delete approach introduced unnecessary permission concerns per reviewer feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I had forgotten to push the commit that reverted the frontend changes. Done. Merging now, and backport is up: #25350 |
…l cache bug (7.0) (#25350) * Fix per-message MongoDB query caused by TimeStampConfig null cache bug The grace period cache in ProcessBufferProcessor uses null to mean both "not yet loaded" and "loaded value was null." When the grace period is null (normalization disabled, the default), the cache can never store the result, so every message triggers a fresh MongoDB read. - Add gracePeriodLoaded boolean flag to distinguish not-loaded from null - Delete TimeStampConfig document when normalization is disabled instead of writing one with a null payload - Add remove action to ConfigurationsStore for cluster config deletion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add changelog entry for PR #25344 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update changelog wording Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove frontend changes from cherry-pick (backend-only backport) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Saving the Message Processors configuration causes MongoDB (
cluster_configcollection) to be queried on every ingested message, indefinitely. The grace period cache inProcessBufferProcessorusesnullto mean both "not yet loaded" and "loaded value was null." When the grace period is null (normalization disabled, the default), the cache can never store the result, so every message triggers a fresh MongoDB read.It seems this was introduced when PR #24686 changed the
TimeStampConfigdefault from a non-null sentinel (giant duration) value tonull. Once changed to null, the cache could not distinguish it from "not loaded yet."This fix will need to be backported to 7.0.
Closes https://github.com/Graylog2/graylog-plugin-enterprise/issues/13535
Changes
ProcessBufferProcessor.java: Added agracePeriodLoadedboolean flag so the cache can distinguish "not loaded" from "loaded as null" (backwards-compatible handling)MessageTimestampTest.java: Added 8 new tests covering null caching, transitions between enabled/disabled, and high-volume scenariosFixes
🤖 Generated with Claude Code