Random sampling TTL prototype #136430
Random sampling TTL prototype #136430
Conversation
…java Co-authored-by: Copilot <[email protected]>
…mpling/TransportPutSampleConfigurationAction.java Co-authored-by: Copilot <[email protected]>
…nfigurationExecutorTests.java Co-authored-by: Copilot <[email protected]>
# Conflicts: # server/src/main/java/org/elasticsearch/ingest/SamplingService.java # server/src/test/java/org/elasticsearch/ingest/UpdateSamplingConfigurationExecutorTests.java
masseyke
added
the
:Data Management/Ingest Node
…mpling/SamplingConfiguration.java Co-authored-by: Copilot <[email protected]>
…ticsearch into random-sampling-everything
There was a problem hiding this comment.
Pull Request Overview
This PR implements a random sampling TTL (Time-To-Live) prototype that automatically removes expired sampling configurations. The implementation adds a scheduled job that periodically checks for expired configurations and removes them from the cluster state.
Key changes:
- Added TTL checking functionality with configurable polling interval
- Modified SamplingService constructor to accept Settings instead of time supplier
- Integrated SamplingService into Node lifecycle management
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SamplingService.java | Core implementation of TTL functionality with scheduler and lifecycle management |
| NodeConstruction.java | Updated constructor call to pass settings instead of time supplier |
| Node.java | Added SamplingService to node start/stop/close lifecycle |
| ClusterSettings.java | Registered TTL_POLL_INTERVAL_SETTING as cluster setting |
| SamplingServiceTests.java | Updated test to use new constructor signature |
| SamplingServiceIT.java | Added integration test for TTL functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| private final SetOnce<SchedulerEngine> scheduler = new SetOnce<>(); | ||
| private SchedulerEngine.Job scheduledJob; | ||
| private volatile TimeValue pollInterval = TimeValue.timeValueSeconds(1); |
There was a problem hiding this comment.
The default pollInterval value (1 second) conflicts with the TTL_POLL_INTERVAL_SETTING default (30 minutes). This could cause confusion and should be initialized from the setting's default value.
| private volatile TimeValue pollInterval = TimeValue.timeValueSeconds(1); | |
| private volatile TimeValue pollInterval = TTL_POLL_INTERVAL_SETTING.getDefault(Settings.EMPTY); |
Copilot uses AI. Check for mistakes.
| "@timestamp", | ||
| randomTimeValue().toHumanReadableString(2), |
There was a problem hiding this comment.
Using randomTimeValue().toHumanReadableString(2) for @timestamp field is problematic. The toHumanReadableString() method returns human-readable format like '1.5s' which is not a valid timestamp format. This should be an ISO timestamp string or epoch milliseconds.
Copilot uses AI. Check for mistakes.
…ticsearch into random-sampling-everything
3 participants
DRAFT FOR SHARING CODE ONLY
Prototype of random sampling TTL