Record whether data streams for logs-- exist for logsdb enablement in 9.x #120708

martijnvg · 2025-01-23T13:32:25Z

Add LogsPatternUsageService that records whether there are data streams matching with logs-*-* pattern. This is recorded via the new logsdb.prior_logs_usage cluster setting. Upon upgrade to 9.x this can be used to determine whether logsdb should be enabled by default if cluster.logsdb.enabled hasn't been set.

The recommended upgrade path to 9.x is always to go to 8.latest. This component will run in clusters with version greater than 8.18.0 but not on 9.0 and newer.

Note that a follow change is required that removes / disables LogsPatternUsageService in main. Additionally updating the default of the LogsDBPlugin.CLUSTER_LOGSDB_ENABLED setting:

public static final Setting<Boolean> CLUSTER_LOGSDB_ENABLED = Setting.boolSetting(
        "cluster.logsdb.enabled",
        settings -> {
            if (LOGSDB_PRIOR_LOGS_USAGE.exists(settings)) {
                return Boolean.FALSE.toString();
            } else {
                return Boolean.TRUE.toString();
            }
        },
        Setting.Property.Dynamic,
        Setting.Property.NodeScope
    );

Add LogsPatternUsageService that records whether there are data streams matching with logs-*-* pattern. This is recorded via logsdb.prior_logs_usage cluster setting. Upon upgrade to 9.x this can be used to determine whether logsdb should be enabled by default if cluster.logsdb.enabled hasn't been set.

elasticsearchmachine · 2025-01-23T15:17:57Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

kkrik-es · 2025-01-23T15:30:09Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+import java.util.function.Supplier;
+
+/**
+ * A component that check in the background whether there are data streams that match log-*-* pattern and if so records this as persistent


Nit: checks*

kkrik-es · 2025-01-23T15:30:30Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+
+/**
+ * A component that check in the background whether there are data streams that match log-*-* pattern and if so records this as persistent
+ * setting in cluster state. If logs-*-* data stream usage has been found then this component will no longer in the background.


Nit: no longer run

kkrik-es · 2025-01-27T07:56:53Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java

+
+        var clusterService = services.clusterService();
+        Supplier<Metadata> metadataSupplier = () -> clusterService.state().metadata();
+        var historicLogsUsageService = new LogsPatternUsageService(services.client(), settings, services.threadPool(), metadataSupplier);


Consider initializing in the constructor and calling an init function here, similar to LogsdbIndexModeSettingsProvider.

kkrik-es · 2025-01-27T07:58:05Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java

+        Supplier<Metadata> metadataSupplier = () -> clusterService.state().metadata();
+        var historicLogsUsageService = new LogsPatternUsageService(services.client(), settings, services.threadPool(), metadataSupplier);
+        clusterService.addLocalNodeMasterListener(historicLogsUsageService);
+        clusterService.addLifecycleListener(new LifecycleListener() {


Add a function in LogsPatternUsageService to construct and return a listener?

kkrik-es · 2025-01-27T07:59:25Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+ */
+final class LogsPatternUsageService implements LocalNodeMasterListener {
+
+    private static final String LOGS_PATTERN = "logs-*-*";


Deduplicate with LogsdbIndexModeSettingsProvider.LOGS_PATTERN ?

kkrik-es · 2025-01-27T07:59:54Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+    private static final Logger LOGGER = LogManager.getLogger(LogsPatternUsageService.class);
+    static final Setting<TimeValue> USAGE_CHECK_PERIOD = Setting.timeSetting(
+        "logsdb.usage_check.period",
+        new TimeValue(24, TimeUnit.HOURS),


Consider adding a comment explaining how this value is picked.

kkrik-es · 2025-01-27T08:01:11Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+        new TimeValue(24, TimeUnit.HOURS),
+        Setting.Property.NodeScope
+    );
+    static final Setting<Boolean> LOGSDB_PRIOR_LOGS_USAGE = Setting.boolSetting(


Do we want to restrict this as internal? Or, maybe allow updates in case we get it wrong?

At any rate, let's document expectations.

Only index settings can be made internal/private. Not node/ cluster settings.

I think we shouldn't document this setting. If someone sets this setting, then they might just as well configure cluster.logsdb.enabled? The advantage of this being public, is that for debugging purposes we can see that a 8.18 cluster is the right state to upgrade.

I will add a comment on top of this setting constant.

kkrik-es · 2025-01-27T08:03:45Z

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageService.java

+    }
+
+    void scheduleNext(TimeValue waitTime) {
+        if (isMaster && hasPriorLogsUsage == false) {


So, if the master changes, we may miss an update in 24h? Worth documenting, though this seems fine.

I updated this wait time logic a bit via this commit: e0a5c11

kkrik-es · 2025-01-27T08:07:44Z

...db/src/test/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageServiceIntegrationTests.java

+                .actionGet();
+            assertThat(response.persistentSettings().get("logsdb.prior_logs_usage"), equalTo("true"));
+        });
+    }


Add a test for a non-matching pattern?

Also, is it possible to test that it only applies to master and it no longer runs after setting to true?

kkrik-es

Looks good, just a few minor comments.

…elected. Changed `logsdb.usage_check.period` to `logsdb.usage_check.max_period` and starting to check 60s after node has been elected as master. This gets doubled after each check that doesn't see any log-*-* usage upto `logsdb.usage_check.max_period` (defaults to 24h)

…n found

kkrik-es · 2025-01-27T11:58:24Z

...db/src/test/java/org/elasticsearch/xpack/logsdb/LogsPatternUsageServiceIntegrationTests.java

+        assertThat(indexResponse.getResult(), equalTo(DocWriteResponse.Result.CREATED));
+
+        ensureGreen("log-myapp-prod");
+        assertBusy(() -> {


In fairness, add a short sleep() to give time for the usage service to run?

Good point. I did this which is better than a thread.sleep() :572d670010d88ff1f1639197fe18fec7544ddf88

martijnvg · 2025-01-27T15:27:00Z

@elasticmachine update branch

martijnvg · 2025-01-28T06:55:58Z

@elasticmachine update branch

elasticsearchmachine · 2025-01-28T08:05:41Z

💔 Backport failed

Status	Branch	Result
❌	8.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 120708

…in 9.x (elastic#120708) Add LogsPatternUsageService that records whether there are data streams matching with logs-*-* pattern. This is recorded via the new logsdb.prior_logs_usage cluster setting. Upon upgrade to 9.x this can be used to determine whether logsdb should be enabled by default if cluster.logsdb.enabled hasn't been set. The recommended upgrade path to 9.x is always to go to 8.latest. This component will run in clusters with version greater than 8.18.0 but not on 9.0 and newer.

…ement in 9.x (#121025) Backporting #120708 to 8.x branch. Add LogsPatternUsageService that records whether there are data streams matching with logs-*-* pattern. This is recorded via the new logsdb.prior_logs_usage cluster setting. Upon upgrade to 9.x this can be used to determine whether logsdb should be enabled by default if cluster.logsdb.enabled hasn't been set. The recommended upgrade path to 9.x is always to go to 8.latest. This component will run in clusters with version greater than 8.18.0 but not on 9.0 and newer.

Enable logsdb by default if logsdb.prior_logs_usage has not been set to true. Meaning that if no data streams were created matching with the logs-- pattern in 8.x, then logsdb will be enabled by default for data streams matching with logs-*-* pattern. Also removes LogsPatternUsageService as with version 9.0 and beyond, this component is no longer necessary. Followup from #120708 Closes #106489

elasticsearchmachine added the v9.0.0 label Jan 23, 2025

elasticsearchmachine and others added 3 commits January 23, 2025 13:39

[CI] Auto commit changes from spotless

8888377

iter

f3e1d3c

more tests

8ceae2b

martijnvg added :StorageEngine/Logs You know, for Logs v8.18.0 >non-issue labels Jan 23, 2025

martijnvg changed the title ~~LogsPatternUsageService~~ Record whether data streams for logs-*-* exist for logsdb enablement in 9.x Jan 23, 2025

martijnvg marked this pull request as ready for review January 23, 2025 15:17

elasticsearchmachine added the Team:StorageEngine label Jan 23, 2025

kkrik-es reviewed Jan 23, 2025

View reviewed changes

martijnvg and others added 3 commits January 24, 2025 10:01

Merge remote-tracking branch 'es/main' into LogsPatternUsageService

b9c8d96

iter

3b4f3ac

[CI] Auto commit changes from spotless

85f1c6b

martijnvg force-pushed the LogsPatternUsageService branch from 0486afe to 85f1c6b Compare January 24, 2025 09:08

kkrik-es reviewed Jan 27, 2025

View reviewed changes

Merge remote-tracking branch 'es/main' into LogsPatternUsageService

a6c45d6

kkrik-es reviewed Jan 27, 2025

View reviewed changes

martijnvg added 3 commits January 27, 2025 11:30

applied feedback

835928c

verify that nextWaitTime gets double after no log-*-* usage has bee…

1aca5b8

…n found

martijnvg requested a review from kkrik-es January 27, 2025 11:10

kkrik-es reviewed Jan 27, 2025

View reviewed changes

kkrik-es approved these changes Jan 27, 2025

View reviewed changes

give LogsPatternUsageService opportunity to run

572d670

Merge branch 'main' into LogsPatternUsageService

d29c766

Merge branch 'main' into LogsPatternUsageService

52e47c0

martijnvg added the auto-backport Automatically create backport pull requests when merged label Jan 28, 2025

martijnvg enabled auto-merge (squash) January 28, 2025 06:56

martijnvg merged commit 1da8285 into elastic:main Jan 28, 2025
16 checks passed

elasticsearchmachine added the backport pending label Jan 28, 2025

martijnvg mentioned this pull request Jan 28, 2025

[8.x] Record whether data streams for logs-*-* exist for logsdb enablement in 9.x #121025

Merged

martijnvg mentioned this pull request Jan 28, 2025

Conditionally enable logsdb by default #121049

Merged

Record whether data streams for logs-*-* exist for logsdb enablement in 9.x #120708

Record whether data streams for logs-*-* exist for logsdb enablement in 9.x #120708

Uh oh!

Conversation

martijnvg commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkrik-es left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnvg commented Jan 27, 2025

Uh oh!

martijnvg commented Jan 28, 2025

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 28, 2025

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Record whether data streams for logs-- exist for logsdb enablement in 9.x #120708

Record whether data streams for logs-- exist for logsdb enablement in 9.x #120708

martijnvg commented Jan 23, 2025 •

edited

Loading