Skip to content

Conversation

@original-brownbear
Copy link
Contributor

With #48371 merged there is no need to keep the BwC logic
for writing snapshots in the old style.
The only thing necessary to retain is the SnapshotsInProgress.Entry
serialization BwC.

With elastic#48371 merged there is no need to keep the BwC logic
for writing snapshots in the old style.
The only thing necessary to retain is the `SnapshotsInProgress.Entry`
serialization BwC.
@original-brownbear original-brownbear added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 labels Oct 23, 2019
@elasticmachine
Copy link
Collaborator

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

@original-brownbear
Copy link
Contributor Author

Jenkins run elasticsearch-ci/bwc (failed to build only)

@original-brownbear
Copy link
Contributor Author

Jenkins run elasticsearch-ci/default-distro (random build stuff)

@original-brownbear
Copy link
Contributor Author

@elasticmachine update branch

} else {
useShardGenerations = false;
final Version version = in.getVersion();
if (version.before(Version.V_8_0_0) && version.onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

given that this logic can only be removed in 9.0, perhaps simpler to just unconditionally do a in.readBoolean(); here and add an assertion that fails once we branch 9.0. Also note that version.onOrAfter(SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION) should be unnecessary once we stop testing master against 7.5. This is also why I prefer to wait with PRs like this until that has happened, it simplifies the BWC conditions.

Copy link
Contributor Author

@original-brownbear original-brownbear Oct 24, 2019

Choose a reason for hiding this comment

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

This is also why I prefer to wait with PRs like this until that has happened, it simplifies the BWC conditions.

Right that's a fair point. I'll just close here then and look into this again once 7.6 is out :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants