Skip to content

Commit 25c6cec

Browse files
committed
Add BWC layer for Exceptions
Today it's not possible to add exceptions to the serialization layer without breaking BWC. This commit adds the ability to specify the Version an exception was added that allows to fall back not NotSerializableExceptionWrapper if the expection is not present in the streams version. Relates to elastic#21656
1 parent 3662415 commit 25c6cec

File tree

6 files changed

+210
-150
lines changed

6 files changed

+210
-150
lines changed

core/src/main/java/org/elasticsearch/ElasticsearchException.java

Lines changed: 156 additions & 135 deletions
Large diffs are not rendered by default.

core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ public void writeException(Throwable throwable) throws IOException {
783783
writeVInt(17);
784784
} else {
785785
ElasticsearchException ex;
786-
if (throwable instanceof ElasticsearchException && ElasticsearchException.isRegistered(throwable.getClass())) {
786+
if (throwable instanceof ElasticsearchException && ElasticsearchException.isRegistered(throwable.getClass(), version)) {
787787
ex = (ElasticsearchException) throwable;
788788
} else {
789789
ex = new NotSerializableExceptionWrapper(throwable);

core/src/main/java/org/elasticsearch/env/ShardLockObtainFailedException.java

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,36 @@
1919

2020
package org.elasticsearch.env;
2121

22+
import org.elasticsearch.ElasticsearchException;
23+
import org.elasticsearch.common.io.stream.StreamInput;
2224
import org.elasticsearch.index.shard.ShardId;
2325

26+
import java.io.IOException;
27+
2428
/**
2529
* Exception used when the in-memory lock for a shard cannot be obtained
2630
*/
27-
public class ShardLockObtainFailedException extends Exception {
28-
private final ShardId shardId;
31+
public class ShardLockObtainFailedException extends ElasticsearchException {
2932

3033
public ShardLockObtainFailedException(ShardId shardId, String message) {
31-
super(message);
32-
this.shardId = shardId;
34+
super(buildMessage(shardId, message));
35+
this.setShard(shardId);
3336
}
3437

3538
public ShardLockObtainFailedException(ShardId shardId, String message, Throwable cause) {
36-
super(message, cause);
37-
this.shardId = shardId;
39+
super(buildMessage(shardId, message), cause);
40+
this.setShard(shardId);
41+
}
42+
43+
public ShardLockObtainFailedException(StreamInput in) throws IOException {
44+
super(in);
3845
}
3946

40-
@Override
41-
public String getMessage() {
47+
private static String buildMessage(ShardId shardId, String message) {
4248
StringBuilder sb = new StringBuilder();
4349
sb.append(shardId.toString());
4450
sb.append(": ");
45-
sb.append(super.getMessage());
51+
sb.append(message);
4652
return sb.toString();
4753
}
4854
}

core/src/main/java/org/elasticsearch/index/store/Store.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,6 @@ public static MetadataSnapshot readMetadataSnapshot(Path indexLocation, ShardId
388388
// that's fine - happens all the time no need to log
389389
} catch (FileNotFoundException | NoSuchFileException ex) {
390390
logger.info("Failed to open / find files while reading metadata snapshot");
391-
} catch (ShardLockObtainFailedException ex) {
392-
logger.info((Supplier<?>) () -> new ParameterizedMessage("{}: failed to obtain shard lock", shardId), ex);
393391
}
394392
return MetadataSnapshot.EMPTY;
395393
}

core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.elasticsearch.common.xcontent.XContentFactory;
5050
import org.elasticsearch.common.xcontent.XContentLocation;
5151
import org.elasticsearch.discovery.DiscoverySettings;
52+
import org.elasticsearch.env.ShardLockObtainFailedException;
5253
import org.elasticsearch.index.AlreadyExpiredException;
5354
import org.elasticsearch.index.Index;
5455
import org.elasticsearch.index.engine.RecoveryEngineException;
@@ -107,6 +108,7 @@
107108
import static java.util.Collections.emptySet;
108109
import static java.util.Collections.singleton;
109110
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
111+
import static org.hamcrest.Matchers.instanceOf;
110112

111113
public class ExceptionSerializationTests extends ESTestCase {
112114

@@ -160,10 +162,10 @@ private void checkClass(Class<?> clazz) {
160162
if (isEsException(clazz) == false) {
161163
return;
162164
}
163-
if (ElasticsearchException.isRegistered(clazz.asSubclass(Throwable.class)) == false
165+
if (ElasticsearchException.isRegistered(clazz.asSubclass(Throwable.class), Version.CURRENT) == false
164166
&& ElasticsearchException.class.equals(clazz.getEnclosingClass()) == false) {
165167
notRegistered.add(clazz);
166-
} else if (ElasticsearchException.isRegistered(clazz.asSubclass(Throwable.class))) {
168+
} else if (ElasticsearchException.isRegistered(clazz.asSubclass(Throwable.class), Version.CURRENT)) {
167169
registered.add(clazz);
168170
try {
169171
if (clazz.getMethod("writeTo", StreamOutput.class) != null) {
@@ -218,10 +220,17 @@ public TestException(StreamInput in) throws IOException {
218220
}
219221

220222
private <T extends Exception> T serialize(T exception) throws IOException {
221-
ElasticsearchAssertions.assertVersionSerializable(VersionUtils.randomVersion(random()), exception);
223+
return serialize(exception, VersionUtils.randomVersion(random()));
224+
}
225+
226+
private <T extends Exception> T serialize(T exception, Version version) throws IOException {
227+
ElasticsearchAssertions.assertVersionSerializable(version, exception);
222228
BytesStreamOutput out = new BytesStreamOutput();
229+
out.setVersion(version);
223230
out.writeException(exception);
231+
224232
StreamInput in = out.bytes().streamInput();
233+
in.setVersion(version);
225234
return in.readException();
226235
}
227236

@@ -769,6 +778,7 @@ public void testIds() {
769778
ids.put(144, org.elasticsearch.cluster.NotMasterException.class);
770779
ids.put(145, org.elasticsearch.ElasticsearchStatusException.class);
771780
ids.put(146, org.elasticsearch.tasks.TaskCancelledException.class);
781+
ids.put(147, org.elasticsearch.env.ShardLockObtainFailedException.class);
772782

773783
Map<Class<? extends ElasticsearchException>, Integer> reverse = new HashMap<>();
774784
for (Map.Entry<Integer, Class<? extends ElasticsearchException>> entry : ids.entrySet()) {
@@ -826,4 +836,28 @@ public void testElasticsearchRemoteException() throws IOException {
826836
assertEquals(ex.status(), e.status());
827837
assertEquals(RestStatus.TOO_MANY_REQUESTS, e.status());
828838
}
839+
840+
public void testShardLockObtainFailedException() throws IOException {
841+
ShardId shardId = new ShardId("foo", "_na_", 1);
842+
ShardLockObtainFailedException orig = new ShardLockObtainFailedException(shardId, "boom");
843+
Version version = VersionUtils.randomVersionBetween(random(),
844+
Version.V_5_0_0, Version.CURRENT);
845+
if (version.before(ElasticsearchException.V_5_1_0_UNRELEASED)) {
846+
// remove this once 5_1_0 is released randomVersionBetween asserts that this version is in the constant table..
847+
version = ElasticsearchException.V_5_1_0_UNRELEASED;
848+
}
849+
ShardLockObtainFailedException ex = serialize(orig, version);
850+
assertEquals(orig.getMessage(), ex.getMessage());
851+
assertEquals(orig.getShardId(), ex.getShardId());
852+
}
853+
854+
public void testBWCShardLockObtainFailedException() throws IOException {
855+
ShardId shardId = new ShardId("foo", "_na_", 1);
856+
ShardLockObtainFailedException orig = new ShardLockObtainFailedException(shardId, "boom");
857+
Exception ex = serialize((Exception)orig, Version.V_5_0_0);
858+
assertThat(ex, instanceOf(NotSerializableExceptionWrapper.class));
859+
assertEquals("shard_lock_obtain_failed_exception: [foo][1]: boom", ex.getMessage());
860+
}
861+
862+
829863
}

core/src/test/java/org/elasticsearch/VersionTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ public void testUnknownVersions() {
284284
assertUnknownVersion(OsStats.V_5_1_0); // once we released 5.1.0 and it's added to Version.java we need to remove this constant
285285
assertUnknownVersion(QueryStringQueryBuilder.V_5_1_0_UNRELEASED);
286286
assertUnknownVersion(SimpleQueryStringBuilder.V_5_1_0_UNRELEASED);
287+
assertUnknownVersion(ElasticsearchException.V_5_1_0_UNRELEASED);
287288
// once we released 5.0.0 and it's added to Version.java we need to remove this constant
288289
assertUnknownVersion(Script.V_5_1_0_UNRELEASED);
289290
// once we released 5.0.0 and it's added to Version.java we need to remove this constant

0 commit comments

Comments
 (0)