Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump TransportVersion and add RecoveryCommitTooNewException #99591

Conversation

pxsalehi
Copy link
Member

No description provided.

@pxsalehi pxsalehi added :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >non-issue labels Sep 14, 2023
@pxsalehi pxsalehi marked this pull request as ready for review September 14, 2023 16:26
@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Sep 14, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM

@pxsalehi
Copy link
Member Author

@elasticmachine update branch

@@ -141,6 +141,7 @@ static TransportVersion def(int id) {
public static final TransportVersion V_8_500_074 = def(8_500_074);
public static final TransportVersion NODE_INFO_INDEX_VERSION_ADDED = def(8_500_075);
public static final TransportVersion FIRST_NEW_ID_LAYOUT = def(8_501_00_0);
public static final TransportVersion RECOVERY_COMMIT_TOO_NEW_EXCEPTION_ADDED = def(8_501_00_1);
Copy link
Member Author

@pxsalehi pxsalehi Sep 20, 2023

Choose a reason for hiding this comment

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

@thecoop it seems we are now partially using a different format for TVs. Since this seems to be a first, could you please double check if this is correct? I've just bumped the patch version.

Copy link
Member

Choose a reason for hiding this comment

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

We are using a different format - see #99640. As in the comment above, the next version should be 8_502_00_0

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

@thecoop it seems the same mistake has been made and it is on main already! See

public static final TransportVersion COMMIT_PRIMARY_TERM_GENERATION = def(8_501_00_1);

Copy link
Member Author

Choose a reason for hiding this comment

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

so the very last part (_P) is for a patch release? IOW, in a M_NXX_YY_P, M is major, that's clear. Is P patch version as in 8.10.2 ? and is N some sort of minor version? It seems for "normal" dev-related bumping (due to incompatible changes e.g.) we either increment XX or YY depending on stateful or serverless. Is that correct? M, N and P change due to releases?

Copy link
Member

Choose a reason for hiding this comment

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

P is a patch to the transport protocol (for emergency fixes etc)
NNN is the version for base elasticsearch
YY is the version for serverless only

@pxsalehi pxsalehi requested a review from thecoop September 20, 2023 07:49
@pxsalehi
Copy link
Member Author

@elasticmachine run elasticsearch-ci/part-1

failure was #99156

@pxsalehi
Copy link
Member Author

@elasticmachine run elasticsearch-ci/part-1

failure was #96578

@pxsalehi
Copy link
Member Author

@elasticmachine run elasticsearch-ci/part-1

failure was #99734

@pxsalehi pxsalehi merged commit d0e91c9 into elastic:main Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants