Prohibit restoring a data stream alias with a conflicting write data stream #81217
Conversation
…stream Prohibit restoring a data stream alias if its write data stream conflicts with the write data stream of an existing data stream alias. Currently, the write data stream of data stream alias in the snapshot is ignored and the write data stream of the data stream alias currently active in the cluster is always picked. Closes elastic#80976
…tore_different_write_data_streams
|
Pinging @elastic/es-data-management (Team:Data Management) |
| if (mergedDataStreams.contains(previous.getWriteDataStream())) { | ||
| writeDataStream = previous.getWriteDataStream(); | ||
| } | ||
| // else throw error? |
There was a problem hiding this comment.
Does the case referenced by this comment need to be handled? I don't know how an alias would get into the state where it didn't have its write data stream, but it does seem like we shouldn't just fall through in that case.
There was a problem hiding this comment.
Also, do we need to handle the case where this instance has a write data stream and the previous instance does not have a write data stream?
There was a problem hiding this comment.
Does the case referenced by this comment need to be handled? I don't know how an alias would get into the state where it didn't have its write data stream, but it does seem like we shouldn't just fall through in that case.
Previously we were also not handling this, this is why I added the comment here. I think we should handle it and throw an IllegalStateException if this happens.
Also, do we need to handle the case where this instance has a write data stream and the previous instance does not have a write data stream?
We kind of handle this via an assertion in the constructor in DataStreamAlias. If we would handle this via a normal exception then I think it shouldn't be handled here but the mentioned constructor.
There was a problem hiding this comment.
I don't think it is possible to get into the state whether the write data stream of previous instance doesn't exist in mergedDataStreams set:
- Defining a data stream alias with a write data stream that does't exist in the list of data streams isn't possible. Mainly because when defining a write data stream then this is done via
is_write_indexboolean flag from the update aliases api and templates. Also there is the assertion inDataStreamAliasclass' constructor. - The
mergedDataStreamsset is based on the data streams of boththisandpreviousinstances.
I've turned this into an assertion in the latest commit that I've pushed.
…tore_different_write_data_streams
|
Thanks, @martijnvg. LGTM. |
…stream (elastic#81217) Prohibit restoring a data stream alias if its write data stream conflicts with the write data stream of an existing data stream alias. Currently, the write data stream of data stream alias in the snapshot is ignored and the write data stream of the data stream alias currently active in the cluster is always picked. In order to resolve this properly, I've merged the merge(...) and renameDataStreams(...) methods into a restore(...) method, which merges a data stream and does renaming at the same time. This should be ok, since renaming can only be done as part of restoring. More importantly, this allows to validate whether an existing alias and an alias from a snapshot to not have conflicting write data streams while at the same time allowing that a write data stream to be renamed (because data streams got renamed). Closes elastic#80976
💔 Backport failed
You can use sqren/backport to manually backport by running |
…stream (#81217) (#81843) Prohibit restoring a data stream alias if its write data stream conflicts with the write data stream of an existing data stream alias. Currently, the write data stream of data stream alias in the snapshot is ignored and the write data stream of the data stream alias currently active in the cluster is always picked. In order to resolve this properly, I've merged the merge(...) and renameDataStreams(...) methods into a restore(...) method, which merges a data stream and does renaming at the same time. This should be ok, since renaming can only be done as part of restoring. More importantly, this allows to validate whether an existing alias and an alias from a snapshot to not have conflicting write data streams while at the same time allowing that a write data stream to be renamed (because data streams got renamed). Closes #80976
Prohibit restoring a data stream alias if its write data stream conflicts with
the write data stream of an existing data stream alias.
Currently, the write data stream of data stream alias in the snapshot is ignored and
the write data stream of the data stream alias currently active in the cluster
is always picked.
In order to resolve this properly, I've merged the
merge(...)andrenameDataStreams(...)methods into a
restore(...)method, which merges a data stream and does renaming atthe same time. This should be ok, since renaming can only be done as part of restoring.
More importantly, this allows to validate whether an existing alias and
an alias from a snapshot to not have conflicting write data streams while at the same time
allowing that a write data stream to be renamed (because data streams got renamed).
Closes #80976