Skip to content

Require soft deletes for searchable snapshots#55453

Merged
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2020-04-20-require-soft-deletes-for-searchable-snapshots
Apr 20, 2020
Merged

Require soft deletes for searchable snapshots#55453
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2020-04-20-require-soft-deletes-for-searchable-snapshots

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner commented Apr 20, 2020

A simpler fix that lets us support deleted docs in searchable snapshots.

@DaveCTurner DaveCTurner added >enhancement :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.8.0 labels Apr 20, 2020
@DaveCTurner DaveCTurner requested a review from tlrx April 20, 2020 10:50
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Apr 20, 2020

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

DaveCTurner
DaveCTurner commented Apr 20, 2020
View reviewed changes
...src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java
);

createIndex(indexName);
assertAcked(prepareCreate(indexName, Settings.builder().put(INDEX_SOFT_DELETES_SETTING.getKey(), true)));
Copy link
Copy Markdown
Member Author

@DaveCTurner DaveCTurner Apr 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact this is always true in master, but not in 7.x. Setting it in both places for ease of backport.

@ywelsch
Copy link
Copy Markdown
Contributor

ywelsch commented Apr 20, 2020 •
edited
Loading

I would rather like to avoid this, as it won't allow us to load most indices created with 6.x.

Can we just adopt the test change for now with a new TODO?

@DaveCTurner
Copy link
Copy Markdown
Member Author

DaveCTurner commented Apr 20, 2020

Do you mean to revert the change to SearchableSnapshotDirectory and not throw an IAE if soft deletes is disabled? Yes, we can do that, the main drawback is that if you try and mount a 6.x snapshot that contains deletes then you will get mysterious shard failures.

I expect we'll be using Lucene 8.6 and therefore be able to fix this properly before too long.

@DaveCTurner
Copy link
Copy Markdown
Member Author

DaveCTurner commented Apr 20, 2020

@elasticmachine please run elasticsearch-ci/1

@DaveCTurner DaveCTurner merged commit b8ccf79 into elastic:master Apr 20, 2020
@DaveCTurner DaveCTurner deleted the 2020-04-20-require-soft-deletes-for-searchable-snapshots branch April 20, 2020 13:17
DaveCTurner added a commit that referenced this pull request Apr 20, 2020
@DaveCTurner
Use soft deletes for searchable snapshots tests (#55453)
0df329d 
This allows us to perform some dummy indexing including updates/deletes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tlrx tlrx Awaiting requested review from tlrx

1 more reviewer

@ywelsch ywelsch ywelsch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v7.8.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DaveCTurner @elasticmachine @ywelsch @jakelandis