Disable deprecation log indexing until templates are loaded by pgomulka · Pull Request #80406 · elastic/elasticsearch

pgomulka · 2021-11-05T11:10:31Z

Deprecation log indexing relies on index templates to be present in
cluster state in order to create a data stream.
If a very early deprecation is emitted after node is started up and
templates are not loaded yet this can result in regular index with
default settings and mappings to be created.
To prevent this a bulk processor used for indexing deprecation logs
delays flushing until templates are loaded.

closes #80265

Deprecation log indexing relies on index templates to be present in cluster state in order to create a data stream. If a very early deprecation is emitted after node is started up and templates are not loaded yet this can result in regular index with default settings and mappings to be created. To prevent this a bulk processor used for indexing deprecation logs delays flushing. closes elastic#80265

…or_flush_delay

elasticmachine · 2021-11-05T14:56:38Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

elasticmachine · 2021-11-05T14:56:38Z

Pinging @elastic/es-data-management (Team:Data Management)

pgomulka · 2021-11-05T14:58:22Z

x-pack/plugin/deprecation/qa/common/build.gradle

+apply plugin: 'elasticsearch.build'
+tasks.named("test").configure { enabled = false }
+
+dependencies {


@mark-vieira would you be able to review this PR from gradle point of view?
I wasn't too sure if I should use api or implementation for dependency? do we have a readme for this?
also do I have to copy license files here as well?

Licenses are only required once, you don't need them every time you use a dependency. We favor implementation as those dependencies are not transitively inherited by the compile classpath of consuming projects. Since this is a QA project it's mostly irrelevant (there are no consumers) but I'd still leave it all implementation here.

if I remove the licence folder I get

Execution failed for task ':x-pack:plugin:deprecation:qa:common:dependencyLicenses'. > Licences dir /Users/przemek/workspace/pgomulka/elasticsearch/x-pack/plugin/deprecation/qa/common/licenses does not exist, but there are dependencies: jackson-annotations-2.10.4.jar jackson-databind-2.10.4.jar

I wonder if I miss any plugin or import?

You shouldn't be adding the elasticsearch.build plugin to QA projects. That's specifically for production code. What is this "common" project for anyhow? I don't see any actual code belonging to it?

What is this "common" project for anyhow?

it has only one class DeprecationTestUtils - I felt that copying that code would be a smell.
The problem boils down to the fact that EarlyDeprecationIndexingIT has to be first running against a fresh test cluster - and hence it was easiest to move it to its own project

I removed the elasticsearch.build but I had to add elasticsearch.java otherwise gradle would complain about implementation

Could not find method implementation() for arguments

Yep, elasticsearch.java is the correct choice here.

pgomulka · 2021-11-05T15:00:09Z

x-pack/plugin/deprecation/qa/early-deprecation-rest/build.gradle

@@ -0,0 +1,41 @@
+import org.elasticsearch.gradle.util.GradleUtils


this is basically a copy of x-pack/plugin/deprecation/qa/rest/build.gradle
can I move shared bits to common module?

I had to create that module, so that I could make sure that EarlyDeprecationIndexingIT is the test that runs first thing after test cluster starts

…or_flush_delay

martijnvg

Left 2 questions. Other than that this looks good to me.

.../src/main/java/org/elasticsearch/xpack/deprecation/logging/DeprecationIndexingComponent.java

martijnvg · 2021-11-08T12:43:35Z

server/src/main/java/org/elasticsearch/action/bulk/BulkProcessor.java

        private String globalIndex;
        private String globalRouting;
        private String globalPipeline;
+        private Supplier<Boolean> flushCondition = () -> true;


martijnvg · 2021-11-08T12:47:41Z

...cation/qa/common/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationTestUtils.java

+
+package org.elasticsearch.xpack.deprecation;
+
+import com.fasterxml.jackson.databind.JsonNode;


Why is databind being used here? I believe in mast of the rest integration test we use XContentMapValues, could that be used here as well?

good idea, XContentMapValues will make it much cleaner

pgomulka · 2021-11-08T15:59:17Z

@elasticmachine run elasticsearch-ci/bwc

pgomulka · 2021-11-08T16:48:48Z

@elasticmachine update branch

martijnvg · 2021-11-09T10:40:07Z

.../src/main/java/org/elasticsearch/xpack/deprecation/logging/DeprecationIndexingComponent.java

+
+    @Override
+    public void clusterChanged(ClusterChangedEvent event) {
+        if (flushEnabled.get() == false) {


Maybe instead of wrapping multiple if statements, maybe do this:

if (flushEnabled.get()) { return; } if (event.metadataChanged() == false) { return; } // rest of the code

I think this is more readable.

…/elasticsearch into main/bulk_processor_flush_delay

…or_flush_delay

martijnvg

Thanks @pgomulka, LGTM 👍

martijnvg · 2021-11-09T10:57:23Z

...st/src/javaRestTest/java/org/elasticsearch/xpack/deprecation/EarlyDeprecationIndexingIT.java

+            Response response = getIndexSettings();
+            ObjectMapper mapper = new ObjectMapper();
+
+            final JsonNode jsonNode = mapper.readTree(response.getEntity().getContent());


I think we can remove the JsonNode usage here as well? And use XContentMapValues ?

…80406) Deprecation log indexing relies on index templates to be present in cluster state in order to create a data stream. If a very early deprecation is emitted after node is started up and templates are not loaded yet this can result in regular index with default settings and mappings to be created. To prevent this a bulk processor used for indexing deprecation logs delays flushing until templates are loaded. closes elastic#80265

…80406) (#80529) Deprecation log indexing relies on index templates to be present in cluster state in order to create a data stream. If a very early deprecation is emitted after node is started up and templates are not loaded yet this can result in regular index with default settings and mappings to be created. To prevent this a bulk processor used for indexing deprecation logs delays flushing until templates are loaded. closes #80265 backports #80406

…ckport(#80406) (#80345) Deprecation log indexing relies on index templates to be present in cluster state in order to create a data stream. If a very early deprecation is emitted after node is started up and templates are not loaded yet this can result in regular index with default settings and mappings to be created. To prevent this a bulk processor used for indexing deprecation logs delays flushing until templates are loaded. closes #80265 backport #80406

pgomulka added 3 commits November 5, 2021 11:19

Merge remote-tracking branch 'upstream/master' into main/bulk_process…

bc4ca84

…or_flush_delay

precommit

1531e5d

pgomulka added :Core/Infra/Logging Log management and logging utilities :Data Management/Other v8.0.0 v7.16.0 v8.1.0 labels Nov 5, 2021

pgomulka self-assigned this Nov 5, 2021

pgomulka added 4 commits November 5, 2021 12:25

bulk processor tests

cb21250

fix default value

65b87b4

split modules

43ee9f3

spacing and javadoc

0267422

pgomulka marked this pull request as ready for review November 5, 2021 14:56

elasticmachine added Team:Core/Infra Meta label for core/infra team Team:Data Management (obsolete) DO NOT USE. This team no longer exists. labels Nov 5, 2021

pgomulka requested review from mark-vieira and martijnvg November 5, 2021 14:56

pgomulka commented Nov 5, 2021

View reviewed changes

pgomulka added 2 commits November 5, 2021 16:11

Merge remote-tracking branch 'upstream/master' into main/bulk_process…

90dfec1

…or_flush_delay

remove licences and clean up gradle

335a5a4

martijnvg reviewed Nov 8, 2021

View reviewed changes

uxe XContentMapValues in common module

ad09899

Merge branch 'master' into main/bulk_processor_flush_delay

1a3cc24

pgomulka requested a review from martijnvg November 9, 2021 10:27

martijnvg reviewed Nov 9, 2021

View reviewed changes

pgomulka added 3 commits November 9, 2021 11:48

reformat method

ef30d92

Merge branch 'main/bulk_processor_flush_delay' of github.com:pgomulka…

9886d88

…/elasticsearch into main/bulk_processor_flush_delay

Merge remote-tracking branch 'upstream/master' into main/bulk_process…

ce1d600

…or_flush_delay

martijnvg approved these changes Nov 9, 2021

View reviewed changes

martijnvg reviewed Nov 9, 2021

View reviewed changes

pgomulka added auto-backport Automatically create backport pull requests when merged and removed auto-backport Automatically create backport pull requests when merged labels Nov 9, 2021

pgomulka merged commit b48c078 into elastic:master Nov 9, 2021

pgomulka added the >bug label Nov 10, 2021

This was referenced Nov 10, 2021

Investigate shut down problems with deprecation log indexing #79375

Closed

WIP 7.x Prevent indexing when shutting down #79697

Closed

Prevent indexing deprecations when it was disabled #79566

Closed

pugnascotia mentioned this pull request Dec 13, 2021

[elasticsearch] Elasticsearch master only is failing with UnavailableShardsException elastic/helm-charts#1462

Closed

pugnascotia added v8.0.0-rc2 and removed v8.0.0 labels Feb 1, 2022

pgomulka deleted the main/bulk_processor_flush_delay branch July 25, 2022 13:38

masseyke mentioned this pull request Feb 28, 2023

Using BulkProcessor2 in Deprecation Logging #94211

Merged

ChrisHegarty unassigned pgomulka Oct 29, 2024

		@@ -0,0 +1,41 @@
		import org.elasticsearch.gradle.util.GradleUtils


		package org.elasticsearch.xpack.deprecation;

		import com.fasterxml.jackson.databind.JsonNode;

Conversation

pgomulka commented Nov 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Nov 5, 2021

Uh oh!

elasticmachine commented Nov 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka Nov 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka Nov 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

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!

pgomulka commented Nov 8, 2021

Uh oh!

pgomulka commented Nov 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pgomulka commented Nov 5, 2021 •

edited

Loading

pgomulka Nov 5, 2021 •

edited

Loading

pgomulka Nov 5, 2021 •

edited

Loading