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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 21, 2023
Merged

Bump TransportVersion and add RecoveryCommitTooNewException #99591

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0a4e362
Bump TransportVersion and add RecoveryCommitTooNewException
pxsalehi Sep 14, 2023
7cdb7c8
Merge branch 'main' into ps230914-bumpTV-addRecoveryCommitTooNewExcep…
elasticmachine Sep 18, 2023
25387e3
spotless
pxsalehi Sep 18, 2023
1b30915
add the new exception to the test
pxsalehi Sep 18, 2023
3e2dc98
Merge remote-tracking branch 'upstream/main' into ps230914-bumpTV-add…
pxsalehi Sep 20, 2023
bf272ef
correct TV
pxsalehi Sep 20, 2023
96d9faf
Merge remote-tracking branch 'upstream/main' into ps230914-bumpTV-add…
pxsalehi Sep 20, 2023
ed061e3
Merge remote-tracking branch 'upstream/main' into ps230914-bumpTV-add…
pxsalehi Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion server/src/main/java/org/elasticsearch/ElasticsearchException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.index.Index;
import org.elasticsearch.index.mapper.DocumentParsingException;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.indices.recovery.RecoveryCommitTooNewException;
import org.elasticsearch.rest.ApiNotAvailableException;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.SearchException;
Expand Down Expand Up @@ -1846,7 +1847,13 @@ private enum ElasticsearchExceptionHandle {
170,
TransportVersions.V_8_500_020
),
API_NOT_AVAILABLE_EXCEPTION(ApiNotAvailableException.class, ApiNotAvailableException::new, 171, TransportVersions.V_8_500_065);
API_NOT_AVAILABLE_EXCEPTION(ApiNotAvailableException.class, ApiNotAvailableException::new, 171, TransportVersions.V_8_500_065),
RECOVERY_COMMIT_TOO_NEW_EXCEPTION(
RecoveryCommitTooNewException.class,
RecoveryCommitTooNewException::new,
172,
TransportVersions.RECOVERY_COMMIT_TOO_NEW_EXCEPTION_ADDED
);

final Class<? extends ElasticsearchException> exceptionClass;
final CheckedFunction<StreamInput, ? extends ElasticsearchException, IOException> constructor;
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/TransportVersions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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

elasticsearch/server/src/main/java/org/elasticsearch/TransportVersions.java

Line 144 in 68ddb19

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

/*
* STOP! READ THIS FIRST! No, really,
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _
Expand Down
26 changes: 26 additions & 0 deletions server/src/main/java/org/elasticsearch/indices/recovery/RecoveryCommitTooNewException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.indices.recovery;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.index.shard.ShardId;

import java.io.IOException;

public class RecoveryCommitTooNewException extends ElasticsearchException {
public RecoveryCommitTooNewException(ShardId shardId, String message) {
super(message);
setShard(shardId);
}

public RecoveryCommitTooNewException(StreamInput in) throws IOException {
super(in);
}
}
2 changes: 2 additions & 0 deletions server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.elasticsearch.indices.InvalidIndexTemplateException;
import org.elasticsearch.indices.recovery.PeerRecoveryNotFound;
import org.elasticsearch.indices.recovery.RecoverFilesRecoveryException;
import org.elasticsearch.indices.recovery.RecoveryCommitTooNewException;
import org.elasticsearch.ingest.IngestProcessorException;
import org.elasticsearch.repositories.RepositoryConflictException;
import org.elasticsearch.repositories.RepositoryException;
Expand Down Expand Up @@ -832,6 +833,7 @@ public void testIds() {
ids.put(169, HttpHeadersValidationException.class);
ids.put(170, ElasticsearchRoleRestrictionException.class);
ids.put(171, ApiNotAvailableException.class);
ids.put(172, RecoveryCommitTooNewException.class);

Map<Class<? extends ElasticsearchException>, Integer> reverse = new HashMap<>();
for (Map.Entry<Integer, Class<? extends ElasticsearchException>> entry : ids.entrySet()) {
Expand Down