Bug Fix: System Data Streams Should Be Restorable by JVerwolf · Pull Request #124651 · elastic/elasticsearch

JVerwolf · 2025-03-12T14:30:22Z

This PR deletes system data streams prior to a restore operation. To enable this, I refactored the way that datastreams are deleted to allow for a single metadata update operation that both deletes existing system datastreams and then restores them as part of a single operation.

closes #89261

elasticsearchmachine · 2025-03-12T14:31:08Z

Hi @JVerwolf, I've created a changelog YAML for you.

…JVerwolf/elasticsearch into bugfix/datastreams-should-be-restorable

…atastreams-should-be-restorable

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java

rjernst

A few minor initial comments. I suggest someone from data management review.

docs/changelog/124651.yaml

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteDataStreamService.java

…JVerwolf/elasticsearch into bugfix/datastreams-should-be-restorable

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java

dakrone

I left a couple of comments from the Data Management side, but nothing major. I agree with Ryan about trying to combine with MetadataDataStreamsService instead of adding a new service if possible.

docs/changelog/124651.yaml

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteDataStreamService.java

…tion

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsService.java

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

…atastreams-should-be-restorable

dakrone · 2025-03-13T22:43:35Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsService.java

+        // first delete the data streams and then the indices:
+        // (this to avoid data stream validation from failing when deleting an index that is part of a data stream
+        // without updating the data stream)
+        // TODO: change order when "delete index api" also updates the data stream the "index to be removed" is a member of


I don't understand this TODO entirely, why would we want to change the order when invoked by the delete index API?

Me neither, but it was already there in the transport action so I kept it.

nielsbauman

LGTM, thanks a lot for the iterations, @JVerwolf!

elasticsearchmachine · 2025-03-14T15:02:15Z

💔 Backport failed

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

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

This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java

JVerwolf · 2025-03-14T18:01:14Z

💔 Some backports could not be created

Status	Branch	Result
✅	8.x
✅	9.0
❌	8.18	An unhandled error occurred. Please see the logs for details

Manual backport

To create the backport manually run:

backport --pr 124651

Questions ?

Please refer to the Backport tool documentation

This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java

JVerwolf · 2025-03-14T18:26:46Z

💚 All backports created successfully

Status	Branch	Result
✅	8.18

Questions ?

Please refer to the Backport tool documentation

This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java

JVerwolf · 2025-03-14T18:30:28Z

Curiously, the v8 backports are all failing the SystemResourceSnapshotTests (previously SystemIndexSnapshotTests) when the whole test class is run, but they all succeed when run individually. In a debugger, I see missing features that should have been added by the plugins. Seems like a test framework bug. Does anyone here have intuition about what might have changed between the 8/9 versions?

…24910) * Bug Fix: System Data Streams Should Be Restorable (#124651) This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java * Fix test bug, mute test * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…124912) * Bug Fix: System Data Streams Should Be Restorable (#124651) This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java * Fix test bug, mute test

…24900) * Bug Fix: System Data Streams Should Be Restorable (#124651) This PR adds a new MetadataDeleteDataStreamService that allows us to delete system data streams prior to a restore operation. This fixes a bug where system data streams were previously un-restorable. (cherry picked from commit cb3c357) # Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DeleteDataStreamTransportAction.java # server/src/main/java/org/elasticsearch/snapshots/RestoreService.java # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsServiceTests.java * Fix test bug * mute test

draft

e30fd2d

JVerwolf added >bug :Core/Infra/Core Core issues without another label labels Mar 12, 2025

elasticsearchmachine added the v9.1.0 label Mar 12, 2025

JVerwolf and others added 8 commits March 12, 2025 07:31

Update docs/changelog/124651.yaml

62bb968

[CI] Auto commit changes from spotless

ba59b56

Fix metadata system datastream delete service

559e4d5

Spotless

ec21ae0

Merge branch 'bugfix/datastreams-should-be-restorable' of github.com:…

0019d10

…JVerwolf/elasticsearch into bugfix/datastreams-should-be-restorable

Merge branch 'main' of github.com:elastic/elasticsearch into bugfix/d…

d77f0c6

…atastreams-should-be-restorable

Fix merge error

5379635

Clean up comments

7d19d6f

JVerwolf commented Mar 12, 2025

View reviewed changes

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java Outdated Show resolved Hide resolved

JVerwolf added 2 commits March 12, 2025 11:08

Ensure only system datastreams are deleted

c1a490a

Add test for deleted system datastreams

1b0ce59

rjernst reviewed Mar 12, 2025

View reviewed changes

docs/changelog/124651.yaml Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteDataStreamService.java Outdated Show resolved Hide resolved

JVerwolf and others added 3 commits March 12, 2025 12:13

Update changelog summary for PR 124651

530d06f

Change exception

8df4410

Merge branch 'bugfix/datastreams-should-be-restorable' of github.com:…

471b7c4

…JVerwolf/elasticsearch into bugfix/datastreams-should-be-restorable

JVerwolf added v9.0.1 v8.19.0 v8.18.1 auto-backport Automatically create backport pull requests when merged labels Mar 12, 2025

JVerwolf commented Mar 12, 2025

View reviewed changes

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SystemIndicesSnapshotIT.java Outdated Show resolved Hide resolved

JVerwolf requested a review from alexey-ivanov-es March 12, 2025 20:15

dakrone reviewed Mar 12, 2025

View reviewed changes

JVerwolf added 3 commits March 12, 2025 14:59

address feedback and update transport action

1cd7cdd

remove wip

527e1f3

Add tests

165957a

PR feedback: update testDeleteMissing and throw ResourceNotFoundExcep…

248c101

…tion

nielsbauman reviewed Mar 13, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsService.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java Outdated Show resolved Hide resolved

JVerwolf added 6 commits March 13, 2025 12:56

Fix alias restore

7fd3a13

PR feedback: move exception further up

b7c760c

PR feedback: add partial snapshot test

cf0b3b4

spotless

1d35ff5

Merge branch 'main' of github.com:elastic/elasticsearch into bugfix/d…

3bbd02a

…atastreams-should-be-restorable

Remove unused test helper

4d4fd55

JVerwolf requested review from dakrone, nielsbauman and rjernst March 13, 2025 22:29

dakrone reviewed Mar 13, 2025

View reviewed changes

Remove print line

09e2b87

nielsbauman approved these changes Mar 14, 2025

View reviewed changes

JVerwolf merged commit cb3c357 into elastic:main Mar 14, 2025
17 checks passed

elasticsearchmachine added the backport pending label Mar 14, 2025

JVerwolf mentioned this pull request Mar 14, 2025

[8.x] Bug Fix: System Data Streams Should Be Restorable (#124651) #124900

Merged

JVerwolf mentioned this pull request Mar 14, 2025

[9.0] Bug Fix: System Data Streams Should Be Restorable (#124651) #124910

Merged

JVerwolf mentioned this pull request Mar 14, 2025

[8.18] Bug Fix: System Data Streams Should Be Restorable (#124651) #124912

Merged

stefnestor mentioned this pull request Apr 3, 2025

.fleet-action-results cannot be accessed by cloud #116139

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug Fix: System Data Streams Should Be Restorable#124651

Bug Fix: System Data Streams Should Be Restorable#124651
JVerwolf merged 32 commits intoelastic:mainfrom
JVerwolf:bugfix/datastreams-should-be-restorable

JVerwolf commented Mar 12, 2025 •

edited

Loading

elasticsearchmachine commented Mar 12, 2025

Uh oh!

rjernst left a comment

Uh oh!

Uh oh!

Uh oh!

dakrone left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dakrone Mar 13, 2025

JVerwolf Mar 13, 2025 •

edited

Loading

nielsbauman left a comment

Uh oh!

elasticsearchmachine commented Mar 14, 2025

JVerwolf commented Mar 14, 2025

JVerwolf commented Mar 14, 2025

JVerwolf commented Mar 14, 2025 •

edited

Loading

Labels

6 participants

Conversation

JVerwolf commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Mar 12, 2025

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dakrone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dakrone Mar 13, 2025

Choose a reason for hiding this comment

JVerwolf Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

nielsbauman left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Mar 14, 2025

💔 Backport failed

JVerwolf commented Mar 14, 2025

💔 Some backports could not be created

Manual backport

Questions ?

JVerwolf commented Mar 14, 2025

💚 All backports created successfully

Questions ?

JVerwolf commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Labels

6 participants

JVerwolf commented Mar 12, 2025 •

edited

Loading

JVerwolf Mar 13, 2025 •

edited

Loading

JVerwolf commented Mar 14, 2025 •

edited

Loading